Remove most synchronous SVG resource layouts

Since most resources - markers excepted - no longer influence layout, we
can burn down most of SVGLayoutSupport::LayoutResourcesIfNeeded, only
leaving the essential bits (marker layout). Move the remnant of the
function closer to its single user. Move the |is_in_layout_| member
field from LayoutSVGResourceContainer to its LayoutSVGResourceMarker
subclass.

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

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

@@ -19,7 +19,6 @@
 
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_container.h"
 
-#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"
@@ -42,7 +41,6 @@ LocalSVGResource* ResourceForContainer(
 
 LayoutSVGResourceContainer::LayoutSVGResourceContainer(SVGElement* node)
     : LayoutSVGHiddenContainer(node),
-      is_in_layout_(false),
       completed_invalidations_mask_(0),
       is_invalidating_(false) {}
 
@@ -50,17 +48,10 @@ LayoutSVGResourceContainer::~LayoutSVGResourceContainer() = default;
 
 void LayoutSVGResourceContainer::UpdateLayout() {
   NOT_DESTROYED();
-  // FIXME: Investigate a way to detect and break resource layout dependency
-  // cycles early. Then we can remove this method altogether, and fall back onto
-  // LayoutSVGHiddenContainer::layout().
+  // TODO(fs): This is only here to clear the invalidation mask, without that
+  // we wouldn't need to override LayoutSVGHiddenContainer::UpdateLayout().
   DCHECK(NeedsLayout());
-  if (is_in_layout_)
-    return;
-
-  base::AutoReset<bool> in_layout_change(&is_in_layout_, true);
-
   LayoutSVGHiddenContainer::UpdateLayout();
-
   ClearInvalidationMask();
 }
 

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

@@ -100,8 +100,6 @@ class LayoutSVGResourceContainer : public LayoutSVGHiddenContainer {
   void StyleDidChange(StyleDifference, const ComputedStyle* old_style) override;
   void WillBeDestroyed() override;
 
-  bool is_in_layout_;
-
  private:
   // Track global (markAllClientsForInvalidation) invalidations to avoid
   // redundant crawls.

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

@@ -31,7 +31,9 @@
 namespace blink {
 
 LayoutSVGResourceMarker::LayoutSVGResourceMarker(SVGMarkerElement* node)
-    : LayoutSVGResourceContainer(node), needs_transform_update_(true) {}
+    : LayoutSVGResourceContainer(node),
+      needs_transform_update_(true),
+      is_in_layout_(false) {}
 
 LayoutSVGResourceMarker::~LayoutSVGResourceMarker() = default;
 
@@ -43,9 +45,9 @@ void LayoutSVGResourceMarker::UpdateLayout() {
 
   base::AutoReset<bool> in_layout_change(&is_in_layout_, true);
 
-  // LayoutSVGHiddenContainer overwrites layout(). We need the
-  // layouting of LayoutSVGContainer for calculating  local
-  // transformations and paint invalidation.
+  // LayoutSVGHiddenContainer overrides UpdateLayout(). We need the
+  // LayoutSVGContainer behavior for calculating local transformations and paint
+  // invalidation.
   LayoutSVGContainer::UpdateLayout();
 
   ClearInvalidationMask();

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

@@ -80,6 +80,7 @@ class LayoutSVGResourceMarker final : public LayoutSVGResourceContainer {
   AffineTransform local_to_parent_transform_;
   FloatSize viewport_size_;
   bool needs_transform_update_;
+  bool is_in_layout_;
 };
 
 DEFINE_LAYOUT_SVG_RESOURCE_TYPE_CASTS(LayoutSVGResourceMarker,

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

@@ -233,8 +233,6 @@ void LayoutSVGRoot::UpdateLayout() {
   did_screen_scale_factor_change_ =
       transform_change == SVGTransformChange::kFull;
 
-  SVGLayoutSupport::LayoutResourcesIfNeeded(*this);
-
   // selfNeedsLayout() will cover changes to one (or more) of viewBox,
   // current{Scale,Translate}, decorations and 'overflow'.
   const bool viewport_may_have_changed =

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

@@ -7,13 +7,28 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_container.h"
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.h"
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_image.h"
+#include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_marker.h"
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_shape.h"
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_text.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
-#include "third_party/blink/renderer/core/svg/svg_element.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 {
 
+static void LayoutMarkerResourcesIfNeeded(LayoutObject& layout_object) {
+  SVGResources* resources =
+      SVGResourcesCache::CachedResourcesForLayoutObject(layout_object);
+  if (!resources)
+    return;
+  if (LayoutSVGResourceMarker* marker = resources->MarkerStart())
+    marker->LayoutIfNeeded();
+  if (LayoutSVGResourceMarker* marker = resources->MarkerMid())
+    marker->LayoutIfNeeded();
+  if (LayoutSVGResourceMarker* marker = resources->MarkerEnd())
+    marker->LayoutIfNeeded();
+}
+
 void SVGContentContainer::Layout(const SVGContainerLayoutInfo& layout_info) {
   for (LayoutObject* child = children_.FirstChild(); child;
        child = child->NextSibling()) {
@@ -54,11 +69,10 @@ void SVGContentContainer::Layout(const SVGContainerLayoutInfo& layout_info) {
     // SVGSVGElement::svgAttributeChange() or at a higher SubtreeLayoutScope (in
     // LayoutView::layout()). We do not create a SubtreeLayoutScope for
     // resources because their ability to reference each other leads to circular
-    // layout. We protect against that within the layout code for resources, but
-    // it causes assertions if we use a SubTreeLayoutScope for them.
+    // layout. We protect against that within the layout code for marker
+    // resources, but it causes assertions if we use a SubtreeLayoutScope for
+    // them.
     if (child->IsSVGResourceContainer()) {
-      // Lay out any referenced resources before the child.
-      SVGLayoutSupport::LayoutResourcesIfNeeded(*child);
       child->LayoutIfNeeded();
     } else {
       SubtreeLayoutScope layout_scope(*child);
@@ -68,7 +82,7 @@ void SVGContentContainer::Layout(const SVGContainerLayoutInfo& layout_info) {
       }
 
       // Lay out any referenced resources before the child.
-      SVGLayoutSupport::LayoutResourcesIfNeeded(*child);
+      LayoutMarkerResourcesIfNeeded(*child);
       child->LayoutIfNeeded();
     }
   }

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

@@ -245,13 +245,6 @@ bool SVGLayoutSupport::ScreenScaleFactorChanged(const LayoutObject* ancestor) {
   return false;
 }
 
-void SVGLayoutSupport::LayoutResourcesIfNeeded(const LayoutObject& object) {
-  SVGResources* resources =
-      SVGResourcesCache::CachedResourcesForLayoutObject(object);
-  if (resources)
-    resources->LayoutIfNeeded();
-}
-
 bool SVGLayoutSupport::IsOverflowHidden(const LayoutObject& object) {
   // LayoutSVGRoot should never query for overflow state - it should always clip
   // itself to the initial viewport size.

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

@@ -47,9 +47,6 @@ class CORE_EXPORT SVGLayoutSupport {
   STATIC_ONLY(SVGLayoutSupport);
 
  public:
-  // Layout resources used by this node.
-  static void LayoutResourcesIfNeeded(const LayoutObject&);
-
   // Helper function determining whether overflow is hidden.
   static bool IsOverflowHidden(const LayoutObject&);
   static bool IsOverflowHidden(const ComputedStyle&);

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

@@ -237,37 +237,6 @@ std::unique_ptr<SVGResources> SVGResources::BuildResources(
                                                        : std::move(resources);
 }
 
-void SVGResources::LayoutIfNeeded() {
-  if (clipper_filter_masker_data_) {
-    if (LayoutSVGResourceClipper* clipper =
-            clipper_filter_masker_data_->clipper)
-      clipper->LayoutIfNeeded();
-    if (LayoutSVGResourceMasker* masker = clipper_filter_masker_data_->masker)
-      masker->LayoutIfNeeded();
-    if (LayoutSVGResourceFilter* filter = clipper_filter_masker_data_->filter)
-      filter->LayoutIfNeeded();
-  }
-
-  if (marker_data_) {
-    if (LayoutSVGResourceMarker* marker = marker_data_->marker_start)
-      marker->LayoutIfNeeded();
-    if (LayoutSVGResourceMarker* marker = marker_data_->marker_mid)
-      marker->LayoutIfNeeded();
-    if (LayoutSVGResourceMarker* marker = marker_data_->marker_end)
-      marker->LayoutIfNeeded();
-  }
-
-  if (fill_stroke_data_) {
-    if (LayoutSVGResourcePaintServer* fill = fill_stroke_data_->fill)
-      fill->LayoutIfNeeded();
-    if (LayoutSVGResourcePaintServer* stroke = fill_stroke_data_->stroke)
-      stroke->LayoutIfNeeded();
-  }
-
-  if (linked_resource_)
-    linked_resource_->LayoutIfNeeded();
-}
-
 void SVGResources::ResourceDestroyed(LayoutSVGResourceContainer* resource) {
   DCHECK(resource);
   if (!HasResourceData())

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

@@ -70,8 +70,6 @@ class SVGResources {
                             const ComputedStyle&);
   static void ClearMarkers(SVGElement&, const ComputedStyle*);
 
-  void LayoutIfNeeded();
-
   static bool SupportsMarkers(const SVGElement&);
 
   // Ordinary resources