Bug 183273

Summary: [RenderTreeBuilder] Move styleDidChange mutation logic to RenderTreeUpdater
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, ews-watchlist, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews114 for mac-sierra
none
Patch
none
Patch
none
Patch none

Description zalan 2018-03-01 21:19:50 PST
ssia.
Comment 1 Radar WebKit Bug Importer 2018-03-01 21:20:12 PST
<rdar://problem/38054892>
Comment 2 zalan 2018-03-01 21:23:34 PST
Created attachment 334877 [details]
Patch
Comment 3 EWS Watchlist 2018-03-01 22:54:45 PST
Comment on attachment 334877 [details]
Patch

Attachment 334877 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/6729312

New failing tests:
fullscreen/full-screen-fixed-pos-parent.html
Comment 4 EWS Watchlist 2018-03-01 22:54:46 PST
Created attachment 334879 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 5 zalan 2018-03-02 10:30:16 PST
Created attachment 334906 [details]
Patch
Comment 6 zalan 2018-03-02 10:38:50 PST
Created attachment 334907 [details]
Patch
Comment 7 Antti Koivisto 2018-03-03 02:46:39 PST
Comment on attachment 334907 [details]
Patch

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

> Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:341
> +    auto& parent = *renderer.parent();
> +    bool isFloating = renderer.style().isFloating();
> +    bool isOutOfFlowPositioned = renderer.style().hasOutOfFlowPosition();
> +    bool startsAffectingParent = false;
> +    bool noLongerAffectsParent = false;
> +
> +    if (is<RenderBlock>(parent))
> +        noLongerAffectsParent = (!wasFloating && isFloating) || (!wasOufOfFlowPositioned && isOutOfFlowPositioned);
> +
> +    if (is<RenderBlockFlow>(parent) || is<RenderInline>(parent)) {
> +        startsAffectingParent = (wasFloating || wasOufOfFlowPositioned) && !isFloating && !isOutOfFlowPositioned;
> +        ASSERT(!startsAffectingParent || !noLongerAffectsParent);
> +    }
> +
> +    if (startsAffectingParent) {
> +        // We have gone from not affecting the inline status of the parent flow to suddenly
> +        // having an impact. See if there is a mismatch between the parent flow's
> +        // childrenInline() state and our state.
> +        renderer.setInline(renderer.style().isDisplayInlineType());
> +        if (renderer.isInline() != renderer.parent()->childrenInline())
> +            m_builder.childFlowStateChangesAndAffectsParentBlock(renderer);
> +        return;
> +    }
> +
> +    if (noLongerAffectsParent) {
> +        m_builder.childFlowStateChangesAndNoLongerAffectsParentBlock(renderer);
> +
> +        if (is<RenderBlockFlow>(renderer)) {
> +            // Fresh floats need to be reparented if they actually belong to the previous anonymous block.
> +            // It copies the logic of RenderBlock::addChildIgnoringContinuation
> +            if (isFloating && renderer.previousSibling() && renderer.previousSibling()->isAnonymousBlock())
> +                m_builder.move(downcast<RenderBoxModelObject>(parent), downcast<RenderBoxModelObject>(*renderer.previousSibling()), renderer, RenderTreeBuilder::NormalizeAfterInsertion::No);
> +        }
> +    }
> +}

Maybe this stuff could go to the Builder as mutateTreeAfterStyleChange() or something?
Comment 8 zalan 2018-03-03 07:32:51 PST
Created attachment 334957 [details]
Patch
Comment 9 WebKit Commit Bot 2018-03-03 12:13:47 PST
Comment on attachment 334957 [details]
Patch

Clearing flags on attachment: 334957

Committed r229200: <https://trac.webkit.org/changeset/229200>
Comment 10 WebKit Commit Bot 2018-03-03 12:13:48 PST
All reviewed patches have been landed.  Closing bug.