Refactor SVGResourcesCache::ClientLayoutChanged

This moves the code from said method into a new helper class named
SVGResourceInvalidator, streamlining it and splitting invalidation into
'effects' and 'paints'.
At the callsites, make the invalidation calls conditional on the
reference box (bounds) changing rather than layout dirty flags. We can
do this because we no longer have an implicit dependency between effect
bounds and the visual rect of the LayoutObject.

Also use this helper in RemoveFromCacheAndInvalidateDependencies() since
it matches the pattern used there (but add a comment about it possibly
being unnecessary).

Bug: 1028061, 1028063
Change-Id: Ic46481d925c6ac73e9eae4f8a9afc2ddd31c093c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2449995Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#814591}

third_party/blink/renderer/core/layout/svg/layout_svg_container.cc

@@ -69,9 +69,9 @@ void LayoutSVGContainer::UpdateLayout() {
                                    did_screen_scale_factor_change_,
                                    layout_size_changed);
 
-  // Invalidate all resources of this client if our layout changed.
-  if (EverHadLayout() && NeedsLayout())
-    SVGResourcesCache::ClientLayoutChanged(*this);
+  // Invalidate all resources of this client if our reference box changed.
+  if (EverHadLayout() && (SelfNeedsLayout() || needs_boundaries_update_))
+    SVGResourceInvalidator(*this).InvalidateEffects();
 
   if (needs_boundaries_update_ ||
       transform_change != SVGTransformChange::kNone) {

third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc

@@ -23,7 +23,7 @@
 
 #include "third_party/blink/renderer/core/layout/hit_test_result.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
-#include "third_party/blink/renderer/core/layout/svg/svg_resources_cache.h"
+#include "third_party/blink/renderer/core/layout/svg/svg_resources.h"
 #include "third_party/blink/renderer/core/layout/svg/transformed_hit_test_location.h"
 #include "third_party/blink/renderer/core/paint/paint_layer.h"
 #include "third_party/blink/renderer/core/paint/svg_foreign_object_painter.h"
@@ -125,11 +125,14 @@ void LayoutSVGForeignObject::UpdateLayout() {
   // specifying them through CSS.
   SetLocation(LayoutPoint(zoomed_location));
 
-  const bool layout_changed = EverHadLayout() && SelfNeedsLayout();
   LayoutBlock::UpdateLayout();
   DCHECK(!NeedsLayout());
   const bool bounds_changed = old_frame_rect != FrameRect();
 
+  // Invalidate all resources of this client if our reference box changed.
+  if (EverHadLayout() && bounds_changed)
+    SVGResourceInvalidator(*this).InvalidateEffects();
+
   bool update_parent_boundaries = bounds_changed;
   if (UpdateTransformAfterLayout(bounds_changed))
     update_parent_boundaries = true;
@@ -138,10 +141,6 @@ void LayoutSVGForeignObject::UpdateLayout() {
   if (update_parent_boundaries)
     LayoutSVGBlock::SetNeedsBoundariesUpdate();
 
-  // Invalidate all resources of this client if our layout changed.
-  if (layout_changed)
-    SVGResourcesCache::ClientLayoutChanged(*this);
-
   DCHECK(!needs_transform_update_);
 }
 

third_party/blink/renderer/core/layout/svg/layout_svg_image.cc

@@ -164,14 +164,14 @@ void LayoutSVGImage::UpdateLayout() {
   DCHECK(NeedsLayout());
   LayoutAnalyzer::Scope analyzer(*this);
 
-  // Invalidate all resources of this client if our layout changed.
-  if (EverHadLayout() && SelfNeedsLayout())
-    SVGResourcesCache::ClientLayoutChanged(*this);
-
   FloatPoint old_bbox_location = object_bounding_box_.Location();
   bool bbox_changed = UpdateBoundingBox() ||
                       old_bbox_location != object_bounding_box_.Location();
 
+  // Invalidate all resources of this client if our reference box changed.
+  if (EverHadLayout() && bbox_changed)
+    SVGResourceInvalidator(*this).InvalidateEffects();
+
   bool update_parent_boundaries = false;
   if (needs_boundaries_update_) {
     local_visual_rect_ = object_bounding_box_;

third_party/blink/renderer/core/layout/svg/layout_svg_resource_container.cc

@@ -221,24 +221,10 @@ static inline void RemoveFromCacheAndInvalidateDependencies(
   if (!element)
     return;
 
-  if (SVGResources* resources =
-          SVGResourcesCache::CachedResourcesForLayoutObject(object)) {
-    SVGElementResourceClient* client = element->GetSVGResourceClient();
-    if (resources->HasClipOrMaskOrFilter()) {
-      InvalidationModeMask invalidation_mask =
-          SVGResourceClient::kBoundariesInvalidation;
-      bool filter_data_invalidated = false;
-      if (resources->Filter()) {
-        filter_data_invalidated = client->ClearFilterData();
-        invalidation_mask |=
-            filter_data_invalidated ? SVGResourceClient::kPaintInvalidation : 0;
-      }
-      LayoutSVGResourceContainer::MarkClientForInvalidation(object,
-                                                            invalidation_mask);
-      if (filter_data_invalidated)
-        client->MarkFilterDataDirty();
-    }
-  }
+  // TODO(fs): Do we still need this? (If bounds are invalidated on a leaf
+  // LayoutObject, we will propagate that during the required layout and
+  // invalidate effects of self and any ancestors at that time.)
+  SVGResourceInvalidator(object).InvalidateEffects();
 
   element->NotifyIncomingReferences([needs_layout](SVGElement& element) {
     DCHECK(element.GetLayoutObject());

third_party/blink/renderer/core/layout/svg/layout_svg_shape.cc

@@ -283,10 +283,6 @@ void LayoutSVGShape::UpdateLayout() {
   NOT_DESTROYED();
   LayoutAnalyzer::Scope analyzer(*this);
 
-  // Invalidate all resources of this client if our layout changed.
-  if (EverHadLayout() && SelfNeedsLayout())
-    SVGResourcesCache::ClientLayoutChanged(*this);
-
   // The cached stroke may be affected by the ancestor transform, and so needs
   // to be cleared regardless of whether the shape or bounds have changed.
   stroke_path_cache_.reset();
@@ -314,6 +310,13 @@ void LayoutSVGShape::UpdateLayout() {
     update_parent_boundaries = true;
   }
 
+  // Invalidate all resources of this client if our reference box changed.
+  if (EverHadLayout() && bbox_changed) {
+    SVGResourceInvalidator resource_invalidator(*this);
+    resource_invalidator.InvalidateEffects();
+    resource_invalidator.InvalidatePaints();
+  }
+
   if (!needs_transform_update_ && transform_uses_reference_box_) {
     needs_transform_update_ = CheckForImplicitTransformChange(bbox_changed);
     if (needs_transform_update_)

third_party/blink/renderer/core/layout/svg/layout_svg_text.cc

@@ -40,7 +40,6 @@
 #include "third_party/blink/renderer/core/layout/svg/line/svg_root_inline_box.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_resources.h"
-#include "third_party/blink/renderer/core/layout/svg/svg_resources_cache.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_text_layout_attributes_builder.h"
 #include "third_party/blink/renderer/core/layout/svg/transformed_hit_test_location.h"
 #include "third_party/blink/renderer/core/paint/svg_text_painter.h"
@@ -254,18 +253,19 @@ void LayoutSVGText::UpdateLayout() {
   needs_reordering_ = false;
 
   const bool bounds_changed = old_boundaries != ObjectBoundingBox();
-  if (bounds_changed)
+  if (bounds_changed) {
+    // Invalidate all resources of this client if our reference box changed.
+    SVGResourceInvalidator resource_invalidator(*this);
+    resource_invalidator.InvalidateEffects();
+    resource_invalidator.InvalidatePaints();
     update_parent_boundaries = true;
+  }
 
   if (UpdateTransformAfterLayout(bounds_changed))
     update_parent_boundaries = true;
 
   ClearLayoutOverflow();
 
-  // Invalidate all resources of this client if our layout changed.
-  if (EverHadLayout() && SelfNeedsLayout())
-    SVGResourcesCache::ClientLayoutChanged(*this);
-
   // If our bounds changed, notify the parents.
   if (update_parent_boundaries)
     LayoutSVGBlock::SetNeedsBoundariesUpdate();

third_party/blink/renderer/core/layout/svg/svg_resources.cc

@@ -808,4 +808,43 @@ void SVGElementResourceClient::Trace(Visitor* visitor) const {
   SVGResourceClient::Trace(visitor);
 }
 
+SVGResourceInvalidator::SVGResourceInvalidator(LayoutObject& object)
+    : resources_(SVGResourcesCache::CachedResourcesForLayoutObject(object)),
+      object_(object) {}
+
+void SVGResourceInvalidator::InvalidateEffects() {
+  if (!resources_)
+    return;
+  if (resources_->Filter()) {
+    SVGElementResourceClient* client = SVGResources::GetClient(object_);
+    if (client->ClearFilterData()) {
+      object_.SetNeedsPaintPropertyUpdate();
+      client->MarkFilterDataDirty();
+    }
+  }
+  if (resources_->Clipper())
+    object_.InvalidateClipPathCache();
+  if (resources_->Masker())
+    object_.SetNeedsPaintPropertyUpdate();
+}
+
+void SVGResourceInvalidator::InvalidatePaints() {
+  if (!resources_)
+    return;
+  bool needs_invalidation = false;
+  SVGElementResourceClient* client = SVGResources::GetClient(object_);
+  if (LayoutSVGResourcePaintServer* fill = resources_->Fill()) {
+    fill->RemoveClientFromCache(*client);
+    needs_invalidation = true;
+  }
+  if (LayoutSVGResourcePaintServer* stroke = resources_->Stroke()) {
+    stroke->RemoveClientFromCache(*client);
+    needs_invalidation = true;
+  }
+  if (!needs_invalidation)
+    return;
+  object_.SetSubtreeShouldDoFullPaintInvalidation(
+      PaintInvalidationReason::kSVGResource);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/layout/svg/svg_resources.h

@@ -237,6 +237,25 @@ class SVGElementResourceClient final
   bool filter_data_dirty_;
 };
 
+// Helper class for handling invalidation of resources (generally after the
+// reference box of a LayoutObject may have changed).
+class SVGResourceInvalidator {
+  STACK_ALLOCATED();
+
+ public:
+  explicit SVGResourceInvalidator(LayoutObject& object);
+
+  // Invalidate any associated clip-path/mask/filter.
+  void InvalidateEffects();
+
+  // Invalidate any associated paints (fill/stroke).
+  void InvalidatePaints();
+
+ private:
+  const SVGResources* resources_;
+  LayoutObject& object_;
+};
+
 }  // namespace blink
 
 #endif

third_party/blink/renderer/core/layout/svg/svg_resources_cache.cc

@@ -82,40 +82,6 @@ SVGResources* SVGResourcesCache::CachedResourcesForLayoutObject(
   return ResourcesCache(layout_object.GetDocument()).cache_.at(&layout_object);
 }
 
-void SVGResourcesCache::ClientLayoutChanged(LayoutObject& object) {
-  SVGResources* resources = CachedResourcesForLayoutObject(object);
-  if (!resources)
-    return;
-  // Invalidate the resources if either the LayoutObject itself changed,
-  // or we have filter resources, which could depend on the layout of children.
-  if (!object.SelfNeedsLayout() && !resources->Filter())
-    return;
-  SVGElementResourceClient* client = SVGResources::GetClient(object);
-  InvalidationModeMask invalidation_flags = 0;
-  if (resources->HasClipOrMaskOrFilter())
-    invalidation_flags = SVGResourceClient::kBoundariesInvalidation;
-  bool filter_data_invalidated = false;
-  if (resources->Filter()) {
-    filter_data_invalidated = client->ClearFilterData();
-    invalidation_flags |=
-        filter_data_invalidated ? SVGResourceClient::kPaintInvalidation : 0;
-  }
-  if (LayoutSVGResourcePaintServer* fill = resources->Fill()) {
-    fill->RemoveClientFromCache(*client);
-    invalidation_flags |= SVGResourceClient::kPaintInvalidation;
-  }
-  if (LayoutSVGResourcePaintServer* stroke = resources->Stroke()) {
-    stroke->RemoveClientFromCache(*client);
-    invalidation_flags |= SVGResourceClient::kPaintInvalidation;
-  }
-  if (invalidation_flags) {
-    LayoutSVGResourceContainer::MarkClientForInvalidation(object,
-                                                          invalidation_flags);
-    if (filter_data_invalidated)
-      client->MarkFilterDataDirty();
-  }
-}
-
 static inline bool LayoutObjectCanHaveResources(
     const LayoutObject& layout_object) {
   return layout_object.GetNode() && layout_object.GetNode()->IsSVGElement() &&

third_party/blink/renderer/core/layout/svg/svg_resources_cache.h

@@ -52,9 +52,6 @@ class SVGResourcesCache {
   // LayoutSVGResourceContainer.
   static void ClientDestroyed(LayoutObject&);
 
-  // Called from all SVG layoutObjects layout() methods.
-  static void ClientLayoutChanged(LayoutObject&);
-
   // Called from all SVG layoutObjects styleDidChange() methods.
   static void ClientStyleChanged(LayoutObject&,
                                  StyleDifference,