Tracking filter mutation via SVGElementProxy

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

Index: third_party/WebKit/Source/core/paint/PaintLayer.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.cpp b/third_party/WebKit/Source/core/paint/PaintLayer.cpp
index 01825db216bfcbc45fe7b06d16d95043328839c5..0e984abecd70e4a1ac964439f87746506300dd0c 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayer.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayer.cpp
@@ -188,8 +188,10 @@ PaintLayer::PaintLayer(LayoutBoxModelObject* layoutObject)
 }
 
 PaintLayer::~PaintLayer() {
-  if (m_rareData && m_rareData->filterInfo)
+  if (m_rareData && m_rareData->filterInfo) {
+    layoutObject()->styleRef().filter().removeClient(m_rareData->filterInfo);
     m_rareData->filterInfo->clearLayer();
+  }
   if (layoutObject()->frame() && layoutObject()->frame()->page()) {
     if (ScrollingCoordinator* scrollingCoordinator =
             layoutObject()->frame()->page()->scrollingCoordinator())
@@ -2651,7 +2653,8 @@ void PaintLayer::ensureCompositedLayerMapping() {
   m_rareData->compositedLayerMapping->setNeedsGraphicsLayerUpdate(
       GraphicsLayerUpdateSubtree);
 
-  updateOrRemoveFilterEffect();
+  if (PaintLayerFilterInfo* filterInfo = this->filterInfo())
+    filterInfo->invalidateFilterChain();
 }
 
 void PaintLayer::clearCompositedLayerMapping(bool layerBeingDestroyed) {
@@ -2669,8 +2672,11 @@ void PaintLayer::clearCompositedLayerMapping(bool layerBeingDestroyed) {
   if (m_rareData)
     m_rareData->compositedLayerMapping.reset();
 
-  if (!layerBeingDestroyed)
-    updateOrRemoveFilterEffect();
+  if (layerBeingDestroyed)
+    return;
+
+  if (PaintLayerFilterInfo* filterInfo = this->filterInfo())
+    filterInfo->invalidateFilterChain();
 }
 
 void PaintLayer::setGroupedMapping(CompositedLayerMapping* groupedMapping,
@@ -2865,9 +2871,17 @@ void PaintLayer::updateFilters(const ComputedStyle* oldStyle,
   if (!newStyle.hasFilterInducingProperty() &&
       (!oldStyle || !oldStyle->hasFilterInducingProperty()))
     return;
-
-  updateOrRemoveFilterClients();
-  updateOrRemoveFilterEffect();
+  const bool hadFilterInfo = filterInfo();
+  if (newStyle.hasFilterInducingProperty())
+    newStyle.filter().addClient(&ensureFilterInfo());
+  if (hadFilterInfo && oldStyle)
+    oldStyle->filter().removeClient(filterInfo());
+  if (!newStyle.hasFilterInducingProperty()) {
+    removeFilterInfo();
+    return;
+  }
+  if (PaintLayerFilterInfo* filterInfo = this->filterInfo())
+    filterInfo->invalidateFilterChain();
 }
 
 bool PaintLayer::attemptDirectCompositingUpdate(StyleDifference diff,
@@ -2891,14 +2905,6 @@ bool PaintLayer::attemptDirectCompositingUpdate(StyleDifference diff,
   if (potentialCompositingReasonsFromStyle() !=
       oldPotentialCompositingReasonsFromStyle)
     return false;
-  // If we're unwinding a scheduleSVGFilterLayerUpdateHack(), then we can't
-  // perform a direct compositing update because the filters code is going
-  // to produce different output this time around. We can remove this code
-  // once we fix the chicken/egg bugs in the filters code and delete the
-  // scheduleSVGFilterLayerUpdateHack().
-  if (layoutObject()->node() &&
-      layoutObject()->node()->svgFilterNeedsLayerUpdate())
-    return false;
   if (!m_rareData || !m_rareData->compositedLayerMapping)
     return false;
 
@@ -3012,6 +3018,13 @@ PaintLayerFilterInfo& PaintLayer::ensureFilterInfo() {
   return *rareData.filterInfo;
 }
 
+void PaintLayer::removeFilterInfo() {
+  if (!m_rareData || !m_rareData->filterInfo)
+    return;
+  m_rareData->filterInfo->clearLayer();
+  m_rareData->filterInfo = nullptr;
+}
+
 void PaintLayer::removeAncestorOverflowLayer(const PaintLayer* removedLayer) {
   // If the current ancestor overflow layer does not match the removed layer
   // the ancestor overflow layer has changed so we can stop searching.
@@ -3034,27 +3047,11 @@ void PaintLayer::removeAncestorOverflowLayer(const PaintLayer* removedLayer) {
   }
 }
 
-void PaintLayer::updateOrRemoveFilterClients() {
-  const auto& filter = layoutObject()->style()->filter();
-  if (filter.isEmpty() && m_rareData && m_rareData->filterInfo) {
-    m_rareData->filterInfo->clearLayer();
-    m_rareData->filterInfo = nullptr;
-  } else if (filter.hasReferenceFilter()) {
-    ensureFilterInfo().updateReferenceFilterClients(filter);
-  } else if (filterInfo()) {
-    filterInfo()->clearFilterReferences();
-  }
-}
-
-FilterEffect* PaintLayer::updateFilterEffect() const {
+FilterEffect* PaintLayer::lastFilterEffect() const {
   // TODO(chrishtr): ensure (and assert) that compositing is clean here.
-
   if (!paintsWithFilters())
     return nullptr;
-
   PaintLayerFilterInfo* filterInfo = this->filterInfo();
-
-  // Should have been added by updateOrRemoveFilterEffect().
   DCHECK(filterInfo);
 
   if (filterInfo->lastEffect())
@@ -3071,16 +3068,12 @@ FilterEffect* PaintLayer::updateFilterEffect() const {
   return filterInfo->lastEffect();
 }
 
-FilterEffect* PaintLayer::lastFilterEffect() const {
-  return updateFilterEffect();
-}
-
 FloatRect PaintLayer::mapRectForFilter(const FloatRect& rect) const {
   if (!hasFilterThatMovesPixels())
     return rect;
 
   // Ensure the filter-chain is refreshed wrt reference filters.
-  updateFilterEffect();
+  lastFilterEffect();

[pdr., 2016/11/02 06:35:22]

I agree with your and Elliott's review comments about this being a little weird.
Your idea of calling mapRect on the FilterEffect sounded good to me. WDYT of
adding a TODO above lastFilterEffect to clean this up?

[fs, 2016/11/02 15:49:22]

After a somewhat related discussion with senorblanco, I have some other ideas
for how this could end up looking. Adding a general TODO (like above.)

 
   FilterOperations filterOperations =
       addReflectionToFilterOperations(layoutObject()->styleRef());
@@ -3104,37 +3097,6 @@ bool PaintLayer::hasFilterThatMovesPixels() const {
   return false;
 }
 
-void PaintLayer::updateOrRemoveFilterEffect() {
-  // FilterEffectBuilder is only used to render the filters in software mode,
-  // so we always need to run updateOrRemoveFilterEffect after the composited
-  // mode might have changed for this layer.
-  if (!paintsWithFilters()) {
-    if (PaintLayerFilterInfo* filterInfo = this->filterInfo())
-      filterInfo->setLastEffect(nullptr);
-    return;
-  }
-
-  ensureFilterInfo().setLastEffect(nullptr);
-}
-
-void PaintLayer::filterNeedsPaintInvalidation() {
-  {
-    DeprecatedScheduleStyleRecalcDuringLayout marker(
-        layoutObject()->document().lifecycle());
-    // It's possible for scheduleSVGFilterLayerUpdateHack to schedule a style
-    // recalc, which is a problem because this function can be called right
-    // before performing layout but after style recalc.
-    //
-    // See LayoutView::layout() and the call to
-    // invalidateSVGRootsWithRelativeLengthDescendents(). This violation is
-    // worked around in FrameView::updateStyleAndLayoutIfNeededRecursive() by
-    // doing an extra style recalc and layout in case it's needed.
-    toElement(layoutObject()->node())->scheduleSVGFilterLayerUpdateHack();
-  }
-
-  layoutObject()->setShouldDoFullPaintInvalidation();
-}
-
 void PaintLayer::addLayerHitTestRects(LayerHitTestRects& rects) const {
   computeSelfHitTestRects(rects);
   for (PaintLayer* child = firstChild(); child; child = child->nextSibling())