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

Description zalan 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.
Comment 1 zalan 2016-11-07 20:49:15 PST
Created attachment 294129 [details]
Patch
Comment 2 Simon Fraser (smfr) 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.
Comment 3 Antti Koivisto 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?
Comment 4 zalan 2016-11-08 22:26:14 PST
Created attachment 294217 [details]
Patch
Comment 5 zalan 2016-11-09 11:18:23 PST
Created attachment 294252 [details]
Patch
Comment 6 Antti Koivisto 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.
Comment 7 zalan 2016-11-09 11:35:58 PST
Created attachment 294255 [details]
Patch
Comment 8 zalan 2016-11-09 11:37:12 PST
Created attachment 294256 [details]
Patch
Comment 9 WebKit Commit Bot 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>
Comment 10 WebKit Commit Bot 2016-11-09 12:41:37 PST
All reviewed patches have been landed.  Closing bug.