Bug 164503

Summary: Move RenderNamedFlowThread nextRendererForElement logic to RenderTreeUpdater.
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, koivisto, kondapallykalyan, simon.fraser, WebkitBugTracker
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

zalan
Reported 2016-11-07 20:44:43 PST
This logic has no render tree dependency. Also by moving the logic to RenderTreeUpdate, we can remove RenderNamedFlowThread::m_flowThreadChildList -> more secure.
Attachments
Patch (12.35 KB, patch)
2016-11-07 20:49 PST, zalan 		no flags
DetailsFormatted DiffDiff
Patch (15.59 KB, patch)
2016-11-08 22:26 PST, zalan 		no flags
DetailsFormatted DiffDiff
Patch (15.67 KB, patch)
2016-11-09 11:18 PST, zalan 		no flags
DetailsFormatted DiffDiff
Patch (15.60 KB, patch)
2016-11-09 11:35 PST, zalan 		no flags
DetailsFormatted DiffDiff
Patch (15.60 KB, patch)
2016-11-09 11:37 PST, zalan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
zalan
Comment 1 2016-11-07 20:49:15 PST
Created attachment 294129 [details] Patch
Simon Fraser (smfr)
Comment 2 2016-11-07 21:42:29 PST
Comment on attachment 294129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294129&action=review > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + No change in functionality. Moar words here pls.
Antti Koivisto
Comment 3 2016-11-07 22:41:15 PST
Comment on attachment 294129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294129&action=review > Source/WebCore/style/RenderTreeUpdater.cpp:335 > renderTreePosition().computeNextSibling(element); > - > - RenderTreePosition insertionPosition = parentFlowRenderer > - ? RenderTreePosition(*parentFlowRenderer, parentFlowRenderer->nextRendererForElement(element)) > - : renderTreePosition(); > - > + RenderTreePosition insertionPosition = !parentFlowRenderer ? renderTreePosition() : RenderTreePosition(*parentFlowRenderer, > + renderTreePosition().nextSiblingRenderer(element, m_styleUpdate->elementStyle(element))); Wouldn't just using 'style' here work instead of 'm_styleUpdate->elementStyle(element)'? Could we just provide style or Style::Update& to computeNextSibling() above and eliminate the parentFlowRenderer special case completely?
zalan
Comment 4 2016-11-08 22:26:14 PST
Created attachment 294217 [details] Patch
zalan
Comment 5 2016-11-09 11:18:23 PST
Created attachment 294252 [details] Patch
Antti Koivisto
Comment 6 2016-11-09 11:23:54 PST
Comment on attachment 294252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294252&action=review > Source/WebCore/style/RenderTreeUpdater.cpp:324 > + bool willMoveToFlowThread = element.shouldMoveToFlowThread(style); > + > + if (willMoveToFlowThread) Doesn't really need boolean here.
zalan
Comment 7 2016-11-09 11:35:58 PST
Created attachment 294255 [details] Patch
zalan
Comment 8 2016-11-09 11:37:12 PST
Created attachment 294256 [details] Patch
WebKit Commit Bot
Comment 9 2016-11-09 12:41:32 PST
Comment on attachment 294256 [details] Patch Clearing flags on attachment: 294256 Committed r208470: <http://trac.webkit.org/changeset/208470>
WebKit Commit Bot
Comment 10 2016-11-09 12:41:37 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.