Fix Compositing of Opaque Scrolling Layers and Add Tests

third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@gmail.com>
  *   Randall Jesup <rjesup@wgate.com>
@@ skipping 1484 matching lines @@
         return false;
 
     Node* node = layer->enclosingNode();
     if (node && node->isElementNode() && (toElement(node)->compositorMutableProperties() & (CompositorMutableProperty::kScrollTop | CompositorMutableProperty::kScrollLeft)))
         return true;
 
     // TODO(schenney) The color test alone is inadequate. When https://codereview.chromium.org/2196583002 lands
     // we should use PaintLayer::shouldPaintBackgroundOntoForeground() because we will not still get
     // LCD text unless the conditions there are met. It also unifies logic for scrolling compositing decisions.
     bool backgroundSupportsLCDText = RuntimeEnabledFeatures::compositeOpaqueScrollersEnabled()
-        && !layer->layoutObject()->style()->visitedDependentColor(CSSPropertyBackgroundColor).hasAlpha();
+        && layer->shouldPaintBackgroundOntoScrollingContentsLayer()

[Stephen Chennney, 2016/08/26 18:01:41]

My understanding is that here we cannot use the CompositedLayerMapping flag
because it will not have been updated yet.

[flackr, 2016/08/26 18:24:23]

I believe that's correct - but should be easy to test.

Have you also verified that we have the up to date value for not having negative
z index children? See !m_owningLayer.stackingNode()->hasNegativeZOrderList() at
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/co...

[Stephen Chennney, 2016/08/26 18:42:08]

I'll check.

+        && layer->backgroundIsKnownToBeOpaqueInRect(toLayoutBox(layer->layoutObject())->paddingBoxRect());

[Stephen Chennney, 2016/08/26 20:46:01]

We have a chicken and egg problem. The first thing required in
backgroundIsKnownToBeOpaqueInRect is that the layer is self painting, but the
code to determine self painting status calls this method to check if we need
overflow scrolling, so we never set isSelfPaintingLayer.

So here we need something else that is not concerned about what the layer is
painting into but is only concerned with the background itself.

Interestingly, in the unit test this manages to work because the layer is self
painting, and I have no idea why yet.

     if (mode == PaintLayerScrollableArea::ConsiderLCDText
         && !layer->compositor()->preferCompositingToLCDTextEnabled()
         && !backgroundSupportsLCDText)
         return false;
 
     // TODO(schenney) Tests fail if we do not also exclude layer->layoutObject()->style()->hasBorderDecoration()
     // (missing background behind dashed borders). Resolve this case, or not, and update this check with
     // the results.
     return !(layer->size().isEmpty()
         || layer->hasDescendantWithClipPath()
@@ skipping 256 matching lines @@
 
 void PaintLayerScrollableArea::DelayScrollPositionClampScope::clampScrollableAreas()
 {
     for (auto& scrollableArea : *s_needsClamp)
         scrollableArea->clampScrollPositionsAfterLayout();
     delete s_needsClamp;
     s_needsClamp = nullptr;
 }
 
 } // namespace blink