Bug 244735

Summary: [LFC][IFC] Cleanup InlineFormattingContext::lineLayout
Product: WebKit Reporter: alan <zalan>
Component: Layout and RenderingAssignee: alan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

alan
Reported 2022-09-02 15:49:59 PDT
ssia
Attachments
Patch (16.93 KB, patch)
2022-09-02 15:54 PDT, alan 		no flags
DetailsFormatted DiffDiff
Patch (16.93 KB, patch)
2022-09-04 10:03 PDT, alan 		no flags
DetailsFormatted DiffDiff
Patch (16.98 KB, patch)
2022-09-04 10:17 PDT, alan 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (16.86 KB, patch)
2022-09-04 11:24 PDT, alan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
alan
Comment 1 2022-09-02 15:54:27 PDT
Created attachment 462106 [details] Patch
Antti Koivisto
Comment 2 2022-09-04 07:36:12 PDT
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.
alan
Comment 3 2022-09-04 09:45:41 PDT
(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.
alan
Comment 4 2022-09-04 10:03:59 PDT
Created attachment 462120 [details] Patch
alan
Comment 5 2022-09-04 10:17:57 PDT
Created attachment 462121 [details] Patch
alan
Comment 6 2022-09-04 11:24:41 PDT
Created attachment 462123 [details] Patch
EWS
Comment 7 2022-09-04 15:08:13 PDT
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
Comment 8 2022-09-04 15:09:15 PDT
<rdar://problem/99551574>
Note You need to log in before you can comment on or make changes to this bug.