Fix paint property under-invalidation checking about reference filters

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

 // Copyright 2015 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/PaintPropertyTreeBuilder.h"
 
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutInline.h"
@@ skipping 364 matching lines @@
   // TODO(trchen): Can't omit effect node if we have 3D children.
   // TODO(trchen): Can't omit effect node if we have blending children.
   if (object.needsPaintPropertyUpdate() || context.forceSubtreeUpdate) {
     bool effectNodeNeeded = false;
 
     float opacity = style.opacity();
     if (opacity != 1.0f)
       effectNodeNeeded = true;
 
     CompositorFilterOperations filter;
+#if DCHECK_IS_ON()
+    FilterOperations styleFilterOperations;
+#endif
     if (object.isSVG() && !object.isSVGRoot()) {
       // TODO(trchen): SVG caches filters in SVGResources. Implement it.
     } else if (PaintLayer* layer = toLayoutBoxModelObject(object).layer()) {
       // TODO(trchen): Eliminate PaintLayer dependency.
+#if DCHECK_IS_ON()
+      filter = layer->createCompositorFilterOperationsForFilter(
+          style, &styleFilterOperations);
+#else
       filter = layer->createCompositorFilterOperationsForFilter(style);
+#endif
     }
 
     const ClipPaintPropertyNode* outputClip = context.inputClipOfCurrentEffect;
     // The CSS filter spec didn't specify how filters interact with overflow
     // clips. The implementation here mimics the old Blink/WebKit behavior for
     // backward compatibility.
     // Basically the output of the filter will be affected by clips that applies
     // to the current element. The descendants that paints into the input of the
     // filter ignores any clips collected so far. For example:
     // <div style="overflow:scroll">
@@ skipping 15 matching lines @@
       // TODO(trchen): A filter may contain spatial operations such that an
       // output pixel may depend on an input pixel outside of the output clip.
       // We should generate a special clip node to represent this expansion.
     }
 
     if (effectNodeNeeded) {
       auto& properties = object.getMutableForPainting().ensurePaintProperties();
       context.forceSubtreeUpdate |= properties.updateEffect(
           context.currentEffect, context.current.transform, outputClip,
           std::move(filter), opacity);
+#if DCHECK_IS_ON()
+      properties.setStyleFilterOperations(styleFilterOperations);

[pdr., 2016/12/08 02:00:41]

How much additional value will these checks give us? Looking at
ReferenceFilterOperation::operator==, I think we just check the style
(style="filter: url(#cat)" != style="filter: url('#dog')") but not the actual
value, such as if the #dog element's children change.

equalsIgnoringReferenceFilters already catches many cases such as when the total
number of operations change. WDYT of just relying on
equalsIgnoringReferenceFilters and not doing the extra checks of
styleFilterOperations? This will mean we fail to catch some cases, but it lets
us avoid adding a new pattern to FindObjectPropertiesNeedingUpdateScope and new
members to ObjectPaintProperties.

[Xianzhu, 2016/12/08 17:34:35]

You are right. The little additional value is not worth the complexity.
Done.

+#endif
     } else {
       if (auto* properties = object.getMutableForPainting().paintProperties())
         context.forceSubtreeUpdate |= properties->clearEffect();
     }
   }
 
   const auto* properties = object.paintProperties();
   if (properties && properties->effect()) {
     context.currentEffect = properties->effect();
     if (!properties->effect()->filter().isEmpty()) {
@@ skipping 472 matching lines @@
 #endif
 
   updateOverflowClip(object, context);
   updatePerspective(object, context);
   updateSvgLocalToBorderBoxTransform(object, context);
   updateScrollAndScrollTranslation(object, context);
   updateOutOfFlowContext(object, context);
 }
 
 }  // namespace blink