Changeset 244215 in webkit


Ignore:
Timestamp:
Apr 12, 2019 7:54:12 AM (5 years ago)
Author:
Simon Fraser
Message:

[iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overflow scroll
​https://bugs.webkit.org/show_bug.cgi?id=196146

Reviewed by Antti Koivisto.

Source/WebCore:

computeCoordinatedPositioningForLayer() failed to handle nested positions elements
inside overflow scroll, because it only walked up to the first containing block of
a nested position:absolute. We need to walk all the way up the ancestor layer chain,
looking at containing block, scroller and composited ancestor relationships.

Make this code easier to understand by writing it in terms of "is foo scrolled by bar", rather than
trying to collapse all the logic into a single ancestor walk, which was really hard. This is a few
more ancestor traversals, but we now only run this code if there's composited scrolling
in the ancestor chain.

Tests: scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html

scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html
scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html
scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html

  • rendering/RenderLayerCompositor.cpp:

(WebCore::enclosingCompositedScrollingLayer):
(WebCore::isScrolledByOverflowScrollLayer):
(WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
(WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary):
(WebCore::collectStationaryLayerRelatedOverflowNodes):
(WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
(WebCore::collectRelatedCoordinatedScrollingNodes):
(WebCore::layerParentedAcrossCoordinatedScrollingBoundary): Deleted.

LayoutTests:

Dump the scrolling tree for various configurations of positioned, overflow and stacking context
elements.

  • fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt: Progressed results.
  • platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
  • platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
  • platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
  • platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
  • scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html: Added.
Location:
trunk
Files:
12 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r244213 r244215  
     12019-04-11  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overflow scroll
     4        https://bugs.webkit.org/show_bug.cgi?id=196146
     5
     6        Reviewed by Antti Koivisto.
     7       
     8        Dump the scrolling tree for various configurations of positioned, overflow and stacking context
     9        elements.
     10
     11        * fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt: Progressed results.
     12        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
     13        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
     14        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
     15        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
     16        * scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow-expected.txt: Added.
     17        * scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html: Added.
     18        * scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow-expected.txt: Added.
     19        * scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html: Added.
     20        * scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow-expected.txt: Added.
     21        * scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html: Added.
     22        * scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow-expected.txt: Added.
     23        * scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html: Added.
     24
    1252019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
    226

  • trunk/LayoutTests/fast/scrolling/ios/overflow-scroll-overlap-6-expected.txt

    r243380 r244215  
    22
    33case 1:
    4 case 2: Scrollable 2
    5 case 3: Scrollable 3
     4case 2:
     5case 3:
    66

  • trunk/Source/WebCore/ChangeLog

    r244213 r244215  
     12019-04-11  Simon Fraser  <simon.fraser@apple.com>
     2
     3        [iOS WK2] Wrong scrolling behavior for nested absolute position elements inside overflow scroll
     4        https://bugs.webkit.org/show_bug.cgi?id=196146
     5
     6        Reviewed by Antti Koivisto.
     7       
     8        computeCoordinatedPositioningForLayer() failed to handle nested positions elements
     9        inside overflow scroll, because it only walked up to the first containing block of
     10        a nested position:absolute. We need to walk all the way up the ancestor layer chain,
     11        looking at containing block, scroller and composited ancestor relationships.
     12
     13        Make this code easier to understand by writing it in terms of "is foo scrolled by bar", rather than
     14        trying to collapse all the logic into a single ancestor walk, which was really hard. This is a few
     15        more ancestor traversals, but we now only run this code if there's composited scrolling
     16        in the ancestor chain.
     17
     18        Tests: scrollingcoordinator/scrolling-tree/nested-absolute-in-absolute-overflow.html
     19               scrollingcoordinator/scrolling-tree/nested-absolute-in-overflow.html
     20               scrollingcoordinator/scrolling-tree/nested-absolute-in-relative-in-overflow.html
     21               scrollingcoordinator/scrolling-tree/nested-absolute-in-sc-overflow.html
     22
     23        * rendering/RenderLayerCompositor.cpp:
     24        (WebCore::enclosingCompositedScrollingLayer):
     25        (WebCore::isScrolledByOverflowScrollLayer):
     26        (WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
     27        (WebCore::RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary):
     28        (WebCore::collectStationaryLayerRelatedOverflowNodes):
     29        (WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
     30        (WebCore::collectRelatedCoordinatedScrollingNodes):
     31        (WebCore::layerParentedAcrossCoordinatedScrollingBoundary): Deleted.
     32
    1332019-04-12  Manuel Rego Casasnovas  <rego@igalia.com>
    234

  • trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

    r244209 r244215  
    29052905}
    29062906
    2907 // Is this layer's containingBlock an ancestor of scrollable overflow, and is the layer's compositing ancestor inside that overflow?
     2907static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
     2908{
     2909    const auto* currLayer = &layer;
     2910    while (currLayer) {
     2911        if (currLayer == &intermediateLayer)
     2912            sawIntermediateLayer = true;
     2913
     2914        if (currLayer->hasCompositedScrollableOverflow())
     2915            return const_cast<RenderLayer*>(currLayer);
     2916
     2917        currLayer = currLayer->parent();
     2918    }
     2919
     2920    return nullptr;
     2921}
     2922
     2923// Return true if overflowScrollLayer is in layer's containing block chain.
     2924static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
     2925{
     2926    bool containingBlockCanSkipLayers = layer.renderer().isAbsolutelyPositioned();
     2927
     2928    for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
     2929        bool inContainingBlockChain = true;
     2930        if (containingBlockCanSkipLayers) {
     2931            inContainingBlockChain = currLayer->renderer().canContainAbsolutelyPositionedObjects();
     2932            if (inContainingBlockChain)
     2933                containingBlockCanSkipLayers = currLayer->renderer().isAbsolutelyPositioned();
     2934        }
     2935
     2936        if (currLayer == &overflowScrollLayer)
     2937            return inContainingBlockChain;
     2938    }
     2939
     2940    return false;
     2941}
     2942
     2943static bool isNonScrolledLayerInsideScrolledCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor, const RenderLayer& scrollingAncestor)
     2944{
     2945    bool ancestorMovedByScroller = &compositedAncestor == &scrollingAncestor || isScrolledByOverflowScrollLayer(compositedAncestor, scrollingAncestor);
     2946    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, scrollingAncestor);
     2947
     2948    return ancestorMovedByScroller && !layerMovedByScroller;
     2949}
     2950
    29082951bool RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
    29092952{
     2953    bool compositedAncestorIsInsideScroller = false;
     2954    auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, compositedAncestor, compositedAncestorIsInsideScroller);
     2955    if (!scrollingAncestor) {
     2956        ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
     2957        return false;
     2958    }
     2959   
     2960    if (!compositedAncestorIsInsideScroller)
     2961        return false;
     2962
     2963    return isNonScrolledLayerInsideScrolledCompositedAncestor(layer, compositedAncestor, *scrollingAncestor);
     2964}
     2965
     2966static void collectStationaryLayerRelatedOverflowNodes(const RenderLayer& layer, const RenderLayer& /*compositedAncestor*/, Vector<ScrollingNodeID>& scrollingNodes)
     2967{
     2968    ASSERT(layer.isComposited());
     2969   
     2970    auto appendOverflowLayerNodeID = [&scrollingNodes] (const RenderLayer& overflowLayer) {
     2971        ASSERT(overflowLayer.isComposited());
     2972        auto scrollingNodeID = overflowLayer.backing()->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
     2973        if (scrollingNodeID)
     2974            scrollingNodes.append(scrollingNodeID);
     2975        else
     2976            LOG(Scrolling, "Layer %p doesn't have scrolling node ID yet", &overflowLayer);
     2977    };
     2978
    29102979    ASSERT(layer.renderer().isAbsolutelyPositioned());
    2911 
    2912     bool sawCompositingAncestor = false;
     2980    bool containingBlockCanSkipLayers = true;
     2981
    29132982    for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
    2914         if (currLayer->renderer().canContainAbsolutelyPositionedObjects())
    2915             return false;
    2916 
    2917         if (currLayer == &compositedAncestor)
    2918             sawCompositingAncestor = true;
    2919 
    2920         if (currLayer->hasCompositedScrollableOverflow())
    2921             return sawCompositingAncestor;
    2922     }
    2923 
    2924     return false;
    2925 }
    2926 
    2927 // Is there scrollable overflow between this layer and its composited ancestor, and the containing block is the scroller or inside the scroller.
    2928 static bool layerParentedAcrossCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor, bool checkContainingBlock, bool& containingBlockIsInsideOverflow)
    2929 {
    2930     ASSERT(layer.isComposited());
    2931 
    2932     for (const auto* currLayer = layer.parent(); currLayer != &compositedAncestor; currLayer = currLayer->parent()) {
    2933         if (checkContainingBlock && currLayer->renderer().canContainAbsolutelyPositionedObjects())
    2934             containingBlockIsInsideOverflow = true;
    2935 
    2936         if (currLayer->hasCompositedScrollableOverflow())
    2937             return true;
    2938     }
    2939 
    2940     return false;
    2941 }
     2983        bool inContainingBlockChain = true;
     2984        if (containingBlockCanSkipLayers) {
     2985            inContainingBlockChain = currLayer->renderer().canContainAbsolutelyPositionedObjects();
     2986            if (inContainingBlockChain)
     2987                containingBlockCanSkipLayers = currLayer->renderer().isAbsolutelyPositioned();
     2988        }
     2989
     2990        if (currLayer->hasCompositedScrollableOverflow()) {
     2991            appendOverflowLayerNodeID(*currLayer);
     2992            break;
     2993        }
     2994    }
     2995}
     2996
    29422997
    29432998ScrollPositioningBehavior RenderLayerCompositor::computeCoordinatedPositioningForLayer(const RenderLayer& layer) const
    … …  
    29553010    if (!scrollingCoordinator)
    29563011        return ScrollPositioningBehavior::None;
     3012
     3013    auto* compositedAncestor = layer.ancestorCompositingLayer();
     3014    if (!compositedAncestor) {
     3015        ASSERT_NOT_REACHED();
     3016        return ScrollPositioningBehavior::None;
     3017    }
     3018
     3019    bool compositedAncestorIsInsideScroller = false;
     3020    auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, *compositedAncestor, compositedAncestorIsInsideScroller);
     3021    if (!scrollingAncestor) {
     3022        ASSERT_NOT_REACHED(); // layer.hasCompositedScrollingAncestor() should guarantee we have one.
     3023        return ScrollPositioningBehavior::None;
     3024    }
    29573025
    29583026    // There are two cases we have to deal with here:
    … …  
    29603028    //    composited (z-order) ancestor is inside the scroller or is the scroller. In this case, we have to compensate for scroll position
    29613029    //    changes to make the positioned layer stay in the same place. This only applies to position:absolute (since we handle fixed elsewhere).
    2962     auto* compositedAncestor = layer.ancestorCompositingLayer();
    2963 
    2964     auto& renderer = layer.renderer();
    2965     bool containingBlockCanSkipOverflow = renderer.isOutOfFlowPositioned() && renderer.style().position() == PositionType::Absolute;
    2966     if (containingBlockCanSkipOverflow) {
    2967         if (layerContainingBlockCrossesCoordinatedScrollingBoundary(layer, *compositedAncestor))
     3030    if (layer.renderer().isAbsolutelyPositioned()) {
     3031        if (compositedAncestorIsInsideScroller && isNonScrolledLayerInsideScrolledCompositedAncestor(layer, *compositedAncestor, *scrollingAncestor))
    29683032            return ScrollPositioningBehavior::Stationary;
    29693033    }
    … …  
    29723036    //    outside the overflow:scroll. In that case, we have to move the layer via the scrolling tree to make
    29733037    //    it move along with the overflow scrolling.
    2974     bool containingBlockIsInsideOverflow = !containingBlockCanSkipOverflow;
    2975     if (layerParentedAcrossCoordinatedScrollingBoundary(layer, *compositedAncestor, containingBlockCanSkipOverflow, containingBlockIsInsideOverflow) && containingBlockIsInsideOverflow)
     3038    if (!compositedAncestorIsInsideScroller && isScrolledByOverflowScrollLayer(layer, *scrollingAncestor))
    29763039        return ScrollPositioningBehavior::Moves;
    29773040
    … …  
    29993062    }
    30003063    case ScrollPositioningBehavior::Stationary: {
    3001         // Collect all the composited scrollers between this layer and its containing block.
    3002         ASSERT(layer.renderer().style().position() == PositionType::Absolute);
    3003         for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) {
    3004             if (currLayer->renderer().canContainAbsolutelyPositionedObjects())
    3005                 break;
    3006 
    3007             if (currLayer->hasCompositedScrollableOverflow()) {
    3008                 auto scrollingNodeID = currLayer->backing()->scrollingNodeIDForRole(ScrollCoordinationRole::Scrolling);
    3009                 if (scrollingNodeID)
    3010                     overflowNodeData.append(scrollingNodeID);
    3011                 else
    3012                     LOG(Scrolling, "Layer %p doesn't have scrolling node ID yet", &layer);
    3013             }
    3014         }
    3015         // Don't need to do anything because the layer is a descendant of the overflow in stacking.
     3064        ASSERT(layer.renderer().isAbsolutelyPositioned());
     3065        // Collect all the composited scrollers between this layer and its composited ancestor.
     3066        auto* compositedAncestor = layer.ancestorCompositingLayer();
     3067        if (!compositedAncestor)
     3068            return overflowNodeData;
     3069        collectStationaryLayerRelatedOverflowNodes(layer, *compositedAncestor, overflowNodeData);
    30163070        break;
    30173071    }
Note: See TracChangeset for help on using the changeset viewer.