Bug 244735 – [LFC][IFC] Cleanup InlineFormattingContext::lineLayout

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

ssia

[zalan]

Created attachment 462106 [details]
Patch

[Antti Koivisto]

Comment on attachment 462106 [details]
Patch

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

> Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:219
> +    if (!previousLine)
> +        return lineContentRange.end - 1;
> +
> +    auto lineLayoutHasAdvanced = lineContentRange.end > previousLineLastInlineItemIndex
> +        || (previousLine->partialOverflowingContent && previousLine->partialOverflowingContent->length > lineContent.partialOverflowingContent->length);
> +    if (!lineLayoutHasAdvanced) {
> +        // 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 otherwise.
> +        ASSERT_NOT_REACHED();
> +        return lineContentRange.end;
> +    }
> +    ASSERT(lineContentRange.end);
> +    return lineContentRange.end - 1;

Isn't this whole thing equivalent of just 

return lineContentRange.end - 1;

(except for the path that asserst.

[zalan]

(In reply to Antti Koivisto from comment #2)
> Comment on attachment 462106 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=462106&action=review
> 
> > Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:219
> > +    if (!previousLine)
> > +        return lineContentRange.end - 1;
> > +
> > +    auto lineLayoutHasAdvanced = lineContentRange.end > previousLineLastInlineItemIndex
> > +        || (previousLine->partialOverflowingContent && previousLine->partialOverflowingContent->length > lineContent.partialOverflowingContent->length);
> > +    if (!lineLayoutHasAdvanced) {
> > +        // 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 otherwise.
> > +        ASSERT_NOT_REACHED();
> > +        return lineContentRange.end;
> > +    }
> > +    ASSERT(lineContentRange.end);
> > +    return lineContentRange.end - 1;
> 
> Isn't this whole thing equivalent of just 
> 
> return lineContentRange.end - 1;
> 
> (except for the path that asserst.
Yeah you are right, I should collapse them.

[zalan]

Created attachment 462120 [details]
Patch

[zalan]

Created attachment 462121 [details]
Patch

[zalan]

Created attachment 462123 [details]
Patch

[EWS]

Committed 254150@main (29727b096d48): <https://commits.webkit.org/254150@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 462123 [details].

[Radar WebKit Bug Importer]

<rdar://problem/99551574>