Revert changes is the use of CompositingReasonOutOfFlowClipping

third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp

Index: third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
index 51777eb381099c656b3338a2eac962f1aff9c8df..4de0e3d26e9cb3f77802a53d2f4148ee3084b1af 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
@@ -137,8 +137,13 @@ CompositingReasons CompositingReasonFinder::nonStyleDeterminedDirectReasons(cons
     CompositingReasons directReasons = CompositingReasonNone;
     LayoutObject* layoutObject = layer->layoutObject();
 
-    if (layer->needsCompositedScrolling())
+    bool checkForClipParent = m_compositingTriggers & OverflowScrollTrigger;
+    if (layer->needsCompositedScrolling()) {
+        checkForClipParent = true;
         directReasons |= CompositingReasonOverflowScrollingTouch;
+    }
+    if (checkForClipParent && layer->clipParent())

[flackr, 2016/09/29 14:21:20]

To restore the original logic we only need to check (m_compositingTriggers &
OverflowScrollTrigger) && layer->clipParent() right?

In order to handle the new clip promotion cases correctly you need to know if an
ancestor overflow scroller has been promoted, but I believe with line 142 you
are only setting checkForClipParent if the current layer is promoted.

[Stephen Chennney, 2016/09/30 17:12:25]

Still working on this, but ...

On 2016/09/29 14:21:20, flackr wrote:
The original logic would have made the clipParent check regardless of whether
you were scrolling or not, which means it did the check for composited scrollers
too. With the new path we will be compositing only the scrollers, but I presume
that because we needed it for all composited layers before, we now also need it
for the specific case of composited scrollers.
Do we have a call that checks for any composited ancestor scroller?

[flackr, 2016/09/30 20:13:16]

+vollick, I'm having a hard time putting together a test case, but as I
understand it, the clipParent check is intended to promote elements which need
to be stacked differently than the scrolling contents (i.e. not part of the same
stacking tree) but still clipped by the scroll. My concern with this approach is
that it seems with the CompositeOpaqueScrollers flag (i.e. OverflowScrollTrigger
is false, right?) we will only test whether overflow scrollers need to be
promoted due to a clip and only if they have already been promoted.
You should be able to check if it has a layer->ancestorScrollingLayer and if
that layer has been promoted.

[Ian Vollick, 2016/10/03 21:09:15]

Some background on clip parent (because it's confusing). You get a clip parent
if you're positioned below a clipping layer in the composited layer tree, but
you don't actually want to inherit that clip. Consider a fixed-pos element
within a non-cb-inducing overflow scroller. Even if the fixed-pos element's
composited layer is below the scroll clip, it needs to ignore it, so we assign a
clipParent to that layer to indicate that we should ignore all clips up to the
clip parent (or, put another way, that the layer is only clipped by stuff from
the clipParent up).

So I'm a bit confused by the code requiring that the layer needs to be a
composited scroller to get a promoted due to having a clipParent.

+        directReasons |= CompositingReasonOutOfFlowClipping;
 
     // Composite |layer| if it is inside of an ancestor scrolling layer, but that
     // scrolling layer is not on the stacking context ancestor chain of |layer|.