Make scroll button for tab strip repeat while held (#35930)

* Make scroll button for tab strip repeat while held

The scroll button for the tab strip now repeats while held, just like the
scroll buttons for the scroll bars.

browser/ui/views/frame/brave_tab_strip_region_view.cc

@@ -5,8 +5,6 @@
 
 #include "brave/browser/ui/views/frame/brave_tab_strip_region_view.h"
 
-#include <algorithm>
-
 #include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "brave/browser/ui/tabs/brave_tab_prefs.h"
@@ -29,12 +27,123 @@
 #include "components/prefs/pref_service.h"
 #include "components/vector_icons/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/metadata/metadata_header_macros.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/events/event.h"
+#include "ui/views/controls/button/button.h"
+#include "ui/views/controls/button/button_controller.h"
 #include "ui/views/layout/flex_layout_types.h"
+#include "ui/views/repeat_controller.h"
 #include "ui/views/view_class_properties.h"
 #include "ui/views/view_utils.h"
 
 namespace {
+// Pattern from ui/views/controls/scrollbar/scroll_bar_button.h (scroll bar
+// line/page repeat). Uses kOnPress so the primary action is not fired again on
+// release.
+class BraveTabStripScrollButton : public TabStripControlButton {
+  METADATA_HEADER(BraveTabStripScrollButton, TabStripControlButton)
+
+ public:
+  BraveTabStripScrollButton(BrowserWindowInterface* browser_window_interface,
+                            base::RepeatingClosure scroll_action,
+                            const gfx::VectorIcon& icon);
+
+  BraveTabStripScrollButton(const BraveTabStripScrollButton&) = delete;
+  BraveTabStripScrollButton& operator=(const BraveTabStripScrollButton&) =
+      delete;
+
+  ~BraveTabStripScrollButton() override;
+
+  // views::View
+  bool OnMousePressed(const ui::MouseEvent& event) override;
+  void OnMouseReleased(const ui::MouseEvent& event) override;
+  void OnMouseCaptureLost() override;
+  void OnGestureEvent(ui::GestureEvent* event) override;
+
+ private:
+  void OnRepeaterFired();
+
+  base::RepeatingClosure scroll_action_;
+  views::RepeatController repeater_;
+};
+
+BraveTabStripScrollButton::BraveTabStripScrollButton(
+    BrowserWindowInterface* browser_window_interface,
+    base::RepeatingClosure scroll_action,
+    const gfx::VectorIcon& icon)
+    : TabStripControlButton(browser_window_interface,
+                            views::Button::PressedCallback(scroll_action),
+                            icon,
+                            Edge::kNone,
+                            Edge::kNone),
+      scroll_action_(std::move(scroll_action)),
+      repeater_(base::BindRepeating(&BraveTabStripScrollButton::OnRepeaterFired,
+                                    base::Unretained(this))) {
+  button_controller()->set_notify_action(
+      views::ButtonController::NotifyAction::kOnPress);
+}
+
+BraveTabStripScrollButton::~BraveTabStripScrollButton() {
+  repeater_.Stop();
+}
+
+void BraveTabStripScrollButton::OnRepeaterFired() {
+  scroll_action_.Run();
+}
+
+bool BraveTabStripScrollButton::OnMousePressed(const ui::MouseEvent& event) {
+  const bool result = TabStripControlButton::OnMousePressed(event);
+  if (GetState() != views::Button::STATE_DISABLED &&
+      event.IsOnlyLeftMouseButton()) {
+    repeater_.Start();
+  }
+  return result;
+}
+
+void BraveTabStripScrollButton::OnMouseReleased(const ui::MouseEvent& event) {
+  repeater_.Stop();
+  TabStripControlButton::OnMouseReleased(event);
+}
+
+void BraveTabStripScrollButton::OnMouseCaptureLost() {
+  repeater_.Stop();
+  TabStripControlButton::OnMouseCaptureLost();
+}
+
+void BraveTabStripScrollButton::OnGestureEvent(ui::GestureEvent* event) {
+  if (GetState() == views::Button::STATE_DISABLED) {
+    TabStripControlButton::OnGestureEvent(event);
+    return;
+  }
+
+  if (event->type() == ui::EventType::kGestureTapDown) {
+    TabStripControlButton::OnGestureEvent(event);
+    if (GetState() == views::Button::STATE_PRESSED) {
+      scroll_action_.Run();
+      repeater_.Start();
+    }
+    event->SetHandled();
+    return;
+  }
+
+  if (event->type() == ui::EventType::kGestureLongPress) {
+    return;
+  }
+
+  repeater_.Stop();
+
+  if (event->type() == ui::EventType::kGestureTap) {
+    SetState(views::Button::STATE_HOVERED);
+    event->SetHandled();
+    return;
+  }
+
+  TabStripControlButton::OnGestureEvent(event);
+}
+
+BEGIN_METADATA(BraveTabStripScrollButton)
+END_METADATA
 
 #if BUILDFLAG(IS_LINUX)
 ui::DropTargetEvent ConvertRootLocation(views::View* view,
@@ -69,20 +178,20 @@ void BraveHorizontalTabStripRegionView::CreateScrollButtonsIfNeeded() {
   // Child order for FlexLayout: leading scroll, tab strip, trailing scroll,
   // then (via base layout) new tab button after the strip cluster.
   tab_scroll_next_button_ = AddChildViewAt(
-      std::make_unique<TabStripControlButton>(
+      std::make_unique<BraveTabStripScrollButton>(
           bwi,
           base::BindRepeating(
               &BraveHorizontalTabStripRegionView::OnScrollNextPressed,
               weak_factory_.GetWeakPtr()),
-          vector_icons::kForwardArrowIcon, Edge::kNone, Edge::kNone),
+          vector_icons::kForwardArrowIcon),
       strip_idx.value() + 1);
   tab_scroll_previous_button_ = AddChildViewAt(
-      std::make_unique<TabStripControlButton>(
+      std::make_unique<BraveTabStripScrollButton>(
           bwi,
           base::BindRepeating(
               &BraveHorizontalTabStripRegionView::OnScrollPreviousPressed,
               weak_factory_.GetWeakPtr()),
-          vector_icons::kBackArrowIcon, Edge::kNone, Edge::kNone),
+          vector_icons::kBackArrowIcon),
       strip_idx.value());
 
   tab_scroll_previous_button_->SetProperty(views::kCrossAxisAlignmentKey,

browser/ui/views/tabs/BUILD.gn

@@ -31,6 +31,7 @@ source_set("browser_tests") {
     "//chrome/test:test_support",
     "//components/prefs",
     "//ui/base:test_support",
+    "//ui/events:test_support",
   ]
 
   if (use_ozone) {

browser/ui/views/tabs/brave_tab_container_browsertest.cc

@@ -5,6 +5,11 @@
 
 #include "brave/browser/ui/views/tabs/brave_tab_container.h"
 
+#include <algorithm>
+
+#include "base/location.h"
+#include "base/run_loop.h"
+#include "base/task/sequenced_task_runner.h"
 #include "base/test/run_until.h"
 #include "base/test/scoped_feature_list.h"
 #include "brave/browser/ui/tabs/brave_tab_prefs.h"
@@ -29,7 +34,10 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/event_constants.h"
+#include "ui/events/test/event_generator.h"
+#include "ui/views/repeat_controller.h"
 #include "ui/views/view_utils.h"
+#include "ui/views/widget/widget_utils.h"
 #include "url/url_constants.h"
 
 class HorizontalScrollableTabStripBrowserTest : public InProcessBrowserTest {
@@ -77,6 +85,27 @@ class HorizontalScrollableTabStripBrowserTest : public InProcessBrowserTest {
         tab_strip->parent());
   }
 
+  // Grows the strip so at least |n| scroll-button steps can apply before
+  // clamping (viewport/4 as step is often greater than the max offset after
+  // the first few overflow tabs).
+  void GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep(
+      BraveTabContainer* container,
+      int n,
+      base::Location location = base::Location::Current()) {
+    SCOPED_TRACE(location.ToString());
+    for (int i = 0; i < 100; i++) {
+      const int max_offset = container->GetMaxScrollOffsetForTesting();
+      const int step = container->GetHorizontalTabScrollStep();
+      if (step > 0 && max_offset >= n * step) {
+        return;
+      }
+      AppendTab();
+      StopAnimatingAndLayout();
+    }
+    GTEST_FAIL()
+        << "Failed to get enough scroll headroom for repeat/hold test.";
+  }
+
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
     // Horizontal scrolling requires kBraveScrollableTabStrip and this pref.
@@ -410,3 +439,109 @@ IN_PROC_BROWSER_TEST_F(HorizontalScrollableTabStripBrowserTest,
   ASSERT_TRUE(region->tab_scroll_previous_for_testing());
   EXPECT_FALSE(region->tab_scroll_previous_for_testing()->GetVisible());
 }
+
+IN_PROC_BROWSER_TEST_F(
+    HorizontalScrollableTabStripBrowserTest,
+    HorizontalScrollPreviousButtonSingleClickScrollsByOneStep) {
+  auto* tab_strip = views::AsViewClass<BraveTabStrip>(
+      browser_view()->horizontal_tab_strip_for_testing());
+  BraveTabContainer* container = views::AsViewClass<BraveTabContainer>(
+      tab_strip->GetTabContainerForTesting());
+  ASSERT_TRUE(container);
+
+  browser()->profile()->GetPrefs()->SetBoolean(
+      brave_tabs::kShowHorizontalTabScrollButtons, true);
+  StopAnimatingAndLayout();
+  GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep(container, 2);
+
+  BraveHorizontalTabStripRegionView* region = tab_strip_region();
+  ASSERT_TRUE(region);
+  const int max_scroll_offset = container->GetMaxScrollOffsetForTesting();
+  ASSERT_GT(max_scroll_offset, 0);
+  // Exercise the leading (back) control at max scroll. The trailing (forward)
+  // button is disabled at the end; growing many tabs often leaves the last
+  // tab active with the strip scrolled to the end.
+  browser()->tab_strip_model()->ActivateTabAt(
+      std::max(0, browser()->tab_strip_model()->count() - 1));
+  container->SetScrollOffsetForTesting(max_scroll_offset);
+  StopAnimatingAndLayout();
+
+  TabStripControlButton* back = region->tab_scroll_previous_for_testing();
+  ASSERT_TRUE(back);
+  ASSERT_TRUE(back->GetVisible());
+  ASSERT_TRUE(back->GetEnabled());
+
+  const int step = container->GetHorizontalTabScrollStep();
+  ASSERT_GT(step, 0);
+
+  const int before = container->GetScrollOffsetForTesting();
+
+  ui::test::EventGenerator event_generator(
+      views::GetRootWindow(browser_view()->GetWidget()),
+      browser_view()->GetNativeWindow());
+  event_generator.MoveMouseTo(back->GetBoundsInScreen().CenterPoint());
+  event_generator.PressLeftButton();
+  event_generator.ReleaseLeftButton();
+  StopAnimatingAndLayout();
+
+  EXPECT_EQ(before - container->GetScrollOffsetForTesting(), step)
+      << "before: " << before
+      << " after: " << container->GetScrollOffsetForTesting()
+      << " One press+release should match one scroll action (not double on "
+         "press, "
+         "repeater should not have fired).";
+}
+
+IN_PROC_BROWSER_TEST_F(
+    HorizontalScrollableTabStripBrowserTest,
+    HorizontalScrollPreviousButtonHoldScrollsByMoreThanOneStep) {
+  auto* tab_strip = views::AsViewClass<BraveTabStrip>(
+      browser_view()->horizontal_tab_strip_for_testing());
+  BraveTabContainer* container = views::AsViewClass<BraveTabContainer>(
+      tab_strip->GetTabContainerForTesting());
+  ASSERT_TRUE(container);
+
+  browser()->profile()->GetPrefs()->SetBoolean(
+      brave_tabs::kShowHorizontalTabScrollButtons, false);
+  browser()->profile()->GetPrefs()->SetBoolean(
+      brave_tabs::kShowHorizontalTabScrollButtons, true);
+  StopAnimatingAndLayout();
+  GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep(container, 2);
+
+  const int max_scroll_offset = container->GetMaxScrollOffsetForTesting();
+  const int step = container->GetHorizontalTabScrollStep();
+  ASSERT_GT(step, 0);
+  ASSERT_GE(max_scroll_offset, 2 * step);
+
+  BraveHorizontalTabStripRegionView* region = tab_strip_region();
+  ASSERT_TRUE(region);
+  browser()->tab_strip_model()->ActivateTabAt(
+      std::max(0, browser()->tab_strip_model()->count() - 1));
+  container->SetScrollOffsetForTesting(max_scroll_offset);
+  StopAnimatingAndLayout();
+
+  TabStripControlButton* back = region->tab_scroll_previous_for_testing();
+  ASSERT_TRUE(back);
+  ASSERT_TRUE(back->GetVisible());
+  ASSERT_TRUE(back->GetEnabled());
+
+  const int before = container->GetScrollOffsetForTesting();
+
+  ui::test::EventGenerator event_generator(
+      views::GetRootWindow(browser_view()->GetWidget()),
+      browser_view()->GetNativeWindow());
+  event_generator.MoveMouseTo(back->GetBoundsInScreen().CenterPoint());
+  event_generator.PressLeftButton();
+  base::RunLoop run_loop;
+  base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(),
+      views::RepeatController::GetInitialWaitForTesting() +
+          views::RepeatController::GetRepeatingWaitForTesting() * 2);
+  run_loop.Run();
+  event_generator.ReleaseLeftButton();
+  StopAnimatingAndLayout();
+
+  const int after = container->GetScrollOffsetForTesting();
+  EXPECT_GT(before - after, step)
+      << "Holding should run the repeat timer and scroll more than one action.";
+}