Remove ScrollbarThemeMock and refactor scrollbar themes

https://chromium-review.googlesource.com/c/chromium/src/+/1866913 removed usage of ScrollbarThemeMock from web tests. This CL removes usage of it from unit tests, and removes ScrollbarThemeMock. It also removes the additional level of mock theme in blink, and refactors scrollbar themes and their settings to avoid misuse of scrollbars. See https://chromium-review.googlesource.com/c/chromium/src/+/1866913 for the new rule of testing scrollbars that now also apply to unit tests. Change-Id: Ibd0566fea55c6b89c767cffa311df00e235d2478 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879650 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#712008}
chromium · Nov 2, 2019 · 4b043ef · 4b043ef
1 parent 5123ac8
commit 4b043ef
Show file tree

Hide file tree

Showing 49 changed files with 651 additions and 601 deletions.
diff --git a/third_party/blink/renderer/core/BUILD.gn b/third_party/blink/renderer/core/BUILD.gn
@@ -300,6 +300,7 @@ jumbo_source_set("testing") {
     "testing/origin_trials_test_partial.h",
     "testing/record_test.cc",
     "testing/record_test.h",
+    "testing/scoped_mock_overlay_scrollbars.h",
     "testing/sequence_test.cc",
     "testing/sequence_test.h",
     "testing/static_selection.cc",
@@ -309,7 +310,6 @@ jumbo_source_set("testing") {
     "testing/type_conversions.h",
     "testing/union_types_test.cc",
     "testing/union_types_test.h",
-    "testing/use_mock_scrollbar_settings.h",
     "testing/v8/web_core_test_support.cc",
     "testing/v8/web_core_test_support.h",
     "testing/wait_for_event.cc",

diff --git a/third_party/blink/renderer/core/dom/document_test.cc b/third_party/blink/renderer/core/dom/document_test.cc
@@ -62,6 +62,7 @@
 #include "third_party/blink/renderer/core/page/validation_message_client.h"
 #include "third_party/blink/renderer/core/testing/color_scheme_helper.h"
 #include "third_party/blink/renderer/core/testing/page_test_base.h"
+#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/scheduler/public/event_loop.h"
@@ -1120,7 +1121,7 @@ TEST_F(DocumentTest, CanExecuteScriptsWithSandboxAndIsolatedWorld) {
 TEST_F(DocumentTest, ElementFromPointOnScrollbar) {
   GetDocument().SetCompatibilityMode(Document::kQuirksMode);
   // This test requires that scrollbars take up space.
-  ScopedOverlayScrollbarsForTest no_overlay_scrollbars(false);
+  ScopedMockOverlayScrollbars no_overlay_scrollbars(false);
 
   SetHtmlInnerHTML(R"HTML(
     <style>
@@ -1148,7 +1149,7 @@ TEST_F(DocumentTest, ElementFromPointOnScrollbar) {
 TEST_F(DocumentTest, ElementFromPointWithPageZoom) {
   GetDocument().SetCompatibilityMode(Document::kQuirksMode);
   // This test requires that scrollbars take up space.
-  ScopedOverlayScrollbarsForTest no_overlay_scrollbars(false);
+  ScopedMockOverlayScrollbars no_overlay_scrollbars(false);
 
   SetHtmlInnerHTML(R"HTML(
     <style>

diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
@@ -146,6 +146,7 @@
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/core/resize_observer/resize_observation.h"
 #include "third_party/blink/renderer/core/scroll/scrollable_area.h"
+#include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
 #include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
 #include "third_party/blink/renderer/core/svg/svg_a_element.h"
 #include "third_party/blink/renderer/core/svg/svg_element.h"
@@ -1200,7 +1201,8 @@ int Element::clientWidth() {
       (in_quirks_mode && IsHTMLElement() && GetDocument().body() == this)) {
     auto* layout_view = GetDocument().GetLayoutView();
     if (layout_view) {
-      if (!RuntimeEnabledFeatures::OverlayScrollbarsEnabled() ||
+      // TODO(crbug.com/740879): Use per-page overlay scrollbar settings.
+      if (!ScrollbarThemeSettings::OverlayScrollbarsEnabled() ||
           !GetDocument().GetFrame()->IsLocalRoot())
         GetDocument().UpdateStyleAndLayoutForNode(this);
       if (GetDocument().GetPage()->GetSettings().GetForceZeroLayoutHeight())
@@ -1238,7 +1240,8 @@ int Element::clientHeight() {
       (in_quirks_mode && IsHTMLElement() && GetDocument().body() == this)) {
     auto* layout_view = GetDocument().GetLayoutView();
     if (layout_view) {
-      if (!RuntimeEnabledFeatures::OverlayScrollbarsEnabled() ||
+      // TODO(crbug.com/740879): Use per-page overlay scrollbar settings.
+      if (!ScrollbarThemeSettings::OverlayScrollbarsEnabled() ||
           !GetDocument().GetFrame()->IsLocalRoot())
         GetDocument().UpdateStyleAndLayoutForNode(this);
       if (GetDocument().GetPage()->GetSettings().GetForceZeroLayoutHeight())

diff --git a/third_party/blink/renderer/core/editing/compute_layer_selection_test.cc b/third_party/blink/renderer/core/editing/compute_layer_selection_test.cc
@@ -15,7 +15,6 @@
 #include "third_party/blink/renderer/core/layout/layout_box.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/testing/use_mock_scrollbar_settings.h"
 
 namespace blink {
 
@@ -41,9 +40,6 @@ class ComputeLayerSelectionTest : public EditingTestBase {
   void FocusAndSelectAll(TextControlElement* target) {
     FocusAndSelectAll(target, *target->InnerEditorElement());
   }
-
- private:
-  UseMockScrollbarSettings mock_scrollbars_;
 };
 
 TEST_F(ComputeLayerSelectionTest, ComputeLayerSelection) {

diff --git a/third_party/blink/renderer/core/frame/browser_controls_test.cc b/third_party/blink/renderer/core/frame/browser_controls_test.cc
@@ -48,6 +48,7 @@
 #include "third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.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/testing/scoped_mock_overlay_scrollbars.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/testing/testing_platform_support.h"
@@ -59,7 +60,8 @@ namespace blink {
 // These tests cover browser controls scrolling on main-thread.
 // The animation for completing a partial show/hide is done in compositor so
 // it is not covered here.
-class BrowserControlsTest : public testing::Test {
+class BrowserControlsTest : public testing::Test,
+                            public ScopedMockOverlayScrollbars {
  public:
   BrowserControlsTest() : base_url_("http://www.test.com/") {
     RegisterMockedHttpURLLoad("large-div.html");

diff --git a/third_party/blink/renderer/core/frame/frame_test_helpers.h b/third_party/blink/renderer/core/frame/frame_test_helpers.h
@@ -59,7 +59,7 @@
 #include "third_party/blink/renderer/core/exported/web_view_impl.h"
 #include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
-#include "third_party/blink/renderer/core/testing/use_mock_scrollbar_settings.h"
+#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
@@ -309,7 +309,7 @@ class TestWebViewClient : public WebViewClient {
 
 // Convenience class for handling the lifetime of a WebView and its associated
 // mainframe in tests.
-class WebViewHelper {
+class WebViewHelper : public ScopedMockOverlayScrollbars {
   USING_FAST_MALLOC(WebViewHelper);
 
  public:
@@ -392,7 +392,6 @@ class WebViewHelper {
   bool viewport_enabled_ = false;
 
   WebViewImpl* web_view_;
-  UseMockScrollbarSettings mock_scrollbar_settings_;
 
   std::unique_ptr<TestWebViewClient> owned_test_web_view_client_;
   TestWebViewClient* test_web_view_client_ = nullptr;

diff --git a/third_party/blink/renderer/core/frame/root_frame_viewport_test.cc b/third_party/blink/renderer/core/frame/root_frame_viewport_test.cc
@@ -12,17 +12,13 @@
 #include "third_party/blink/renderer/core/scroll/scroll_alignment.h"
 #include "third_party/blink/renderer/core/scroll/scroll_types.h"
 #include "third_party/blink/renderer/core/scroll/scrollable_area.h"
-#include "third_party/blink/renderer/core/scroll/scrollbar_theme_mock.h"
+#include "third_party/blink/renderer/core/scroll/scrollbar_theme_overlay_mock.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
 #include "third_party/blink/renderer/platform/geometry/double_rect.h"
 #include "third_party/blink/renderer/platform/geometry/layout_rect.h"
 #include "third_party/blink/renderer/platform/scheduler/public/thread.h"
 #include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
 
-namespace {
-blink::ScrollbarThemeMock scrollbar_theme_;
-}
-
 namespace blink {
 
 class ScrollableAreaStub : public GarbageCollected<ScrollableAreaStub>,
@@ -83,7 +79,8 @@ class ScrollableAreaStub : public GarbageCollected<ScrollableAreaStub>,
   }
 
   ScrollbarTheme& GetPageScrollbarTheme() const override {
-    return scrollbar_theme_;
+    DEFINE_STATIC_LOCAL(ScrollbarThemeOverlayMock, theme, ());
+    return theme;
   }
 
   void Trace(blink::Visitor* visitor) override {

diff --git a/third_party/blink/renderer/core/frame/visual_viewport.cc b/third_party/blink/renderer/core/frame/visual_viewport.cc
@@ -59,7 +59,7 @@
 #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"
-#include "third_party/blink/renderer/core/scroll/scrollbar_theme_overlay.h"
+#include "third_party/blink/renderer/core/scroll/scrollbar_theme_overlay_mobile.h"
 #include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
 #include "third_party/blink/renderer/platform/geometry/double_rect.h"
 #include "third_party/blink/renderer/platform/geometry/float_size.h"
@@ -656,7 +656,7 @@ void VisualViewport::InitializeScrollbars() {
 }
 
 int VisualViewport::ScrollbarThickness() const {
-  ScrollbarThemeOverlay& theme = ScrollbarThemeOverlay::MobileTheme();
+  auto& theme = ScrollbarThemeOverlayMobile::GetInstance();
   int thickness = theme.ScrollbarThickness(kRegularScrollbar);
   return clampTo<int>(
       std::floor(GetPage().GetChromeClient().WindowToViewportScalar(
@@ -690,7 +690,7 @@ void VisualViewport::SetupScrollbar(ScrollbarOrientation orientation) {
     ScrollingCoordinator* coordinator = GetPage().GetScrollingCoordinator();
     DCHECK(coordinator);
 
-    ScrollbarThemeOverlay& theme = ScrollbarThemeOverlay::MobileTheme();
+    auto& theme = ScrollbarThemeOverlayMobile::GetInstance();
     int thumb_thickness = clampTo<int>(
         std::floor(GetPage().GetChromeClient().WindowToViewportScalar(
             MainFrame(), theme.ThumbThickness())));

diff --git a/third_party/blink/renderer/core/frame/visual_viewport_test.cc b/third_party/blink/renderer/core/frame/visual_viewport_test.cc
@@ -42,7 +42,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/scroll/scrollbar_theme_overlay_mobile.h"
 #include "third_party/blink/renderer/core/scroll/smooth_scroll_sequencer.h"
 #include "third_party/blink/renderer/core/testing/color_scheme_helper.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_request.h"
@@ -259,7 +259,7 @@ TEST_P(VisualViewportTest, TestResize) {
 // Make sure that the visibleContentRect method acurately reflects the scale and
 // scroll location of the viewport with and without scrollbars.
 TEST_P(VisualViewportTest, TestVisibleContentRect) {
-  ScopedOverlayScrollbarsForTest overlay_scrollbars(false);
+  USE_NON_OVERLAY_SCROLLBARS();
   InitializeWithDesktopSettings();
 
   RegisterMockedHttpURLLoad("200-by-300.html");
@@ -2426,7 +2426,7 @@ TEST_P(VisualViewportTest, EnsureEffectNodeForScrollbars) {
   auto& horizontal_scrollbar_state =
       horizontal_scrollbar->GetPropertyTreeState();
 
-  ScrollbarThemeOverlay& theme = ScrollbarThemeOverlay::MobileTheme();
+  auto& theme = ScrollbarThemeOverlayMobile::GetInstance();
   int scrollbar_thickness = clampTo<int>(std::floor(
       GetFrame()->GetPage()->GetChromeClient().WindowToViewportScalar(
           GetFrame(), theme.ScrollbarThickness(kRegularScrollbar))));

diff --git a/third_party/blink/renderer/core/inspector/dev_tools_emulator.cc b/third_party/blink/renderer/core/inspector/dev_tools_emulator.cc
@@ -15,6 +15,7 @@
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
 #include "third_party/blink/renderer/core/input/event_handler.h"
 #include "third_party/blink/renderer/core/page/page.h"
+#include "third_party/blink/renderer/core/scroll/scrollbar_theme.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/geometry/float_rect.h"
 #include "third_party/blink/renderer/platform/geometry/float_size.h"
@@ -291,8 +292,8 @@ void DevToolsEmulator::EnableMobileEmulation() {
     return;
   emulate_mobile_enabled_ = true;
   is_overlay_scrollbars_enabled_ =
-      RuntimeEnabledFeatures::OverlayScrollbarsEnabled();
-  RuntimeEnabledFeatures::SetOverlayScrollbarsEnabled(true);
+      ScrollbarThemeSettings::OverlayScrollbarsEnabled();
+  ScrollbarThemeSettings::SetOverlayScrollbarsEnabled(true);
   is_orientation_event_enabled_ =
       RuntimeEnabledFeatures::OrientationEventEnabled();
   RuntimeEnabledFeatures::SetOrientationEventEnabled(true);
@@ -333,7 +334,7 @@ void DevToolsEmulator::EnableMobileEmulation() {
 void DevToolsEmulator::DisableMobileEmulation() {
   if (!emulate_mobile_enabled_)
     return;
-  RuntimeEnabledFeatures::SetOverlayScrollbarsEnabled(
+  ScrollbarThemeSettings::SetOverlayScrollbarsEnabled(
       is_overlay_scrollbars_enabled_);
   RuntimeEnabledFeatures::SetOrientationEventEnabled(
       is_orientation_event_enabled_);

diff --git a/third_party/blink/renderer/core/layout/layout_block_test.cc b/third_party/blink/renderer/core/layout/layout_block_test.cc
@@ -31,7 +31,8 @@ TEST_F(LayoutBlockTest, LayoutNameCalledWithNullStyle) {
 }
 
 TEST_F(LayoutBlockTest, WidthAvailableToChildrenChanged) {
-  ScopedOverlayScrollbarsForTest overlay_scrollbars(false);
+  USE_NON_OVERLAY_SCROLLBARS();
+
   SetBodyInnerHTML(R"HTML(
     <!DOCTYPE html>
     <div id='list' style='overflow-y:auto; width:150px; height:100px'>

diff --git a/third_party/blink/renderer/core/layout/map_coordinates_test.cc b/third_party/blink/renderer/core/layout/map_coordinates_test.cc
@@ -1820,7 +1820,8 @@ TEST_F(MapCoordinatesTest, IgnoreScrollOffsetWithWritingModes) {
 // non-overlay scrollbar.
 TEST_F(MapCoordinatesTest,
        IgnoreScrollOffsetWithWritingModesAndNonOverlayScrollbar) {
-  ScopedOverlayScrollbarsForTest overlay_scrollbars(false);
+  USE_NON_OVERLAY_SCROLLBARS();
+
   SetBodyInnerHTML(R"HTML(
     <style>
       body { margin: 0; }