Ensure filter and reflection outsets are applied on paint invalidation rect

third_party/WebKit/Source/core/style/ComputedStyle.cpp

 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2011 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
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
@@ skipping 517 matching lines @@
             return true;
 
         if (rareNonInheritedData->m_deprecatedFlexibleBox.get() != other.rareNonInheritedData->m_deprecatedFlexibleBox.get()
             && *rareNonInheritedData->m_deprecatedFlexibleBox.get() != *other.rareNonInheritedData->m_deprecatedFlexibleBox.get())
             return true;
 
         if (rareNonInheritedData->m_flexibleBox.get() != other.rareNonInheritedData->m_flexibleBox.get()
             && *rareNonInheritedData->m_flexibleBox.get() != *other.rareNonInheritedData->m_flexibleBox.get())
             return true;
 
-        if (!rareNonInheritedData->reflectionDataEquivalent(*other.rareNonInheritedData.get()))
-            return true;
-
         if (rareNonInheritedData->m_multiCol.get() != other.rareNonInheritedData->m_multiCol.get()
             && *rareNonInheritedData->m_multiCol.get() != *other.rareNonInheritedData->m_multiCol.get())
             return true;
 
         // If the counter directives change, trigger a relayout to re-calculate counter values and rebuild the counter node tree.
         const CounterDirectiveMap* mapA = rareNonInheritedData->m_counterDirectives.get();
         const CounterDirectiveMap* mapB = other.rareNonInheritedData->m_counterDirectives.get();
         if (!(mapA == mapB || (mapA && mapB && *mapA == *mapB)))
             return true;
 
@@ skipping 140 matching lines @@
     }
 
     if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
         if (rareNonInheritedData->m_alignContent != other.rareNonInheritedData->m_alignContent
             || rareNonInheritedData->m_alignItems != other.rareNonInheritedData->m_alignItems
             || rareNonInheritedData->m_alignSelf != other.rareNonInheritedData->m_alignSelf
             || rareNonInheritedData->m_justifyContent != other.rareNonInheritedData->m_justifyContent
             || rareNonInheritedData->m_justifyItems != other.rareNonInheritedData->m_justifyItems
             || rareNonInheritedData->m_justifySelf != other.rareNonInheritedData->m_justifySelf)
             return true;
+
+        if (!RuntimeEnabledFeatures::cssBoxReflectFilterEnabled() && !rareNonInheritedData->reflectionDataEquivalent(*other.rareNonInheritedData.get()))

[chrishtr, 2016/05/03 17:31:38]

Why is it no longer needed when cssBoxReflectFilterEnabled() is true?

Also, is the refactoring here required to fix a known bug?

[Xianzhu, 2016/05/03 18:00:24]

With cssBoxReflectFilterEnabled() we go through the filter path (new line 790).

The bug had not been known until I found it with the new test case for this CL
:)

[chrishtr, 2016/05/03 19:19:57]

Ok. But I still don't understand what was wrong with the old ComputedStyle code
that you are fixing.
Can you give an example? I can see why the change on 790 would be needed, but
what about the other parts?
Which test fails without this change?

[Xianzhu, 2016/05/03 19:53:39]

Previously when reflection changes, we mark the object needs repaint and needs
full layout (in old diffNeedsFullLayoutAndPaintInvalidation()). This is not
adequate because we also need to invalidate the non-composited sub-layers (which
requires needsPaintInvalidationLayer()). The changes at 699 (in
diffNeedsFullLayout()) and 720 (in diffNeedsPaintInvalidationLayer()) ensure
that when reflection changes, we will do full layout and full paint invalidation
of the sub-layers.

The new test fast/repaint/reflection-invalidation-positioned-child.html would
fail without these changes because of missing full paint invalidation of the
positioned child (a wrong incremental invalidation would be issued instead).

[chrishtr, 2016/05/03 20:04:37]

Ah, ok. I see. I had overlooked the difference between
needsPaintInvalidationLayer and needsPaintInvalidationObject. How about renaming
the former to something like
needsPaintInvalidationObjectAndSubtree?

[Xianzhu, 2016/05/03 20:37:42]

Good idea. Will create another CL for this.

+            return true;
     }
 
     return false;
 }
 
 bool ComputedStyle::diffNeedsPaintInvalidationLayer(const ComputedStyle& other) const
 {
     if (position() != StaticPosition && (visual->clip != other.visual->clip || visual->hasAutoClip != other.visual->hasAutoClip))
         return true;
 
     if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
         if (rareNonInheritedData->m_effectiveBlendMode != other.rareNonInheritedData->m_effectiveBlendMode
             || rareNonInheritedData->m_isolation != other.rareNonInheritedData->m_isolation)
             return true;
 
         if (rareNonInheritedData->m_mask != other.rareNonInheritedData->m_mask
             || rareNonInheritedData->m_maskBoxImage != other.rareNonInheritedData->m_maskBoxImage)
             return true;
+
+        if (!RuntimeEnabledFeatures::cssBoxReflectFilterEnabled() && !rareNonInheritedData->reflectionDataEquivalent(*other.rareNonInheritedData.get()))
+            return true;
     }
 
     return false;
 }
 
 bool ComputedStyle::diffNeedsPaintInvalidationObject(const ComputedStyle& other) const
 {
     if (!m_background->outline().visuallyEqual(other.m_background->outline()))
         return true;
 
@@ skipping 48 matching lines @@
 
         if (rareNonInheritedData->m_filter != other.rareNonInheritedData->m_filter)
             diff.setFilterChanged();
 
         if (!rareNonInheritedData->shadowDataEquivalent(*other.rareNonInheritedData.get()))
             diff.setNeedsRecomputeOverflow();
 
         if (rareNonInheritedData->m_backdropFilter != other.rareNonInheritedData->m_backdropFilter)
             diff.setBackdropFilterChanged();
 
-        if (RuntimeEnabledFeatures::cssBoxReflectFilterEnabled() && rareNonInheritedData->m_boxReflect != other.rareNonInheritedData->m_boxReflect)
+        if (RuntimeEnabledFeatures::cssBoxReflectFilterEnabled() && !rareNonInheritedData->reflectionDataEquivalent(*other.rareNonInheritedData.get()))
             diff.setFilterChanged();
     }
 
     if (!m_background->outline().visuallyEqual(other.m_background->outline()) || !surround->border.visualOverflowEqual(other.surround->border))
         diff.setNeedsRecomputeOverflow();
 
     if (!diff.needsPaintInvalidation()) {
         if (inherited->color != other.inherited->color
             || inherited->visitedLinkColor != other.inherited->visitedLinkColor
             || inherited_flags.m_textUnderline != other.inherited_flags.m_textUnderline
@@ skipping 1005 matching lines @@
     if (!shadowList)
         return false;
     for (size_t i = shadowList->shadows().size(); i--; ) {
         if (shadowList->shadows()[i].color().isCurrentColor())
             return true;
     }
     return false;
 }
 
 } // namespace blink