[webkit-reviews] review granted: [Bug 209312] [LFC][IFC] Bail out when busy looping on partial content : [Attachment 394059] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted zalan
<zalan at apple.com>'s request for review:
Bug 209312: [LFC][IFC] Bail out when busy looping on partial content
https://bugs.webkit.org/show_bug.cgi?id=209312

Attachment 394059: Patch

https://bugs.webkit.org/attachment.cgi?id=394059&action=review




--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 394059
  --> https://bugs.webkit.org/attachment.cgi?id=394059
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=394059&action=review

> Source/WebCore/ChangeLog:3
> +	   [LFC][IFC] Bail out when busy looping on partial content

"Avoid infinite loop when..."

> Source/WebCore/ChangeLog:9
> +	   Speculative fix to address busy looping/running out of inline run
vector capacity at InlineFormattingContext::setDisplayBoxesForLine.

"busy looping" sounds like something that happens over time. Is it really an
infinite loop?

> Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:162
> +		   // "sp<->litcontent" -> overflow length: 10 -> leading
partial content length: 10.

Weird comment.

> Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:175
> +		       ASSERT_NOT_REACHED();
> +		       // Move over to the next run if we are stuck on this
partial content (when the overflow content length remains the same).
> +		       // We certainly lose some content, but we would be busy
looping anyway.
> +		       previousLineEnd = PreviousLineEnd { trailingRunIndex, {
} };
> +		       layoutRange.start = previousLineEnd->runIndex + 1;

Weird to assert and then run code.