Don't invalidate paint on background image change if it's obscured

third_party/WebKit/Source/core/layout/LayoutBox.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 1466 matching lines @@
 {
     BoxPainter(*this).paintMask(paintInfo, paintOffset);
 }
 
 void LayoutBox::imageChanged(WrappedImagePtr image, const IntRect*)
 {
     // TODO(chrishtr): support PaintInvalidationDelayedFull for animated border images.
     if ((style()->borderImage().image() && style()->borderImage().image()->data() == image)
         || (style()->maskBoxImage().image() && style()->maskBoxImage().image()->data() == image)) {
         setShouldDoFullPaintInvalidation();
-        return;
+    } else if (!invalidatePaintOfImageLayers(image, style()->backgroundLayers())) {
+        invalidatePaintOfImageLayers(image, style()->maskLayers());
     }
 
     ShapeValue* shapeOutsideValue = style()->shapeOutside();

[chrishtr, 2016/08/09 16:05:17]

I guess you're fixing a bug also that this code didn't use to execute? What was
the effect of this? Add
a test?

[Xianzhu, 2016/08/09 18:51:48]

There might be a bug if background and shape-outside share the same image that
we would miss needed layout when shape-outside image changes. However this can't
be reproduced because we seem to always use different images for background and
shape-outside, perhaps because shape-outside's image request has different
referrer rule.

However, the change makes the code not depend on the special referrer rule.

     if (!frameView()->isInPerformLayout() && isFloating() && shapeOutsideValue && shapeOutsideValue->image() && shapeOutsideValue->image()->data() == image) {
         ShapeOutsideInfo& info = ShapeOutsideInfo::ensureInfo(*this);
         if (!info.isComputingShape()) {
             info.markShapeAsDirty();
             markShapeOutsideDependentsForLayout();
         }
     }
-
-    if (!invalidatePaintOfLayerRectsForImage(image, style()->backgroundLayers(), true))
-        invalidatePaintOfLayerRectsForImage(image, style()->maskLayers(), false);
 }
 
 ResourcePriority LayoutBox::computeResourcePriority() const
 {
     LayoutRect viewBounds = viewRect();
     LayoutRect objectBounds = LayoutRect(absoluteContentBox());
 
     // The object bounds might be empty right now, so intersects will fail since it doesn't deal
     // with empty rects. Use LayoutRect::contains in that case.
     bool isVisible;
     if (!objectBounds.isEmpty())
         isVisible =  viewBounds.intersects(objectBounds);
     else
         isVisible = viewBounds.contains(objectBounds);
 
     LayoutRect screenRect;
     if (!objectBounds.isEmpty()) {
         screenRect = viewBounds;
         screenRect.intersect(objectBounds);
     }
 
     int screenArea = 0;
     if (!screenRect.isEmpty() && isVisible)
         screenArea = static_cast<uint32_t>(screenRect.width() * screenRect.height());
     return ResourcePriority(isVisible ? ResourcePriority::Visible : ResourcePriority::NotVisible, screenArea);
 }
 
-bool LayoutBox::invalidatePaintOfLayerRectsForImage(WrappedImagePtr image, const FillLayer& layers, bool drawingBackground)
+bool LayoutBox::invalidatePaintOfImageLayers(WrappedImagePtr image, const FillLayer& layers)
 {
-    if (drawingBackground && (isDocumentElement() || backgroundStolenForBeingBody()))
+    bool isBackground = &layers == &styleRef().backgroundLayers();

[eae, 2016/08/09 14:41:48]

NIT: Comparing references like this is a little odd; how about adding a ==
operator to the FillLayers calls?

[Xianzhu, 2016/08/09 18:51:48]

We have FillLayer::operator== that compares the values which is not desired
here.  

+    if (isBackground && (isDocumentElement() || backgroundStolenForBeingBody()))

[Stephen Chennney, 2016/08/09 14:22:31]

if (isBackground) {
  if (isDocumentElement() || ...
    return false;

  invalidateBackgroundObscurationStatus();
  ...
}

[Xianzhu, 2016/08/09 18:51:48]

Done.

         return false;
+
+    if (isBackground) {
+        invalidateBackgroundObscurationStatus();
+        if (backgroundIsKnownToBeObscured())
+            return false;
+    }
+
     for (const FillLayer* curLayer = &layers; curLayer; curLayer = curLayer->next()) {
         if (curLayer->image() && image == curLayer->image()->data()) {
             bool maybeAnimated = curLayer->image()->cachedImage() && curLayer->image()->cachedImage()->getImage() && curLayer->image()->cachedImage()->getImage()->maybeAnimated();
-            if (maybeAnimated && drawingBackground)
+            if (maybeAnimated && isBackground) {
                 setShouldDoFullPaintInvalidation(PaintInvalidationDelayedFull);
-            else
+            } else {
                 setShouldDoFullPaintInvalidation();
-
-            if (drawingBackground)
-                invalidateBackgroundObscurationStatus();
-            return true;
+                return true;
+            }
         }
     }
     return false;
 }
 
 bool LayoutBox::intersectsVisibleViewport()
 {
     LayoutRect rect = visualOverflowRect();
     LayoutView* layoutView = view();
     while (layoutView->frame()->ownerLayoutObject())
@@ skipping 3434 matching lines @@
         m_rareData->m_snapAreas->remove(&snapArea);
     }
 }
 
 SnapAreaSet* LayoutBox::snapAreas() const
 {
     return m_rareData ? m_rareData->m_snapAreas.get() : nullptr;
 }
 
 } // namespace blink