[CI] Pass PropertyTreeState to RecordForeignLayer

This shortens call sites, and removes the requirement of
DisplayItemClient at call sites.

Change-Id: I445a48a13fb52b07a1a9fccb912c16f829e1c404
Reviewed-on: https://chromium-review.googlesource.com/c/1371195
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615607}

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -133,7 +133,6 @@
 #include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
 #include "third_party/blink/renderer/platform/graphics/paint/foreign_layer_display_item.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_controller.h"
-#include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h"
 #include "third_party/blink/renderer/platform/histogram.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/traced_value.h"
@@ -2693,49 +2692,25 @@ void LocalFrameView::PerformScrollAnchoringAdjustments() {
   }
 }
 
+static void RecordGraphicsLayerAsForeignLayer(
+    GraphicsContext& context,
+    const GraphicsLayer* graphics_layer) {
+  // TODO(trchen): Currently the GraphicsLayer hierarchy is still built during
+  // CompositingUpdate, and we have to clear them here to ensure no extraneous
+  // layers are still attached. In future we will disable all those layer
+  // hierarchy code so we won't need this line.
+  graphics_layer->CcLayer()->RemoveAllChildren();
+  RecordForeignLayer(context, DisplayItem::kForeignLayerWrapper,
+                     graphics_layer->CcLayer(),
+                     graphics_layer->GetPropertyTreeState());
+}
+
 static void CollectViewportLayersForLayerList(GraphicsContext& context,
                                               VisualViewport& visual_viewport) {
   DCHECK(RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled());
-
-  // Collect the visual viewport container layer.
-  {
-    GraphicsLayer* container_layer = visual_viewport.ContainerLayer();
-    ScopedPaintChunkProperties container_scope(
-        context.GetPaintController(), container_layer->GetPropertyTreeState(),
-        *container_layer, DisplayItem::kForeignLayerWrapper);
-
-    // TODO(trchen): Currently the GraphicsLayer hierarchy is still built during
-    // CompositingUpdate, and we have to clear them here to ensure no extraneous
-    // layers are still attached. In future we will disable all those layer
-    // hierarchy code so we won't need this line.
-    container_layer->CcLayer()->RemoveAllChildren();
-    RecordForeignLayer(context, DisplayItem::kForeignLayerWrapper,
-                       container_layer->CcLayer());
-  }
-
-  // Collect the page scale layer.
-  {
-    GraphicsLayer* scale_layer = visual_viewport.PageScaleLayer();
-    ScopedPaintChunkProperties scale_scope(
-        context.GetPaintController(), scale_layer->GetPropertyTreeState(),
-        *scale_layer, DisplayItem::kForeignLayerWrapper);
-
-    scale_layer->CcLayer()->RemoveAllChildren();
-    RecordForeignLayer(context, DisplayItem::kForeignLayerWrapper,
-                       scale_layer->CcLayer());
-  }
-
-  // Collect the visual viewport scroll layer.
-  {
-    GraphicsLayer* scroll_layer = visual_viewport.ScrollLayer();
-    ScopedPaintChunkProperties scroll_scope(
-        context.GetPaintController(), scroll_layer->GetPropertyTreeState(),
-        *scroll_layer, DisplayItem::kForeignLayerWrapper);
-
-    scroll_layer->CcLayer()->RemoveAllChildren();
-    RecordForeignLayer(context, DisplayItem::kForeignLayerWrapper,
-                       scroll_layer->CcLayer());
-  }
+  RecordGraphicsLayerAsForeignLayer(context, visual_viewport.ContainerLayer());
+  RecordGraphicsLayerAsForeignLayer(context, visual_viewport.PageScaleLayer());
+  RecordGraphicsLayerAsForeignLayer(context, visual_viewport.ScrollLayer());
 }
 
 static void CollectDrawableLayersForLayerListRecursively(
@@ -2750,25 +2725,12 @@ static void CollectDrawableLayersForLayerListRecursively(
   // that don't for the purposes of hit testing. For example, an empty div
   // will not draw content but needs to create a layer to ensure scroll events
   // do not pass through it.
-  if (layer->DrawsContent() || layer->GetHitTestableWithoutDrawsContent()) {
-    ScopedPaintChunkProperties scope(context.GetPaintController(),
-                                     layer->GetPropertyTreeState(), *layer,
-                                     DisplayItem::kForeignLayerWrapper);
-    // TODO(trchen): Currently the GraphicsLayer hierarchy is still built
-    // during CompositingUpdate, and we have to clear them here to ensure no
-    // extraneous layers are still attached. In future we will disable all
-    // those layer hierarchy code so we won't need this line.
-    layer->CcLayer()->RemoveAllChildren();
-    RecordForeignLayer(context, DisplayItem::kForeignLayerWrapper,
-                       layer->CcLayer());
-  }
+  if (layer->DrawsContent() || layer->GetHitTestableWithoutDrawsContent())
+    RecordGraphicsLayerAsForeignLayer(context, layer);
 
   if (auto* contents_layer = layer->ContentsLayer()) {
-    ScopedPaintChunkProperties scope(
-        context.GetPaintController(), layer->GetContentsPropertyTreeState(),
-        *layer, DisplayItem::kForeignLayerContentsWrapper);
     RecordForeignLayer(context, DisplayItem::kForeignLayerContentsWrapper,
-                       contents_layer);
+                       contents_layer, layer->GetContentsPropertyTreeState());
   }
 
   DCHECK(!layer->ContentsClippingMaskLayer());
@@ -2791,11 +2753,8 @@ static void CollectLinkHighlightLayersForLayerListRecursively(
     auto* highlight_layer = highlight->Layer();
     auto property_tree_state = layer->GetPropertyTreeState();
     property_tree_state.SetEffect(highlight->effect());
-    ScopedPaintChunkProperties scope(context.GetPaintController(),
-                                     property_tree_state, *highlight,
-                                     DisplayItem::kForeignLayerLinkHighlight);
     RecordForeignLayer(context, DisplayItem::kForeignLayerLinkHighlight,
-                       highlight_layer);
+                       highlight_layer, property_tree_state);
   }
 
   for (const auto* child : layer->Children())

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

@@ -55,11 +55,9 @@
 #include "third_party/blink/renderer/platform/animation/compositor_target_property.h"
 #include "third_party/blink/renderer/platform/animation/timing_function.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_layer.h"
-#include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
 #include "third_party/blink/renderer/platform/graphics/paint/foreign_layer_display_item.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_canvas.h"
 #include "third_party/blink/renderer/platform/graphics/paint/paint_recorder.h"
-#include "third_party/blink/renderer/platform/graphics/paint/scoped_paint_chunk_properties.h"
 #include "third_party/blink/renderer/platform/web_test_support.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 #include "third_party/skia/include/core/SkMatrix44.h"
@@ -494,11 +492,8 @@ void LinkHighlightImpl::Paint(GraphicsContext& context) {
     DCHECK(fragment->PaintProperties()->LinkHighlightEffect());
     property_tree_state.SetEffect(
         fragment->PaintProperties()->LinkHighlightEffect());
-    ScopedPaintChunkProperties paint_chunk_properties(
-        context.GetPaintController(), property_tree_state, *this,
-        DisplayItem::kForeignLayerLinkHighlight);
-
-    RecordForeignLayer(context, DisplayItem::kForeignLayerLinkHighlight, layer);
+    RecordForeignLayer(context, DisplayItem::kForeignLayerLinkHighlight, layer,
+                       property_tree_state);
   }
 
   if (index < fragments_.size())

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

@@ -16,8 +16,6 @@ namespace blink {
 
 class EffectPaintPropertyNode;
 
-// TODO(wangxianzhu): Combine this class into the core implmentation for
-// CompositeAfterPaint.
 class PLATFORM_EXPORT LinkHighlight : public DisplayItemClient {
  public:
   ~LinkHighlight() override {}
@@ -29,6 +27,8 @@ class PLATFORM_EXPORT LinkHighlight : public DisplayItemClient {
   virtual const EffectPaintPropertyNode* effect() const = 0;
 
   // DisplayItemClient methods
+  // TODO(wangxianzhu): This class doesn't need to be a DisplayItemClient in
+  // CompositeAfterPaint.
   String DebugName() const final { return "LinkHighlight"; }
   LayoutRect VisualRect() const final { return LayoutRect(); }
 };

third_party/blink/renderer/platform/graphics/paint/foreign_layer_display_item.cc

@@ -84,13 +84,26 @@ void ForeignLayerDisplayItem::PropertiesAsJSON(JSONObject& json) const {
 
 void RecordForeignLayer(GraphicsContext& context,
                         DisplayItem::Type type,
-                        scoped_refptr<cc::Layer> layer) {
+                        scoped_refptr<cc::Layer> layer,
+                        const base::Optional<PropertyTreeState>& properties) {
   PaintController& paint_controller = context.GetPaintController();
   if (paint_controller.DisplayItemConstructionIsDisabled())
     return;
 
+  // This is like ScopedPaintChunkProperties but uses null id because foreign
+  // layer chunk doesn't need an id nor a client.
+  base::Optional<PropertyTreeState> previous_properties;
+  if (properties) {
+    previous_properties.emplace(paint_controller.CurrentPaintChunkProperties());
+    paint_controller.UpdateCurrentPaintChunkProperties(base::nullopt,
+                                                       *properties);
+  }
   paint_controller.CreateAndAppend<ForeignLayerDisplayItem>(type,
                                                             std::move(layer));
+  if (properties) {
+    paint_controller.UpdateCurrentPaintChunkProperties(base::nullopt,
+                                                       *previous_properties);
+  }
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/paint/foreign_layer_display_item.h

@@ -7,6 +7,7 @@
 
 #include "cc/layers/layer.h"
 #include "third_party/blink/renderer/platform/graphics/paint/display_item.h"
+#include "third_party/blink/renderer/platform/graphics/paint/property_tree_state.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 
 namespace blink {
@@ -39,9 +40,11 @@ class PLATFORM_EXPORT ForeignLayerDisplayItem final : public DisplayItem {
 
 // Records a foreign layer into a GraphicsContext.
 // Use this where you would use a recorder class.
-PLATFORM_EXPORT void RecordForeignLayer(GraphicsContext&,
-                                        DisplayItem::Type,
-                                        scoped_refptr<cc::Layer>);
+PLATFORM_EXPORT void RecordForeignLayer(
+    GraphicsContext&,
+    DisplayItem::Type,
+    scoped_refptr<cc::Layer>,
+    const base::Optional<PropertyTreeState>& = base::nullopt);
 
 }  // namespace blink