[BlinkGenPropertyTrees] Add layer state for viewport scrollbars

This patch fixes viewport scrollbars flashing page issue. The issue
is causing by it has incorrect effect node. In this patch, we create
the effect and correct the position for viewport scrollbars.

Bug: 870520
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I954d6cddb36d811f693b597644b65ee175137dcf
Reviewed-on: https://chromium-review.googlesource.com/1172919
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583323}

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

@@ -49,6 +49,7 @@
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.h"
 #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
+#include "third_party/blink/renderer/core/paint/paint_property_tree_builder.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/core/scroll/scroll_animator_base.h"
 #include "third_party/blink/renderer/core/scroll/scrollbar.h"
@@ -56,6 +57,9 @@
 #include "third_party/blink/renderer/platform/geometry/double_rect.h"
 #include "third_party/blink/renderer/platform/geometry/float_size.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_layer.h"
+#include "third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.h"
+#include "third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h"
+#include "third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h"
 #include "third_party/blink/renderer/platform/histogram.h"
 #include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
 
@@ -84,8 +88,11 @@ ScrollPaintPropertyNode* VisualViewport::GetScrollNode() const {
 }
 
 void VisualViewport::UpdatePaintPropertyNodes(
-    scoped_refptr<const TransformPaintPropertyNode> transform_parent,
-    scoped_refptr<const ScrollPaintPropertyNode> scroll_parent) {
+    PaintPropertyTreeBuilderFragmentContext& context) {
+  auto* transform_parent = context.current.transform;
+  auto* scroll_parent = context.current.scroll;
+  auto* effect_parent = context.current_effect;
+
   DCHECK(transform_parent);
   DCHECK(scroll_parent);
 
@@ -156,6 +163,49 @@ void VisualViewport::UpdatePaintPropertyNodes(
     }
   }
 
+  if (overlay_scrollbar_horizontal_) {
+    EffectPaintPropertyNode::State state;
+    state.local_transform_space = transform_parent;
+    state.direct_compositing_reasons =
+        CompositingReason::kActiveOpacityAnimation;
+    state.compositor_element_id =
+        GetScrollbarElementId(ScrollbarOrientation::kHorizontalScrollbar);
+    if (!horizontal_scrollbar_effect_node_) {
+      horizontal_scrollbar_effect_node_ =
+          EffectPaintPropertyNode::Create(*effect_parent, std::move(state));
+    } else {
+      horizontal_scrollbar_effect_node_->Update(*effect_parent,
+                                                std::move(state));
+    }
+
+    overlay_scrollbar_horizontal_->SetLayerState(
+        PropertyTreeState(transform_parent, context.current.clip,
+                          horizontal_scrollbar_effect_node_.get()),
+        IntPoint(overlay_scrollbar_horizontal_->GetPosition().X(),
+                 overlay_scrollbar_horizontal_->GetPosition().Y()));
+  }
+
+  if (overlay_scrollbar_vertical_) {
+    EffectPaintPropertyNode::State state;
+    state.local_transform_space = transform_parent;
+    state.direct_compositing_reasons =
+        CompositingReason::kActiveOpacityAnimation;
+    state.compositor_element_id =
+        GetScrollbarElementId(ScrollbarOrientation::kVerticalScrollbar);
+    if (!vertical_scrollbar_effect_node_) {
+      vertical_scrollbar_effect_node_ =
+          EffectPaintPropertyNode::Create(*effect_parent, std::move(state));
+    } else {
+      vertical_scrollbar_effect_node_->Update(*effect_parent, std::move(state));
+    }
+
+    overlay_scrollbar_vertical_->SetLayerState(
+        PropertyTreeState(transform_parent, context.current.clip,
+                          vertical_scrollbar_effect_node_.get()),
+        IntPoint(overlay_scrollbar_vertical_->GetPosition().X(),
+                 overlay_scrollbar_vertical_->GetPosition().Y()));
+  }
+
   if (inner_viewport_scroll_layer_) {
     inner_viewport_scroll_layer_->SetLayerState(
         PropertyTreeState(translation_transform_node_.get(),
@@ -507,27 +557,6 @@ void VisualViewport::InitializeScrollbars() {
 
   if (VisualViewportSuppliesScrollbars() &&
       !GetPage().GetSettings().GetHideScrollbars()) {
-    if (!overlay_scrollbar_horizontal_->Parent()) {
-      inner_viewport_container_layer_->AddChild(
-          overlay_scrollbar_horizontal_.get());
-      if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
-        // TODO(pdr): The viewport overlay scrollbars do not have the correct
-        // paint properties. See: https://crbug.com/836910
-        overlay_scrollbar_horizontal_->SetLayerState(
-            PropertyTreeState(PropertyTreeState::Root()), IntPoint());
-      }
-    }
-    if (!overlay_scrollbar_vertical_->Parent()) {
-      inner_viewport_container_layer_->AddChild(
-          overlay_scrollbar_vertical_.get());
-      if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
-        // TODO(pdr): The viewport overlay scrollbars do not have the correct
-        // paint properties. See: https://crbug.com/836910
-        overlay_scrollbar_vertical_->SetLayerState(
-            PropertyTreeState(PropertyTreeState::Root()), IntPoint());
-      }
-    }
-
     SetupScrollbar(kHorizontalScrollbar);
     SetupScrollbar(kVerticalScrollbar);
   } else {
@@ -551,7 +580,11 @@ void VisualViewport::SetupScrollbar(ScrollbarOrientation orientation) {
   std::unique_ptr<ScrollingCoordinator::ScrollbarLayerGroup>&
       scrollbar_layer_group = is_horizontal ? scrollbar_layer_group_horizontal_
                                             : scrollbar_layer_group_vertical_;
-
+  if (!scrollbar_graphics_layer->Parent()) {
+    inner_viewport_container_layer_->AddChild(scrollbar_graphics_layer);
+    scrollbar_graphics_layer->SetLayerState(
+        PropertyTreeState(PropertyTreeState::Root()), IntPoint());
+  }
   ScrollbarThemeOverlay& theme = ScrollbarThemeOverlay::MobileTheme();
   int thumb_thickness = clampTo<int>(
       std::floor(GetPage().GetChromeClient().WindowToViewportScalar(

third_party/blink/renderer/core/frame/visual_viewport.h

@@ -54,8 +54,10 @@ class IntRect;
 class IntSize;
 class LocalFrame;
 class Page;
+class EffectPaintPropertyNode;
 class ScrollPaintPropertyNode;
 class TransformPaintPropertyNode;
+struct PaintPropertyTreeBuilderFragmentContext;
 
 // Represents the visual viewport the user is currently seeing the page through.
 // This class corresponds to the InnerViewport on the compositor. It is a
@@ -261,8 +263,7 @@ class CORE_EXPORT VisualViewport final
   // container, page scale layer, inner viewport scroll layer) to reference
   // these nodes.
   void UpdatePaintPropertyNodes(
-      scoped_refptr<const TransformPaintPropertyNode> transform_parent,
-      scoped_refptr<const ScrollPaintPropertyNode> scroll_parent);
+      PaintPropertyTreeBuilderFragmentContext& context);
 
  private:
   explicit VisualViewport(Page&);
@@ -331,6 +332,8 @@ class CORE_EXPORT VisualViewport final
   scoped_refptr<TransformPaintPropertyNode> scale_transform_node_;
   scoped_refptr<TransformPaintPropertyNode> translation_transform_node_;
   scoped_refptr<ScrollPaintPropertyNode> scroll_node_;
+  scoped_refptr<EffectPaintPropertyNode> horizontal_scrollbar_effect_node_;
+  scoped_refptr<EffectPaintPropertyNode> vertical_scrollbar_effect_node_;
 
   // Offset of the visual viewport from the main frame's origin, in CSS pixels.
   ScrollOffset offset_;

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

@@ -40,6 +40,7 @@
 #include "third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.h"
 #include "third_party/blink/renderer/core/paint/paint_layer.h"
 #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
+#include "third_party/blink/renderer/core/scroll/scrollbar_theme_overlay.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_request.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_test.h"
 #include "third_party/blink/renderer/platform/geometry/double_point.h"
@@ -2232,6 +2233,49 @@ TEST_P(VisualViewportTest, InvalidateLayoutViewWhenDocumentSmallerThanView) {
   document->View()->SetTracksPaintInvalidations(false);
 }
 
+// Ensure we create effect node for scrollbar properly.
+TEST_P(VisualViewportTest, EnsureEffectNodeForScrollbars) {
+  if (!RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
+    return;
+
+  InitializeWithAndroidSettings();
+  WebView()->Resize(IntSize(400, 400));
+  NavigateTo("about:blank");
+  WebView()->UpdateAllLifecyclePhases();
+
+  VisualViewport& visual_viewport = GetFrame()->GetPage()->GetVisualViewport();
+  auto* vertical_scrollbar = visual_viewport.LayerForVerticalScrollbar();
+  auto* horizontal_scrollbar = visual_viewport.LayerForHorizontalScrollbar();
+  ASSERT_TRUE(vertical_scrollbar);
+  ASSERT_TRUE(horizontal_scrollbar);
+
+  auto& vertical_scrollbar_state = vertical_scrollbar->GetPropertyTreeState();
+  auto& horizontal_scrollbar_state =
+      horizontal_scrollbar->GetPropertyTreeState();
+
+  ScrollbarThemeOverlay& theme = ScrollbarThemeOverlay::MobileTheme();
+  int scrollbar_thickness = clampTo<int>(std::floor(
+      GetFrame()->GetPage()->GetChromeClient().WindowToViewportScalar(
+          theme.ScrollbarThickness(kRegularScrollbar))));
+
+  EXPECT_TRUE(vertical_scrollbar_state.Effect());
+  EXPECT_EQ(vertical_scrollbar_state.Effect()->GetCompositorElementId(),
+            visual_viewport.GetScrollbarElementId(
+                ScrollbarOrientation::kVerticalScrollbar));
+  EXPECT_EQ(vertical_scrollbar->GetOffsetFromTransformNode(),
+            IntPoint(400 - scrollbar_thickness, 0));
+
+  EXPECT_TRUE(horizontal_scrollbar_state.Effect());
+  EXPECT_EQ(horizontal_scrollbar_state.Effect()->GetCompositorElementId(),
+            visual_viewport.GetScrollbarElementId(
+                ScrollbarOrientation::kHorizontalScrollbar));
+  EXPECT_EQ(horizontal_scrollbar->GetOffsetFromTransformNode(),
+            IntPoint(0, 400 - scrollbar_thickness));
+
+  EXPECT_EQ(vertical_scrollbar_state.Effect()->Parent(),
+            horizontal_scrollbar_state.Effect()->Parent());
+}
+
 // Make sure we don't crash when the visual viewport's height is 0. This can
 // happen transiently in autoresize mode and cause a crash. This test passes if
 // it doesn't crash.

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

@@ -61,8 +61,7 @@ void VisualViewportPaintPropertyTreeBuilder::Update(
 
   PaintPropertyTreeBuilderFragmentContext& context = full_context.fragments[0];
 
-  visual_viewport.UpdatePaintPropertyNodes(context.current.transform,
-                                           context.current.scroll);
+  visual_viewport.UpdatePaintPropertyNodes(context);
 
   context.current.transform = visual_viewport.GetScrollTranslationNode();
   context.absolute_position.transform =