[Refactor] Moved scrollbar creation and deletion to ScrollbarManager

third_party/WebKit/Source/core/paint/ScrollbarManager.cpp

Index: third_party/WebKit/Source/core/paint/ScrollbarManager.cpp
diff --git a/third_party/WebKit/Source/core/paint/ScrollbarManager.cpp b/third_party/WebKit/Source/core/paint/ScrollbarManager.cpp
index 4b270c2fdc07abcc787922c4bb576e195c49fb24..938caa45c81331cf17f7e30216e6eeafbaf0bc02 100644
--- a/third_party/WebKit/Source/core/paint/ScrollbarManager.cpp
+++ b/third_party/WebKit/Source/core/paint/ScrollbarManager.cpp
@@ -4,6 +4,16 @@
 
 #include "core/paint/ScrollbarManager.h"
 
+#include "core/frame/LocalFrame.h"
+#include "core/frame/LocalFrameView.h"
+#include "core/layout/LayoutBox.h"
+#include "core/layout/LayoutObject.h"
+#include "core/layout/LayoutScrollbar.h"
+#include "core/layout/LayoutTheme.h"
+#include "core/page/ChromeClient.h"
+#include "core/page/Page.h"
+#include "core/paint/PaintLayerScrollableArea.h"
+
 namespace blink {
 
 ScrollbarManager::ScrollbarManager(ScrollableArea& scrollable_area)
@@ -23,4 +33,56 @@ void ScrollbarManager::Dispose() {
   DestroyScrollbar(kVerticalScrollbar);
 }
 
+Scrollbar* ScrollbarManager::CreateScrollbar(ScrollbarOrientation orientation) {

[szager1, 2017/06/27 20:42:13]

All of this logic could be moved into PLSA and FrameView, to allow moving
ScrollbarManager into platform.

+  DCHECK(scrollable_area_->GetLayoutBox());
+  DCHECK(orientation == kHorizontalScrollbar ? !h_bar_is_attached_
+                                             : !v_bar_is_attached_);
+  LayoutObject* actual_layout_object = nullptr;

[bokan, 2017/06/27 17:48:52]

layout_object_for_style

+
+  Scrollbar* scrollbar = nullptr;
+  if (scrollable_area_->ShouldUseCustomScrollbars(actual_layout_object)) {
+    scrollbar = LayoutScrollbar::CreateCustomScrollbar(
+        scrollable_area_.Get(), orientation,
+        ToElement(actual_layout_object->GetNode()));

[bokan, 2017/06/27 17:48:52]

Add a DCHECK that if we're using custom scrollbars, the layout object isn't a
LayoutView.

+  } else {
+    ScrollbarControlSize scrollbar_size = kRegularScrollbar;
+
+    if (actual_layout_object &&
+        actual_layout_object->StyleRef().HasAppearance() &&
+        scrollable_area_->IsPaintLayerScrollableArea()) {
+      scrollbar_size = LayoutTheme::GetTheme().ScrollbarControlSizeForPart(
+          actual_layout_object->StyleRef().Appearance());
+    }
+
+    scrollbar =
+        Scrollbar::Create(scrollable_area_.Get(), orientation, scrollbar_size,
+                          &scrollable_area_->GetLayoutBox()
+                               ->GetFrame()
+                               ->GetPage()
+                               ->GetChromeClient());
+  }
+
+  scrollable_area_->GetLayoutBox()->GetDocument().View()->AddScrollbar(
+      scrollbar);
+
+  return scrollbar;
+}
+
+void ScrollbarManager::DestroyScrollbar(ScrollbarOrientation orientation) {
+  Member<Scrollbar>& scrollbar =
+      orientation == kHorizontalScrollbar ? h_bar_ : v_bar_;
+  DCHECK(orientation == kHorizontalScrollbar ? !h_bar_is_attached_
+                                             : !v_bar_is_attached_);
+  if (!scrollbar)
+    return;
+
+  scrollable_area_->WillRemoveScrollbar(*scrollbar, orientation);

[bokan, 2017/06/27 17:48:52]

This used to only be called only for non-custom scrollbars but I think it should
be fine to call for custom too (and better to do that). Could you please just
confirm that this doesn't blow up in an obvious way for custom scrollbars?

+
+  scrollable_area_->GetLayoutBox()->GetDocument().View()->RemoveScrollbar(
+      scrollbar);
+
+  scrollbar->DisconnectFromScrollableArea();
+  scrollbar = nullptr;
+}
+
 }  // namespace blink