Simplify SVGResourcesCache::ClientStyleChanged

This is largely a revert of the code-change from
a77da2a9, since that workaround should
no longer be required now that the visual rect no longer has bounds of
clip-path, mask and filter baked in.
Also replace the check of LayoutObjectCanHaveResources() with a DCHECK
since this is guaranteed by the callers.

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

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

@@ -268,18 +268,6 @@ void SVGResources::LayoutIfNeeded() {
     linked_resource_->LayoutIfNeeded();
 }
 
-bool SVGResources::DifferenceNeedsLayout(const SVGResources* a,
-                                         const SVGResources* b) {
-  bool a_has_bounds_affecting_resource = a && a->clipper_filter_masker_data_;
-  bool b_has_bounds_affecting_resource = b && b->clipper_filter_masker_data_;
-  if (a_has_bounds_affecting_resource != b_has_bounds_affecting_resource)
-    return true;
-  if (!a_has_bounds_affecting_resource)
-    return false;
-  return a->Clipper() != b->Clipper() || a->Filter() != b->Filter() ||
-         a->Masker() != b->Masker();
-}
-
 void SVGResources::ResourceDestroyed(LayoutSVGResourceContainer* resource) {
   DCHECK(resource);
   if (!HasResourceData())

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

@@ -120,8 +120,6 @@ class SVGResources {
   void ResourceDestroyed(LayoutSVGResourceContainer*);
   void ClearReferencesTo(LayoutSVGResourceContainer*);
 
-  static bool DifferenceNeedsLayout(const SVGResources*, const SVGResources*);
-
 #if DCHECK_IS_ON()
   void Dump(const LayoutObject*);
 #endif

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

@@ -33,7 +33,7 @@ SVGResourcesCache::SVGResourcesCache() = default;
 
 SVGResourcesCache::~SVGResourcesCache() = default;
 
-SVGResources* SVGResourcesCache::AddResourcesFromLayoutObject(
+bool SVGResourcesCache::AddResourcesFromLayoutObject(
     LayoutObject& object,
     const ComputedStyle& style) {
   DCHECK(!cache_.Contains(&object));
@@ -42,7 +42,7 @@ SVGResources* SVGResourcesCache::AddResourcesFromLayoutObject(
   std::unique_ptr<SVGResources> new_resources =
       SVGResources::BuildResources(object, style);
   if (!new_resources)
-    return nullptr;
+    return false;
 
   // Put object in cache.
   SVGResources* resources =
@@ -57,7 +57,7 @@ SVGResources* SVGResourcesCache::AddResourcesFromLayoutObject(
     if (resource_container->FindCycle(solver))
       resources->ClearReferencesTo(resource_container);
   }
-  return resources;
+  return true;
 }
 
 bool SVGResourcesCache::RemoveResourcesFromLayoutObject(LayoutObject& object) {
@@ -65,15 +65,12 @@ bool SVGResourcesCache::RemoveResourcesFromLayoutObject(LayoutObject& object) {
   return !!resources;
 }
 
-SVGResourcesCache::ResourceUpdateInfo
-SVGResourcesCache::UpdateResourcesFromLayoutObject(
+bool SVGResourcesCache::UpdateResourcesFromLayoutObject(
     LayoutObject& object,
     const ComputedStyle& new_style) {
-  std::unique_ptr<SVGResources> old_resources = cache_.Take(&object);
-  SVGResources* new_resources = AddResourcesFromLayoutObject(object, new_style);
-  return {
-      old_resources || new_resources,
-      SVGResources::DifferenceNeedsLayout(old_resources.get(), new_resources)};
+  bool did_update = RemoveResourcesFromLayoutObject(object);
+  did_update |= AddResourcesFromLayoutObject(object, new_style);
+  return did_update;
 }
 
 static inline SVGResourcesCache& ResourcesCache(Document& document) {
@@ -147,32 +144,23 @@ void SVGResourcesCache::ClientStyleChanged(LayoutObject& layout_object,
 
   // LayoutObjects for SVGFE*Element should not be calling this function.
   DCHECK(!layout_object.IsSVGFilterPrimitive());
+  // We only call this function on LayoutObjects that fulfil this condition.
+  DCHECK(LayoutObjectCanHaveResources(layout_object));
 
   // Dynamic changes of CSS properties like 'clip-path' may require us to
   // recompute the associated resources for a LayoutObject.
   // TODO(fs): Avoid passing in a useless StyleDifference, but instead compare
   // oldStyle/newStyle to see which resources changed to be able to selectively
   // rebuild individual resources, instead of all of them.
-  bool needs_layout = false;
-  if (LayoutObjectCanHaveResources(layout_object)) {
-    SVGResourcesCache& cache = ResourcesCache(layout_object.GetDocument());
-    auto update_info =
-        cache.UpdateResourcesFromLayoutObject(layout_object, new_style);
-    if (update_info) {
-      layout_object.SetNeedsPaintPropertyUpdate();
-      // Since the visual rect has the bounds of the clip-path, mask and filter
-      // baked in, and the visual rect is updated during layout, we need to
-      // trigger layout if the style change could somehow have affected the
-      // bounds that form the visual rect.
-      needs_layout = update_info.needs_layout;
-    }
-  }
+  SVGResourcesCache& cache = ResourcesCache(layout_object.GetDocument());
+  if (cache.UpdateResourcesFromLayoutObject(layout_object, new_style))
+    layout_object.SetNeedsPaintPropertyUpdate();
 
   // If this layoutObject is the child of ResourceContainer and it require
   // repainting that changes of CSS properties such as 'visibility',
   // request repainting.
-  needs_layout |= diff.NeedsPaintInvalidation() &&
-                  IsLayoutObjectOfResourceContainer(layout_object);
+  bool needs_layout = diff.NeedsPaintInvalidation() &&
+                      IsLayoutObjectOfResourceContainer(layout_object);
 
   LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation(
       layout_object, needs_layout);

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

@@ -85,17 +85,9 @@ class SVGResourcesCache {
   };
 
  private:
-  struct ResourceUpdateInfo {
-    bool changed;
-    bool needs_layout;
-
-    explicit operator bool() const { return changed; }
-  };
-  SVGResources* AddResourcesFromLayoutObject(LayoutObject&,
-                                             const ComputedStyle&);
+  bool AddResourcesFromLayoutObject(LayoutObject&, const ComputedStyle&);
   bool RemoveResourcesFromLayoutObject(LayoutObject&);
-  ResourceUpdateInfo UpdateResourcesFromLayoutObject(LayoutObject&,
-                                                     const ComputedStyle&);
+  bool UpdateResourcesFromLayoutObject(LayoutObject&, const ComputedStyle&);
 
   typedef HashMap<const LayoutObject*, std::unique_ptr<SVGResources>> CacheMap;
   CacheMap cache_;