Fix Compositing of Opaque Scrolling Layers and Add Tests

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 124 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 (hasOverflowScrollTrigger()) {
+    if (hasOverflowScrollTrigger() || RuntimeEnabledFeatures::compositeOpaqueScrollersEnabled()) {

[flackr, 2016/08/30 21:14:49]

You mentioned that hasOverflowScrollTrigger() basically proxies whether we
prefer compositing to lcd text, but it seems like layerNeedsCompositedScrolling
in PaintLayerScrollableArea.cpp already includes whether prefer compositing to
lcd text. Assuming my following comment is not a problem, this condition can
probably be removed altogether.

[Stephen Chennney, 2016/08/31 20:50:20]

I don't think we can remove it for the scrolling case, but adjusting logic will
clean it up and prevent confusion as to what applies where.

         if (layer->clipParent())

[flackr, 2016/08/30 21:14:50]

I'm concerned this condition combined with
RuntimeEnabledFeatures::compositeOpaqueScrollersEnabled() could cause us to add
a compositing reason to a scroller when it doesn't have an opaque background. We
should test this.

[Stephen Chennney, 2016/08/31 20:50:20]

I think I'll just adjust the logic.

[flackr, 2016/09/01 21:06:46]

Sorry for the delay, I mistook this to mean you were going to have another
comment later when this was done.

             directReasons |= CompositingReasonOutOfFlowClipping;
 
         if (layer->needsCompositedScrolling())
             directReasons |= CompositingReasonOverflowScrollingTouch;
     }
 
     // Composite |layer| if it is inside of an ancestor scrolling layer, but that
     // scrolling layer is not 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()) {
@@ skipping 21 matching lines @@
 bool CompositingReasonFinder::requiresCompositingForPositionFixed(const PaintLayer* layer) const
 {
     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.
     return layer->scrollsWithViewport() && m_layoutView.frameView()->isScrollable();
 }
 
 } // namespace blink