Rework the SVG resource cycle-checker

This moves the traversal code from SVGResourcesCycleSolver to
LayoutSVGResourceContainer. The reason for this is to be able to perform
slightly more heterogeneous traversal - i.e not only using SVGResources.
The primary use case this is to get rid of the "linked" resource from
SVGResources (currently only used by <pattern>s). This also the reason
for the split into FindCycleFromSelf() and FindCycleInDescendants() -
the former is expected to be overridden by a resource that needs to take
special care of some specific reference. (This mechanism could hopefully
also be used/extended to handled cycles via <feImage> in filters.)

Bug: 1028063
Change-Id: I34c80037516e0059e8b4cb4113fa0f4147d7ec31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066722Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#743488}

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

@@ -22,6 +22,7 @@
 #include "base/auto_reset.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_resources_cycle_solver.h"
 #include "third_party/blink/renderer/core/svg/svg_resource.h"
 #include "third_party/blink/renderer/core/svg/svg_tree_scope_resources.h"
 
@@ -83,6 +84,60 @@ void LayoutSVGResourceContainer::StyleDidChange(
     resource->NotifyResourceAttached(*this);
 }
 
+bool LayoutSVGResourceContainer::FindCycle(
+    SVGResourcesCycleSolver& solver) const {
+  if (solver.IsKnownAcyclic(this))
+    return false;
+  SVGResourcesCycleSolver::Scope scope(solver);
+  if (!scope.Enter(this) || FindCycleFromSelf(solver))
+    return true;
+  solver.AddAcyclicSubgraph(this);
+  return false;
+}
+
+bool LayoutSVGResourceContainer::FindCycleInResources(
+    SVGResourcesCycleSolver& solver,
+    const LayoutObject& layout_object) const {
+  SVGResources* resources =
+      SVGResourcesCache::CachedResourcesForLayoutObject(layout_object);
+  if (!resources)
+    return false;
+  // Fetch all the referenced resources.
+  HashSet<LayoutSVGResourceContainer*> local_resources;
+  resources->BuildSetOfResources(local_resources);
+  // This performs a depth-first search for a back-edge in all the
+  // (potentially disjoint) graphs formed by the referenced resources.
+  for (auto* local_resource : local_resources) {
+    if (local_resource->FindCycle(solver))
+      return true;
+  }
+  return false;
+}
+
+bool LayoutSVGResourceContainer::FindCycleFromSelf(
+    SVGResourcesCycleSolver& solver) const {
+  if (FindCycleInResources(solver, *this))
+    return true;
+  return FindCycleInDescendants(solver);
+}
+
+bool LayoutSVGResourceContainer::FindCycleInDescendants(
+    SVGResourcesCycleSolver& solver) const {
+  LayoutObject* node = FirstChild();
+  while (node) {
+    // Skip subtrees which are themselves resources. (They will be
+    // processed - if needed - when they are actually referenced.)
+    if (node->IsSVGResourceContainer()) {
+      node = node->NextInPreOrderAfterChildren(this);
+      continue;
+    }
+    if (FindCycleInResources(solver, *node))
+      return true;
+    node = node->NextInPreOrder(this);
+  }
+  return false;
+}
+
 void LayoutSVGResourceContainer::MarkAllClientsForInvalidation(
     InvalidationModeMask invalidation_mask) {
   if (is_invalidating_)

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

@@ -25,6 +25,8 @@
 
 namespace blink {
 
+class SVGResourcesCycleSolver;
+
 enum LayoutSVGResourceType {
   kMaskerResourceType,
   kMarkerResourceType,
@@ -64,6 +66,8 @@ class LayoutSVGResourceContainer : public LayoutSVGHiddenContainer {
                                        SubtreeLayoutScope* = nullptr);
   void InvalidateCacheAndMarkForLayout(SubtreeLayoutScope* = nullptr);
 
+  bool FindCycle(SVGResourcesCycleSolver&) const;
+
   static void MarkForLayoutAndParentResourceInvalidation(
       LayoutObject&,
       bool needs_layout = true);
@@ -75,6 +79,11 @@ class LayoutSVGResourceContainer : public LayoutSVGHiddenContainer {
   // Used from RemoveAllClientsFromCache methods.
   void MarkAllClientsForInvalidation(InvalidationModeMask);
 
+  bool FindCycleFromSelf(SVGResourcesCycleSolver&) const;
+  bool FindCycleInDescendants(SVGResourcesCycleSolver&) const;
+  bool FindCycleInResources(SVGResourcesCycleSolver&,
+                            const LayoutObject&) const;
+
   void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
   void WillBeDestroyed() override;
 

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

@@ -52,9 +52,9 @@ SVGResources* SVGResourcesCache::AddResourcesFromLayoutObject(
   HashSet<LayoutSVGResourceContainer*> resource_set;
   resources->BuildSetOfResources(resource_set);
 
-  SVGResourcesCycleSolver solver(object);
+  SVGResourcesCycleSolver solver;
   for (auto* resource_container : resource_set) {
-    if (solver.FindCycle(resource_container))
+    if (resource_container->FindCycle(solver))
       resources->ClearReferencesTo(resource_container);
   }
   return resources;

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

@@ -19,99 +19,30 @@
 
 #include "third_party/blink/renderer/core/layout/svg/svg_resources_cycle_solver.h"
 
-#include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_container.h"
-#include "third_party/blink/renderer/core/layout/svg/svg_resources.h"
-#include "third_party/blink/renderer/core/layout/svg/svg_resources_cache.h"
-
 namespace blink {
 
-SVGResourcesCycleSolver::SVGResourcesCycleSolver(LayoutObject& layout_object)
-    : layout_object_(layout_object) {
-  if (layout_object.IsSVGResourceContainer())
-    active_resources_.insert(ToLayoutSVGResourceContainer(&layout_object));
-}
+SVGResourcesCycleSolver::SVGResourcesCycleSolver() = default;
 
 SVGResourcesCycleSolver::~SVGResourcesCycleSolver() = default;
 
-class ScopedTraversalPath {
- public:
-  typedef SVGResourcesCycleSolver::ResourceSet ResourceSet;
-
-  ScopedTraversalPath(ResourceSet& active_set)
-      : active_set_(active_set), resource_(nullptr) {}
-  ~ScopedTraversalPath() {
-    if (resource_)
-      active_set_.erase(resource_);
-  }
-
-  bool Enter(LayoutSVGResourceContainer* resource) {
-    if (!active_set_.insert(resource).is_new_entry)
-      return false;
-    resource_ = resource;
-    return true;
-  }
-
- private:
-  ResourceSet& active_set_;
-  LayoutSVGResourceContainer* resource_;
-};
-
-bool SVGResourcesCycleSolver::TraverseResourceContainer(
-    LayoutSVGResourceContainer* resource) {
-  // If we've traversed this sub-graph before and no cycles were observed, then
-  // reuse that result.
-  if (dag_cache_.Contains(resource))
-    return false;
-
-  ScopedTraversalPath scope(active_resources_);
-  if (!scope.Enter(resource))
-    return true;
-
-  LayoutObject* node = resource;
-  while (node) {
-    // Skip subtrees which are themselves resources. (They will be
-    // processed - if needed - when they are actually referenced.)
-    if (node != resource && node->IsSVGResourceContainer()) {
-      node = node->NextInPreOrderAfterChildren(resource);
-      continue;
-    }
-    if (TraverseResources(*node))
-      return true;
-    node = node->NextInPreOrder(resource);
-  }
-
-  // No cycles found in (or from) this resource. Add it to the "DAG cache".
-  dag_cache_.insert(resource);
-  return false;
+bool SVGResourcesCycleSolver::IsKnownAcyclic(
+    const LayoutSVGResourceContainer* resource) const {
+  return dag_cache_.Contains(resource);
 }
 
-bool SVGResourcesCycleSolver::TraverseResources(LayoutObject& layout_object) {
-  SVGResources* resources =
-      SVGResourcesCache::CachedResourcesForLayoutObject(layout_object);
-  return resources && TraverseResources(resources);
+void SVGResourcesCycleSolver::AddAcyclicSubgraph(
+    const LayoutSVGResourceContainer* resource) {
+  dag_cache_.insert(resource);
 }
 
-bool SVGResourcesCycleSolver::TraverseResources(SVGResources* resources) {
-  // Fetch all the referenced resources.
-  ResourceSet local_resources;
-  resources->BuildSetOfResources(local_resources);
-
-  // This performs a depth-first search for a back-edge in all the
-  // (potentially disjoint) graphs formed by the referenced resources.
-  for (auto* local_resource : local_resources) {
-    if (TraverseResourceContainer(local_resource))
-      return true;
-  }
-  return false;
+bool SVGResourcesCycleSolver::EnterResource(
+    const LayoutSVGResourceContainer* resource) {
+  return active_resources_.insert(resource).is_new_entry;
 }
 
-bool SVGResourcesCycleSolver::FindCycle(
-    LayoutSVGResourceContainer* start_node) {
-  DCHECK(active_resources_.IsEmpty() ||
-         (active_resources_.size() == 1 &&
-          active_resources_.Contains(
-              ToLayoutSVGResourceContainer(&layout_object_))));
-  return TraverseResourceContainer(start_node);
+void SVGResourcesCycleSolver::LeaveResource(
+    const LayoutSVGResourceContainer* resource) {
+  active_resources_.erase(resource);
 }
 
 }  // namespace blink

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

@@ -21,15 +21,12 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_SVG_SVG_RESOURCES_CYCLE_SOLVER_H_
 
 #include "base/macros.h"
+#include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_container.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 #include "third_party/blink/renderer/platform/wtf/hash_set.h"
 
 namespace blink {
 
-class LayoutObject;
-class LayoutSVGResourceContainer;
-class SVGResources;
-
 // This class traverses the graph formed by SVGResources of
 // LayoutObjects, maintaining the active path as LayoutObjects are
 // visited. It also maintains a cache of sub-graphs that has already
@@ -38,22 +35,40 @@ class SVGResourcesCycleSolver {
   STACK_ALLOCATED();
 
  public:
-  SVGResourcesCycleSolver(LayoutObject&);
+  SVGResourcesCycleSolver();
   ~SVGResourcesCycleSolver();
 
-  // Traverse the graph starting at the resource container
-  // passed. Returns true if a cycle is detected.
-  bool FindCycle(LayoutSVGResourceContainer*);
+  bool IsKnownAcyclic(const LayoutSVGResourceContainer*) const;
+  void AddAcyclicSubgraph(const LayoutSVGResourceContainer*);
 
-  typedef HashSet<LayoutSVGResourceContainer*> ResourceSet;
+  class Scope {
+    STACK_ALLOCATED();
 
- private:
-  bool TraverseResourceContainer(LayoutSVGResourceContainer*);
-  bool TraverseResources(LayoutObject&);
-  bool TraverseResources(SVGResources*);
+   public:
+    Scope(SVGResourcesCycleSolver& solver)
+        : solver_(solver), resource_(nullptr) {}
+    ~Scope() {
+      if (resource_)
+        solver_.LeaveResource(resource_);
+    }
+
+    bool Enter(const LayoutSVGResourceContainer* resource) {
+      if (!solver_.EnterResource(resource))
+        return false;
+      resource_ = resource;
+      return true;
+    }
 
-  LayoutObject& layout_object_;
+   private:
+    SVGResourcesCycleSolver& solver_;
+    const LayoutSVGResourceContainer* resource_;
+  };
+
+ private:
+  bool EnterResource(const LayoutSVGResourceContainer*);
+  void LeaveResource(const LayoutSVGResourceContainer*);
 
+  using ResourceSet = HashSet<const LayoutSVGResourceContainer*>;
   ResourceSet active_resources_;
   ResourceSet dag_cache_;
   DISALLOW_COPY_AND_ASSIGN(SVGResourcesCycleSolver);