Fix visual rect for background box painting in composited scrollers.

third_party/WebKit/Source/core/paint/BoxPainter.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/paint/BoxPainter.h"
 
 #include "core/HTMLNames.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/layout/ImageQualityController.h"
@@ skipping 91 matching lines @@
 {
     bool paintingOverflowContents = isPaintingBackgroundOfPaintContainerIntoScrollingContentsLayer(&m_layoutBox, paintInfo);
     const ComputedStyle& style = m_layoutBox.styleRef();
 
     // FIXME: For now we don't have notification on media buffered range change from media player
     // and miss paint invalidation on buffered range change. crbug.com/484288.
     Optional<DisplayItemCacheSkipper> cacheSkipper;
     if (style.appearance() == MediaSliderPart)
         cacheSkipper.emplace(paintInfo.context);
 
-    if (LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, m_layoutBox, DisplayItem::BoxDecorationBackground))
+    const DisplayItemClient& displayItemClient = paintingOverflowContents ? static_cast<const DisplayItemClient&>(*m_layoutBox.layer()->compositedLayerMapping()->scrollingContentsLayer()) : m_layoutBox;

[Xianzhu, 2016/08/17 23:28:48]

Can you make sure if paint invalidation work for this change? For example, when
m_layoutBox's background changes?

[wkorman, 2016/08/18 17:26:23]

Hmm, it does not, and it does at ToT. Paint flashing does look like it's
invalidating the right areas and logging doesn't seem to show we're using a
cached item improperly.

Also, the paint flashing for the composited scroller is now the full 550px
height of the graphics layer whereas at ToT it's only the ~200px height of the
visible area, though I'm not sure this is a big deal as things probably clip.

Looking further but if you have thoughts on cause or alternate approach, do
tell!

[wkorman, 2016/08/18 18:02:02]

Invalidation not working is specific to 'local' being set in 'background' style.
If one only specifies a color and leaves attachment as default, invalidation
works. So, perhaps related to flackr@ work in http://crrev.com/2196583002.

[flackr, 2016/08/18 19:03:39]

Invalidations were supposed to be fixed by https://cr-rev.appspot.com/411441
which adds a test to verify it invalidates the entire region of the composited
overflow scroller.

[wkorman, 2016/08/18 19:46:00]

OK, that change is helpful re: involved code, thanks. The added test
paint/invalidation/composited-overflow-with-local-background.html passes with a
release build with this patch but is visually incorrect. The scroller content
remains red when it should have been repainted as green. The text expectations
must match however, and the repaint test doesn't include pixel expectations,
which we may want to change.

That test does fail on the trybots or debug build with under-invalidation check
enabled, see sample output:

https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r...

So, like, probably we should actually look into the details of that
under-invalidation. :) Proceeding...

[wkorman, 2016/08/18 21:16:59]

So it looks like we're invalidating the correct rect on correct layer via
setScrollingContentsNeedDisplayInRect() which ends up in
GraphicsLayer::setNeedsDisplayInRect() and is in fact calling
m_layer->layer()->invalidateRect(rect).

But we're failing under-invalidation, excerpt as:

[1:1:0818/141139:1395034311807:ERROR:PaintController.cpp(526)] New display item:
"{client: \"0x7d549542d20 Scrolling Contents Layer\", type:
\"DrawingBoxDecorationBackground\", rect: [0.000000,0.000000
185.000000x300.000000]}"
[1:1:0818/141139:1395034311975:ERROR:PaintController.cpp(527)] Old display item:
"{client: \"0x7d549542d20 Scrolling Contents Layer\", type:
\"DrawingBoxDecorationBackground\", rect: [0.000000,0.000000
185.000000x300.000000]}"

and the picture bitmaps don't match due to pixel color change -- the old pixels
are red and the new are green, which is what we would expect since we've changed
the background color.

I'm not sure how rect invalidation interacts with object/layer invalidation, so
am still researching.

+    if (DrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, displayItemClient, DisplayItem::BoxDecorationBackground))
         return;
 
-    LayoutObjectDrawingRecorder recorder(paintInfo.context, m_layoutBox, DisplayItem::BoxDecorationBackground, boundsForDrawingRecorder(paintInfo, paintOffset));
-
+    DrawingRecorder recorder(paintInfo.context, displayItemClient, DisplayItem::BoxDecorationBackground, FloatRect(boundsForDrawingRecorder(paintInfo, paintOffset)));
     BoxDecorationData boxDecorationData(m_layoutBox);
     GraphicsContextStateSaver stateSaver(paintInfo.context, false);
 
     if (!paintingOverflowContents) {
         // FIXME: Should eventually give the theme control over whether the box shadow should paint, since controls could have
         // custom shadows of their own.
         if (!m_layoutBox.boxShadowShouldBeAppliedToBackground(boxDecorationData.bleedAvoidance)) {
             paintBoxShadow(paintInfo, paintRect, style, Normal);
         }
 
@@ skipping 735 matching lines @@
     }
 }
 
 bool BoxPainter::shouldForceWhiteBackgroundForPrintEconomy(const ComputedStyle& style, const Document& document)
 {
     return document.printing() && style.getPrintColorAdjust() == PrintColorAdjustEconomy
         && (!document.settings() || !document.settings()->shouldPrintBackgrounds());
 }
 
 } // namespace blink