Revert changes is the use of CompositingReasonOutOfFlowClipping

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/layout/compositing/CompositingReasonFinder.h"
 
 #include "core/CSSPropertyNames.h"
 #include "core/dom/Document.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/Settings.h"
@@ skipping 119 matching lines @@
     // Note that we ask the layoutObject if it has a transform, because the style may have transforms,
     // but the layoutObject may be an inline that doesn't support them.
     return layoutObject->hasTransformRelatedProperty() && layoutObject->style()->has3DTransform();
 }
 
 CompositingReasons CompositingReasonFinder::nonStyleDeterminedDirectReasons(const PaintLayer* layer) const
 {
     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|.
     // See the definition of the scrollParent property in Layer for more detail.
     if (const PaintLayer* scrollingAncestor = layer->ancestorScrollingLayer()) {
         if (scrollingAncestor->needsCompositedScrolling() && layer->scrollParent())
             directReasons |= CompositingReasonOverflowScrollingParent;
     }
 
     // TODO(flackr): Rename functions and variables to include sticky position (i.e. ScrollDependentPosition rather than PositionFixed).
@@ skipping 19 matching lines @@
     if (!(m_compositingTriggers & ViewportConstrainedPositionedTrigger))
         return false;
     // Don't promote fixed position elements that are descendants of a non-view container, e.g. transformed elements.
     // They will stay fixed wrt the container rather than the enclosing frame.
     if (layer->scrollsWithViewport())
         return m_layoutView.frameView()->isScrollable();
     return layer->layoutObject()->style()->position() == StickyPosition && layer->ancestorOverflowLayer()->scrollsOverflow();
 }
 
 } // namespace blink