Fix gradient background invalidation when HTML size changes

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

 // Copyright 2016 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/BoxPaintInvalidator.h"
 
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/compositing/CompositedLayerMapping.h"
 #include "core/paint/ObjectPaintInvalidator.h"
@@ skipping 294 matching lines @@
     area->invalidatePaintOfScrollControlsIfNeeded(m_context);
 
   // This is for the next invalidatePaintIfNeeded so must be at the end.
   savePreviousBoxGeometriesIfNeeded();
 
   return reason;
 }
 
 bool BoxPaintInvalidator::needsToSavePreviousBoxGeometries() {
   LayoutSize paintInvalidationSize = m_context.newVisualRect.size();
-  // Don't save old box Geometries if the paint rect is empty because we'll
-  // full invalidate once the paint rect becomes non-empty.
-  if (paintInvalidationSize.isEmpty())
-    return false;
 
-  if (m_box.paintedOutputOfObjectHasNoEffectRegardlessOfSize())
-    return false;
+  // The shortcuts doesn't apply to HTML element. ViewPaintInvalidator needs to
+  // know its previous border box size.

[chrishtr, 2017/02/16 21:11:14]

Even if empty? I wonder in which case.

[Xianzhu, 2017/02/16 22:01:58]

The size of HTML element affects viewport background even if the HTML element
has no painted output (e.g. it has visibility:hidden which causes empty
paintInvalidationSize).

If both paintInvalidationSize and borderSize are empty, we'll still return false
at the end of this function.

Updated comments.

+  if (!m_box.node() || !m_box.node()->isHTMLElement()) {
+    // Don't save old box Geometries if the paint rect is empty because we'll
+    // full invalidate once the paint rect becomes non-empty.

[chrishtr, 2017/02/16 21:11:14]

drive-by nit: s/full/fully/. also "geometries" instead of "Geometries"

[Xianzhu, 2017/02/16 22:01:58]

Done.

+    if (paintInvalidationSize.isEmpty())
+      return false;
+
+    if (m_box.paintedOutputOfObjectHasNoEffectRegardlessOfSize())
+      return false;
+  }
 
   const ComputedStyle& style = m_box.styleRef();
 
   // If we use border-box sizing we need to track changes in the size of the
   // content box.
   if (style.boxSizing() == EBoxSizing::kBorderBox)
     return true;
 
   // No need to save old border box size if we can use size of the old paint
   // rect as the old border box size in the next invalidation.
@@ skipping 21 matching lines @@
     }
     return;
   }
 
   PreviousBoxGeometries geometries = {m_box.size(), m_box.contentBoxRect(),
                                       m_box.layoutOverflowRect()};
   previousBoxGeometriesMap().set(&m_box, geometries);
   m_box.getMutableForPainting().setHasPreviousBoxGeometries(true);
 }
 
+static LayoutSize previousBorderBoxSizeInternal(const LayoutBox& box,
+                                                const LayoutSize& defaultSize) {
+  DCHECK(box.hasPreviousBoxGeometries() ==
+         previousBoxGeometriesMap().contains(&box));
+  if (box.hasPreviousBoxGeometries())
+    return previousBoxGeometriesMap().get(&box).borderBoxSize;
+  return defaultSize;
+}
+
+LayoutSize BoxPaintInvalidator::previousBorderBoxSize(const LayoutBox& box) {
+  return previousBorderBoxSizeInternal(box, box.visualRect().size());
+}
+
 LayoutSize BoxPaintInvalidator::previousBorderBoxSize() {
-  DCHECK(m_box.hasPreviousBoxGeometries() ==
-         previousBoxGeometriesMap().contains(&m_box));
-  if (m_box.hasPreviousBoxGeometries())
-    return previousBoxGeometriesMap().get(&m_box).borderBoxSize;
-
-  // We didn't save the old border box size because it was the same as the size
-  // of oldVisualRect.
-  return m_context.oldVisualRect.size();
+  return previousBorderBoxSizeInternal(m_box, m_context.oldVisualRect.size());

[chrishtr, 2017/02/16 21:11:14]

Wh oldVisualRect() here but visualRect() in the other function variant?

[Xianzhu, 2017/02/16 22:01:58]

Added comments:
// This is used during paint invalidation of the m_box. PaintInvalidator has
// already updated m_box.visualRect() to the current value, so we need to use
// context.oldVisualRect as the default value.

 }
 
 LayoutRect BoxPaintInvalidator::previousContentBoxRect() {
   DCHECK(m_box.hasPreviousBoxGeometries() ==
          previousBoxGeometriesMap().contains(&m_box));
   return m_box.hasPreviousBoxGeometries()
              ? previousBoxGeometriesMap().get(&m_box).contentBoxRect
              : LayoutRect();
 }
 
 LayoutRect BoxPaintInvalidator::previousLayoutOverflowRect() {
   DCHECK(m_box.hasPreviousBoxGeometries() ==
          previousBoxGeometriesMap().contains(&m_box));
   return m_box.hasPreviousBoxGeometries()
              ? previousBoxGeometriesMap().get(&m_box).layoutOverflowRect
              : LayoutRect();
 }
 
 }  // namespace blink