Remove indirection: setup scrollbar scroll layers in the scrollbar constructor

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

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 321 matching lines @@
   ScrollbarMap& scrollbars = orientation == HorizontalScrollbar
                                  ? m_horizontalScrollbars
                                  : m_verticalScrollbars;
   if (std::unique_ptr<WebScrollbarLayer> scrollbarLayer =
           scrollbars.take(scrollableArea))
     GraphicsLayer::unregisterContentsLayer(scrollbarLayer->layer());
 }
 
 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(
       WebScrollbarThemeGeometryNative::create(theme));
 
   std::unique_ptr<WebScrollbarLayer> scrollbarLayer;
   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;
 }
 
 std::unique_ptr<WebScrollbarLayer>
 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;
 }
 
 static void detachScrollbarLayer(GraphicsLayer* scrollbarGraphicsLayer) {
   DCHECK(scrollbarGraphicsLayer);
 
   scrollbarGraphicsLayer->setContentsToPlatformLayer(nullptr);
   scrollbarGraphicsLayer->setDrawsContent(true);
 }
 
 static void setupScrollbarLayer(GraphicsLayer* scrollbarGraphicsLayer,
                                 WebScrollbarLayer* scrollbarLayer,
                                 WebLayer* scrollLayer) {
   DCHECK(scrollbarGraphicsLayer);
   DCHECK(scrollbarLayer);
 
   if (!scrollLayer) {
     detachScrollbarLayer(scrollbarGraphicsLayer);
     return;
   }
-  scrollbarLayer->setScrollLayer(scrollLayer);
   scrollbarGraphicsLayer->setContentsToPlatformLayer(scrollbarLayer->layer());
   scrollbarGraphicsLayer->setDrawsContent(false);
 }
 
 WebScrollbarLayer* ScrollingCoordinator::addWebScrollbarLayer(
     ScrollableArea* scrollableArea,
     ScrollbarOrientation orientation,
     std::unique_ptr<WebScrollbarLayer> scrollbarLayer) {
   ScrollbarMap& scrollbars = orientation == HorizontalScrollbar
                                  ? m_horizontalScrollbars
@@ skipping 31 matching lines @@
       detachScrollbarLayer(scrollbarGraphicsLayer);
       scrollbarGraphicsLayer->platformLayer()->addMainThreadScrollingReasons(
           MainThreadScrollingReason::kCustomScrollbarScrolling);
       return;
     }
 
     // Invalidate custom scrollbar scrolling reason in case a custom
     // 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) {
       Settings* settings = m_page->mainFrame()->settings();
 
       std::unique_ptr<WebScrollbarLayer> webScrollbarLayer;
       if (settings->getUseSolidColorScrollbars()) {
         DCHECK(RuntimeEnabledFeatures::overlayScrollbarsEnabled());
         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
     // blending.
     bool isOpaqueScrollbar = !scrollbar.isOverlayScrollbar();
     scrollbarGraphicsLayer->setContentsOpaque(isMainFrame && isOpaqueScrollbar);
   } else {
     removeWebScrollbarLayer(scrollableArea, orientation);
   }
 }
@@ skipping 716 matching lines @@
           frameView ? toWebLayer(frameView->layoutViewportScrollableArea()
                                      ->layerForScrolling())
                     : nullptr) {
     return WebSize(frameView->layoutViewportScrollableArea()->contentsSize()) !=
            scrollLayer->bounds();
   }
   return false;
 }
 
 }  // namespace blink