RAII-ify SVGMaskPainter

This restructures SVGMaskPainter to use a RAII-structure similar to that
of ClipPathClipper. ScopedSVGPaintState is adjusted, and simplified,
accordingly.

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

third_party/blink/renderer/core/paint/scoped_svg_paint_state.cc

@@ -25,11 +25,8 @@
 #include "third_party/blink/renderer/core/paint/scoped_svg_paint_state.h"
 
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_filter.h"
-#include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_masker.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/paint/svg_mask_painter.h"
 #include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
 
 namespace blink {
@@ -76,15 +73,6 @@ ScopedSVGPaintState::~ScopedSVGPaintState() {
     filter_paint_info_ = nullptr;
     filter_data_ = nullptr;
   }
-
-  if (masker_) {
-    DCHECK(SVGResourcesCache::CachedResourcesForLayoutObject(object_));
-    DCHECK(
-        SVGResourcesCache::CachedResourcesForLayoutObject(object_)->Masker() ==
-        masker_);
-    SVGMaskPainter(*masker_).FinishEffect(object_, display_item_client_,
-                                          paint_info_.context);
-  }
 }
 
 bool ScopedSVGPaintState::ApplyEffects() {
@@ -116,19 +104,14 @@ bool ScopedSVGPaintState::ApplyEffects() {
     ApplyClipIfNecessary();
   }
 
-  SVGResources* resources =
-      SVGResourcesCache::CachedResourcesForLayoutObject(object_);
-
-  if (!ApplyMaskIfNecessary(resources))
-    return false;
+  ApplyMaskIfNecessary();
 
   if (is_svg_root_or_foreign_object) {
     // PaintLayerPainter takes care of filter.
     DCHECK(object_.HasLayer() || !object_.StyleRef().HasFilter());
-  } else if (!ApplyFilterIfNecessary(resources)) {
+  } else if (!ApplyFilterIfNecessary()) {
     return false;
   }
-
   return true;
 }
 
@@ -166,14 +149,9 @@ void ScopedSVGPaintState::ApplyClipIfNecessary() {
   }
 }
 
-bool ScopedSVGPaintState::ApplyMaskIfNecessary(SVGResources* resources) {
-  if (LayoutSVGResourceMasker* masker =
-          resources ? resources->Masker() : nullptr) {
-    if (!SVGMaskPainter(*masker).PrepareEffect(GetPaintInfo().context))
-      return false;
-    masker_ = masker;
-  }
-  return true;
+void ScopedSVGPaintState::ApplyMaskIfNecessary() {
+  if (object_.StyleRef().SvgStyle().MaskerResource())
+    mask_painter_.emplace(paint_info_.context, object_, display_item_client_);
 }
 
 static bool HasReferenceFilterOnly(const ComputedStyle& style) {
@@ -185,7 +163,9 @@ static bool HasReferenceFilterOnly(const ComputedStyle& style) {
   return operations.at(0)->GetType() == FilterOperation::REFERENCE;
 }
 
-bool ScopedSVGPaintState::ApplyFilterIfNecessary(SVGResources* resources) {
+bool ScopedSVGPaintState::ApplyFilterIfNecessary() {
+  SVGResources* resources =
+      SVGResourcesCache::CachedResourcesForLayoutObject(object_);
   if (!resources)
     return !HasReferenceFilterOnly(object_.StyleRef());
   LayoutSVGResourceFilter* filter = resources->Filter();

third_party/blink/renderer/core/paint/scoped_svg_paint_state.h

@@ -30,6 +30,7 @@
 #include "third_party/blink/renderer/core/paint/object_paint_properties.h"
 #include "third_party/blink/renderer/core/paint/paint_info.h"
 #include "third_party/blink/renderer/core/paint/svg_filter_painter.h"
+#include "third_party/blink/renderer/core/paint/svg_mask_painter.h"
 #include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h"
 #include "third_party/blink/renderer/platform/transforms/affine_transform.h"
 
@@ -37,8 +38,6 @@ namespace blink {
 
 class FilterData;
 class LayoutObject;
-class LayoutSVGResourceMasker;
-class SVGResources;
 
 // Hooks up the correct paint property transform node.
 class ScopedSVGTransformState {
@@ -89,8 +88,7 @@ class ScopedSVGPaintState {
       : object_(object),
         paint_info_(paint_info),
         display_item_client_(display_item_client),
-        filter_data_(nullptr),
-        masker_(nullptr) {}
+        filter_data_(nullptr) {}
 
   ~ScopedSVGPaintState();
 
@@ -105,24 +103,21 @@ class ScopedSVGPaintState {
  private:
   void ApplyPaintPropertyState();
   void ApplyClipIfNecessary();
-
-  // Return true if no masking is necessary or if the mask is successfully
-  // applied.
-  bool ApplyMaskIfNecessary(SVGResources*);
+  void ApplyMaskIfNecessary();
 
   // Return true if no filtering is necessary or if the filter is successfully
   // applied.
-  bool ApplyFilterIfNecessary(SVGResources*);
+  bool ApplyFilterIfNecessary();
 
   const LayoutObject& object_;
   PaintInfo paint_info_;
   const DisplayItemClient& display_item_client_;
   std::unique_ptr<PaintInfo> filter_paint_info_;
   FilterData* filter_data_;
-  LayoutSVGResourceMasker* masker_;
   base::Optional<ClipPathClipper> clip_path_clipper_;
   std::unique_ptr<SVGFilterRecordingContext> filter_recording_context_;
   base::Optional<ScopedPaintChunkProperties> scoped_paint_chunk_properties_;
+  base::Optional<SVGMaskPainter> mask_painter_;
 #if DCHECK_IS_ON()
   bool apply_clip_mask_and_filter_if_necessary_called_ = false;
 #endif

third_party/blink/renderer/core/paint/svg_mask_painter.cc

@@ -7,7 +7,6 @@
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_resource_masker.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_resources.h"
 #include "third_party/blink/renderer/core/paint/object_paint_properties.h"
-#include "third_party/blink/renderer/core/paint/paint_info.h"
 #include "third_party/blink/renderer/platform/graphics/paint/drawing_display_item.h"
 #include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_controller.h"
@@ -15,45 +14,48 @@
 
 namespace blink {
 
-bool SVGMaskPainter::PrepareEffect(GraphicsContext& context) {
-  DCHECK(mask_.Style());
-  SECURITY_DCHECK(!mask_.NeedsLayout());
-
-  mask_.ClearInvalidationMask();
-  return mask_.GetElement()->HasChildren();
+SVGMaskPainter::SVGMaskPainter(GraphicsContext& context,
+                               const LayoutObject& layout_object,
+                               const DisplayItemClient& display_item_client)
+    : context_(context),
+      layout_object_(layout_object),
+      display_item_client_(display_item_client) {
+  DCHECK(layout_object_.StyleRef().SvgStyle().MaskerResource());
 }
 
-void SVGMaskPainter::FinishEffect(const LayoutObject& object,
-                                  const DisplayItemClient& display_item_client,
-                                  GraphicsContext& context) {
-  DCHECK(mask_.Style());
-  SECURITY_DCHECK(!mask_.NeedsLayout());
-
-  base::Optional<ScopedPaintChunkProperties> scoped_paint_chunk_properties;
-  const auto* properties = object.FirstFragment().PaintProperties();
+SVGMaskPainter::~SVGMaskPainter() {
+  const auto* properties = layout_object_.FirstFragment().PaintProperties();
   // TODO(crbug.com/814815): This condition should be a DCHECK, but for now
   // we may paint the object for filters during PrePaint before the
   // properties are ready.
-  if (properties && properties->Mask()) {
-    scoped_paint_chunk_properties.emplace(
-        context.GetPaintController(), *properties->Mask(), display_item_client,
-        DisplayItem::kSVGMask);
-  }
-
-  AffineTransform content_transformation;
-  sk_sp<const PaintRecord> record = mask_.CreatePaintRecord(
-      content_transformation, SVGResources::ReferenceBoxForEffects(object),
-      context);
+  if (!properties || !properties->Mask())
+    return;
+  ScopedPaintChunkProperties scoped_paint_chunk_properties(
+      context_.GetPaintController(), *properties->Mask(), display_item_client_,
+      DisplayItem::kSVGMask);
 
-  if (DrawingRecorder::UseCachedDrawingIfPossible(context, display_item_client,
-                                                  DisplayItem::kSVGMask))
+  if (DrawingRecorder::UseCachedDrawingIfPossible(
+          context_, display_item_client_, DisplayItem::kSVGMask))
     return;
+  DrawingRecorder recorder(context_, display_item_client_,
+                           DisplayItem::kSVGMask);
+
+  const SVGComputedStyle& svg_style = layout_object_.StyleRef().SvgStyle();
+  auto* masker =
+      GetSVGResourceAsType<LayoutSVGResourceMasker>(svg_style.MaskerResource());
+  DCHECK(masker);
+  SECURITY_DCHECK(!masker->NeedsLayout());
+  masker->ClearInvalidationMask();
 
-  DrawingRecorder recorder(context, display_item_client, DisplayItem::kSVGMask);
-  context.Save();
-  context.ConcatCTM(content_transformation);
-  context.DrawRecord(std::move(record));
-  context.Restore();
+  AffineTransform content_transformation;
+  sk_sp<const PaintRecord> record = masker->CreatePaintRecord(
+      content_transformation,
+      SVGResources::ReferenceBoxForEffects(layout_object_), context_);
+
+  context_.Save();
+  context_.ConcatCTM(content_transformation);
+  context_.DrawRecord(std::move(record));
+  context_.Restore();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/paint/svg_mask_painter.h

@@ -5,7 +5,6 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_SVG_MASK_PAINTER_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_SVG_MASK_PAINTER_H_
 
-#include "third_party/blink/renderer/platform/geometry/float_rect.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
 namespace blink {
@@ -13,21 +12,20 @@ namespace blink {
 class DisplayItemClient;
 class GraphicsContext;
 class LayoutObject;
-class LayoutSVGResourceMasker;
 
 class SVGMaskPainter {
   STACK_ALLOCATED();
 
  public:
-  SVGMaskPainter(LayoutSVGResourceMasker& mask) : mask_(mask) {}
-
-  bool PrepareEffect(GraphicsContext&);
-  void FinishEffect(const LayoutObject&,
-                    const DisplayItemClient&,
-                    GraphicsContext&);
+  SVGMaskPainter(GraphicsContext&,
+                 const LayoutObject&,
+                 const DisplayItemClient&);
+  ~SVGMaskPainter();
 
  private:
-  LayoutSVGResourceMasker& mask_;
+  GraphicsContext& context_;
+  const LayoutObject& layout_object_;
+  const DisplayItemClient& display_item_client_;
 };
 
 }  // namespace blink