[CompositeSVG] Fix DCHECK failure when an svg root graphics layer becomes hit test only

We need to clear GraphicsLayer::should_create_layers_after_paint_
when a layer becomes not drawing contents as we may not go through
UpdateShouldCreateLayersAfterPaint (which is after a repaint) for
the layer.

Bug: 1124241
Change-Id: Ibc51b2d6d536f2a60d86664b9a514b81f10598db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392941
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804568}

third_party/blink/renderer/platform/graphics/graphics_layer.cc

@@ -385,7 +385,6 @@ bool GraphicsLayer::PaintWithoutCommit(const IntRect* interest_rect) {
 }
 
 void GraphicsLayer::NotifyChildListChange() {
-  // cc::Layers are created in PaintArtifactCompositor.
   client_.GraphicsLayersDidChange();
 }
 
@@ -519,6 +518,11 @@ void GraphicsLayer::SetDrawsContent(bool draws_content) {
   if (draws_content == draws_content_)
     return;
 
+  // This may affect which layers the client collects.
+  client_.GraphicsLayersDidChange();
+  // This flag will be updated when the layer is repainted.
+  should_create_layers_after_paint_ = false;
+
   draws_content_ = draws_content;
   UpdateLayerIsDrawable();
 
@@ -558,9 +562,21 @@ void GraphicsLayer::SetContentsOpaqueForText(bool opaque) {
   CcLayer().SetContentsOpaqueForText(opaque);
 }
 
+void GraphicsLayer::SetPaintsHitTest(bool paints_hit_test) {
+  if (paints_hit_test_ == paints_hit_test)
+    return;
+  // This may affect which layers the client collects.
+  client_.GraphicsLayersDidChange();
+  // This flag will be updated when the layer is repainted.
+  should_create_layers_after_paint_ = false;
+  paints_hit_test_ = paints_hit_test;
+}
+
 void GraphicsLayer::SetHitTestable(bool should_hit_test) {
   if (hit_testable_ == should_hit_test)
     return;
+  // This may affect which layers the client collects.
+  client_.GraphicsLayersDidChange();
   hit_testable_ = should_hit_test;
   CcLayer().SetHitTestable(should_hit_test);
 }

third_party/blink/renderer/platform/graphics/graphics_layer.h

@@ -128,7 +128,7 @@ class PLATFORM_EXPORT GraphicsLayer : public DisplayItemClient,
   // This is different from |DrawsContent| because hit test data are internal
   // to blink and are not copied to the cc::Layer's display list.
   bool PaintsHitTest() const { return paints_hit_test_; }
-  void SetPaintsHitTest(bool paints) { paints_hit_test_ = paints; }
+  void SetPaintsHitTest(bool);
 
   bool PaintsContentOrHitTest() const {
     return draws_content_ || paints_hit_test_;

third_party/blink/renderer/platform/graphics/graphics_layer_test.cc

@@ -35,7 +35,6 @@
 #include "third_party/blink/renderer/platform/testing/fake_graphics_layer.h"
 #include "third_party/blink/renderer/platform/testing/fake_graphics_layer_client.h"
 #include "third_party/blink/renderer/platform/testing/paint_property_test_helpers.h"
-#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h"
 #include "third_party/blink/renderer/platform/testing/viewport_layers_setup.h"
 #include "third_party/blink/renderer/platform/transforms/matrix_3d_transform_operation.h"
 #include "third_party/blink/renderer/platform/transforms/rotate_transform_operation.h"
@@ -43,11 +42,7 @@
 
 namespace blink {
 
-class GraphicsLayerTest : public testing::Test, public PaintTestConfigurations {
- public:
-  GraphicsLayerTest() = default;
-  ~GraphicsLayerTest() = default;
-
+class GraphicsLayerTest : public testing::Test {
  protected:
   void CommitAndFinishCycle(GraphicsLayer& layer) {
     layer.GetPaintController().CommitNewDisplayItems();
@@ -71,9 +66,7 @@ class GraphicsLayerTest : public testing::Test, public PaintTestConfigurations {
   ViewportLayersSetup layers_;
 };
 
-INSTANTIATE_TEST_SUITE_P(All, GraphicsLayerTest, testing::Values(0));
-
-TEST_P(GraphicsLayerTest, Paint) {
+TEST_F(GraphicsLayerTest, Paint) {
   IntRect interest_rect(1, 2, 3, 4);
   auto& layer = layers_.graphics_layer();
   EXPECT_TRUE(layer.PaintWithoutCommitForTesting(interest_rect));
@@ -97,7 +90,7 @@ TEST_P(GraphicsLayerTest, Paint) {
   EXPECT_FALSE(layer.PaintWithoutCommitForTesting(interest_rect));
 }
 
-TEST_P(GraphicsLayerTest, PaintRecursively) {
+TEST_F(GraphicsLayerTest, PaintRecursively) {
   IntRect interest_rect(1, 2, 3, 4);
   const auto& transform_root = TransformPaintPropertyNode::Root();
   auto transform1 =
@@ -136,7 +129,7 @@ TEST_P(GraphicsLayerTest, PaintRecursively) {
   layers_.graphics_layer().GetPaintController().FinishCycle();
 }
 
-TEST_P(GraphicsLayerTest, SetDrawsContentFalse) {
+TEST_F(GraphicsLayerTest, SetDrawsContentFalse) {
   EXPECT_TRUE(layers_.graphics_layer().DrawsContent());
   layers_.graphics_layer().GetPaintController();
   EXPECT_NE(nullptr, GetInternalPaintController(layers_.graphics_layer()));
@@ -148,7 +141,7 @@ TEST_P(GraphicsLayerTest, SetDrawsContentFalse) {
   EXPECT_EQ(nullptr, GetInternalRasterInvalidator(layers_.graphics_layer()));
 }
 
-TEST_P(GraphicsLayerTest, ContentsLayer) {
+TEST_F(GraphicsLayerTest, ContentsLayer) {
   auto& graphics_layer = layers_.graphics_layer();
   auto contents_layer = cc::Layer::Create();
   graphics_layer.SetContentsToCcLayer(contents_layer, true);
@@ -159,4 +152,50 @@ TEST_P(GraphicsLayerTest, ContentsLayer) {
   EXPECT_EQ(nullptr, graphics_layer.ContentsLayer());
 }
 
+TEST_F(GraphicsLayerTest, ShouldCreateLayersAfterPaint) {
+  FakeGraphicsLayerClient svg_root_client;
+  svg_root_client.SetIsSVGRoot(true);
+  auto svg_root_layer = std::make_unique<FakeGraphicsLayer>(svg_root_client);
+  svg_root_layer->SetDrawsContent(true);
+  svg_root_layer->SetHitTestable(true);
+  svg_root_layer->SetLayerState(PropertyTreeState::Root(), IntPoint());
+  layers_.graphics_layer().AddChild(svg_root_layer.get());
+  layers_.graphics_layer().SetDrawsContent(false);
+
+  auto effect = CreateOpacityEffect(EffectPaintPropertyNode::Root(), 1,
+                                    CompositingReason::kWillChangeOpacity);
+  svg_root_client.SetPainter([&](const GraphicsLayer* layer,
+                                 GraphicsContext& context,
+                                 GraphicsLayerPaintingPhase, const IntRect&) {
+    {
+      ScopedPaintChunkProperties properties(context.GetPaintController(),
+                                            *effect, *svg_root_layer,
+                                            kBackgroundType);
+      PaintControllerTestBase::DrawRect(context, *layer, kBackgroundType,
+                                        IntRect(0, 0, 100, 100));
+    }
+  });
+
+  svg_root_client.SetNeedsRepaint(true);
+  HashSet<const GraphicsLayer*> repainted_layers;
+  layers_.graphics_layer().PaintRecursively(repainted_layers);
+  EXPECT_TRUE(repainted_layers.Contains(svg_root_layer.get()));
+  EXPECT_TRUE(svg_root_layer->ShouldCreateLayersAfterPaint());
+  svg_root_client.SetNeedsRepaint(false);
+  svg_root_layer->GetPaintController().FinishCycle();
+
+  svg_root_layer->SetDrawsContent(false);
+  repainted_layers.clear();
+  layers_.graphics_layer().PaintRecursively(repainted_layers);
+  EXPECT_FALSE(repainted_layers.Contains(svg_root_layer.get()));
+  EXPECT_FALSE(svg_root_layer->ShouldCreateLayersAfterPaint());
+
+  svg_root_client.SetNeedsRepaint(true);
+  svg_root_layer->SetPaintsHitTest(true);
+  layers_.graphics_layer().PaintRecursively(repainted_layers);
+  EXPECT_TRUE(repainted_layers.Contains(svg_root_layer.get()));
+  EXPECT_TRUE(svg_root_layer->ShouldCreateLayersAfterPaint());
+  svg_root_layer->GetPaintController().FinishCycle();
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/testing/fake_graphics_layer_client.h

@@ -32,6 +32,7 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
     if (painter_)
       painter_(layer, context, phase, rect);
   }
+  bool IsSVGRoot() const override { return is_svg_root_; }
 
   void SetIsTrackingRasterInvalidations(bool is_tracking_raster_invalidations) {
     is_tracking_raster_invalidations_ = is_tracking_raster_invalidations;
@@ -45,10 +46,13 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
                                      const IntRect&)>;
   void SetPainter(const Painter& painter) { painter_ = painter; }
 
+  void SetIsSVGRoot(bool is_svg_root) { is_svg_root_ = is_svg_root; }
+
  private:
   Painter painter_ = nullptr;
   bool is_tracking_raster_invalidations_ = false;
   bool needs_repaint_ = false;
+  bool is_svg_root_ = false;
 };
 
 }  // namespace blink