Remove indirection: setup scrollbar scroll layers in the scrollbar constructor

third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp

Index: third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
diff --git a/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp b/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
index 970e26a12dd3a76062c2a489a7c45ecbf0bbde5c..55967dcecf10982defa3dcdc4920bf1a23ca3aa1 100644
--- a/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
+++ b/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
@@ -339,7 +339,8 @@ void ScrollingCoordinator::removeWebScrollbarLayer(
 
 static std::unique_ptr<WebScrollbarLayer> createScrollbarLayer(
     Scrollbar& scrollbar,
-    float deviceScaleFactor) {
+    float deviceScaleFactor,
+    WebLayer* scrollLayer) {
   ScrollbarTheme& theme = scrollbar.theme();
   WebScrollbarThemePainter painter(theme, scrollbar, deviceScaleFactor);
   std::unique_ptr<WebScrollbarThemeGeometry> geometry(
@@ -349,11 +350,13 @@ static std::unique_ptr<WebScrollbarLayer> createScrollbarLayer(
   if (theme.usesOverlayScrollbars() && theme.usesNinePatchThumbResource()) {
     scrollbarLayer =
         Platform::current()->compositorSupport()->createOverlayScrollbarLayer(
-            WebScrollbarImpl::create(&scrollbar), painter, std::move(geometry));
+            WebScrollbarImpl::create(&scrollbar), painter, std::move(geometry),
+            scrollLayer);
   } else {
     scrollbarLayer =
         Platform::current()->compositorSupport()->createScrollbarLayer(
-            WebScrollbarImpl::create(&scrollbar), painter, std::move(geometry));
+            WebScrollbarImpl::create(&scrollbar), painter, std::move(geometry),
+            scrollLayer);
   }
   GraphicsLayer::registerContentsLayer(scrollbarLayer->layer());
   return scrollbarLayer;
@@ -364,14 +367,15 @@ ScrollingCoordinator::createSolidColorScrollbarLayer(
     ScrollbarOrientation orientation,
     int thumbThickness,
     int trackStart,
-    bool isLeftSideVerticalScrollbar) {
+    bool isLeftSideVerticalScrollbar,
+    WebLayer* scrollLayer) {
   WebScrollbar::Orientation webOrientation =
       (orientation == HorizontalScrollbar) ? WebScrollbar::Horizontal
                                            : WebScrollbar::Vertical;
   std::unique_ptr<WebScrollbarLayer> scrollbarLayer =
       Platform::current()->compositorSupport()->createSolidColorScrollbarLayer(
           webOrientation, thumbThickness, trackStart,
-          isLeftSideVerticalScrollbar);
+          isLeftSideVerticalScrollbar, scrollLayer);
   GraphicsLayer::registerContentsLayer(scrollbarLayer->layer());
   return scrollbarLayer;
 }
@@ -393,7 +397,6 @@ static void setupScrollbarLayer(GraphicsLayer* scrollbarGraphicsLayer,
     detachScrollbarLayer(scrollbarGraphicsLayer);
     return;
   }
-  scrollbarLayer->setScrollLayer(scrollLayer);
   scrollbarGraphicsLayer->setContentsToPlatformLayer(scrollbarLayer->layer());
   scrollbarGraphicsLayer->setDrawsContent(false);
 }
@@ -445,6 +448,13 @@ void ScrollingCoordinator::scrollableAreaScrollbarLayerDidChange(
     // scrollbar becomes a non-custom one.
     scrollbarGraphicsLayer->platformLayer()->clearMainThreadScrollingReasons(
         MainThreadScrollingReason::kCustomScrollbarScrolling);
+
+    WebLayer* scrollLayer = toWebLayer(scrollableArea->layerForScrolling());
+    if (!scrollLayer) {
+      detachScrollbarLayer(scrollbarGraphicsLayer);

[bokan, 2017/03/06 14:36:34]

Why do we need to do this here now? setupScrollbarLayer will do this, do we need
to avoid the code below in this case?

[pdr., 2017/03/06 19:00:24]

Before this patch, this function called detachScrollbarLayer in both of
"important" return codepaths: the early-out a few lines up for custom
scrollbars, and in setupScrollbarLayer. This patch can skip even creating the
scrollbar layers but I didn't want to change the detachScrollbarLayer logic.

We could remove the DCHECK in setupScrollbarLayer that checks for a scrollbar
layer and call setupScrollbarLayer. I think it's a little cleaner to just make
the early-out behavior more obvious. I don't really think we even need to call
detachScrollbarLayer but I didn't want to tackle that change in this patch.

[bokan, 2017/03/06 21:49:29]

Ok, thanks for the explanation. 

+      return;
+    }
+
     WebScrollbarLayer* scrollbarLayer =
         getWebScrollbarLayer(scrollableArea, orientation);
     if (!scrollbarLayer) {
@@ -456,16 +466,15 @@ void ScrollingCoordinator::scrollableAreaScrollbarLayerDidChange(
         webScrollbarLayer = createSolidColorScrollbarLayer(
             orientation, scrollbar.theme().thumbThickness(scrollbar),
             scrollbar.theme().trackPosition(scrollbar),
-            scrollableArea->shouldPlaceVerticalScrollbarOnLeft());
+            scrollableArea->shouldPlaceVerticalScrollbarOnLeft(), scrollLayer);
       } else {
         webScrollbarLayer = createScrollbarLayer(
-            scrollbar, m_page->deviceScaleFactorDeprecated());
+            scrollbar, m_page->deviceScaleFactorDeprecated(), scrollLayer);
       }
       scrollbarLayer = addWebScrollbarLayer(scrollableArea, orientation,
                                             std::move(webScrollbarLayer));
     }
 
-    WebLayer* scrollLayer = toWebLayer(scrollableArea->layerForScrolling());
     setupScrollbarLayer(scrollbarGraphicsLayer, scrollbarLayer, scrollLayer);
 
     // Root layer non-overlay scrollbars should be marked opaque to disable