Views: Fix the tab controls container while Tab Search is open

This CL updates the tab controls container in the tab strip to remain
fixed while the Tab Search bubble is open.

Keeping the tab controls container fixed is necessary to ensure that
the bubble does not slide around on the screen with the anchoring
button as tabs are removed from the Tab Search UI.

This CL also performs some cleanup and removes unnecessary build
flags.

Bug: 1099917
Change-Id: I28d395f4804175d3ab868d2ea4ae9e3607170571
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377550Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802431}

chrome/browser/ui/BUILD.gn

@@ -3880,6 +3880,8 @@ static_library("ui") {
       "views/tabs/tab_hover_card_bubble_view.h",
       "views/tabs/tab_icon.cc",
       "views/tabs/tab_icon.h",
+      "views/tabs/tab_search_button.cc",
+      "views/tabs/tab_search_button.h",
       "views/tabs/tab_slot_view.cc",
       "views/tabs/tab_slot_view.h",
       "views/tabs/tab_strip.cc",
@@ -4139,13 +4141,6 @@ static_library("ui") {
     if (is_chrome_branded) {
       deps += [ "//chrome/browser/ui/media_router/internal/vector_icons" ]
     }
-
-    if (enable_tab_search) {
-      sources += [
-        "views/tabs/tab_search_button.cc",
-        "views/tabs/tab_search_button.h",
-      ]
-    }
   }
 
   if (use_aura) {

chrome/browser/ui/views/frame/browser_view.cc

@@ -122,6 +122,7 @@
 #include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"
 #include "chrome/browser/ui/views/tabs/new_tab_button.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
+#include "chrome/browser/ui/views/tabs/tab_search_button.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 #include "chrome/browser/ui/views/toolbar/browser_app_menu_button.h"
@@ -2621,6 +2622,8 @@ const views::Widget* BrowserView::GetWidget() const {
 }
 
 void BrowserView::CreateTabSearchBubble() {
+  // TODO(tluk): This should be triggering the TabSearchButton in the tab strip
+  // rather than creating the Tab Search bubble directly.
   TabSearchBubbleView::CreateTabSearchBubble(browser_->profile(),
                                              tabstrip_->tab_search_button());
   base::UmaHistogramEnumeration("Tabs.TabSearch.OpenAction",

chrome/browser/ui/views/tabs/tab_search_button.cc

@@ -71,6 +71,11 @@ void TabSearchButton::OnWidgetClosing(views::Widget* widget) {
   observed_bubble_widget_.Remove(bubble_);
   bubble_ = nullptr;
   pressed_lock_.reset();
+  tab_strip()->OnTabSearchBubbleClosed();
+}
+
+bool TabSearchButton::IsBubbleVisible() const {
+  return bubble_ && bubble_->IsVisible();
 }
 
 void TabSearchButton::PaintIcon(gfx::Canvas* canvas) {

chrome/browser/ui/views/tabs/tab_search_button.h

@@ -45,6 +45,10 @@ class TabSearchButton : public NewTabButton,
   // views::WidgetObserver:
   void OnWidgetClosing(views::Widget* widget) override;
 
+  bool IsBubbleVisible() const;
+
+  views::Widget* bubble_for_testing() { return bubble_; }
+
  protected:
   // NewTabButton:
   void PaintIcon(gfx::Canvas* canvas) override;

chrome/browser/ui/views/tabs/tab_strip.cc

@@ -38,6 +38,7 @@
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/tab_search/tab_search_bubble_view.h"
 #include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"
 #include "chrome/browser/ui/views/tabs/new_tab_button.h"
 #include "chrome/browser/ui/views/tabs/stacked_tab_strip_layout.h"
@@ -48,6 +49,7 @@
 #include "chrome/browser/ui/views/tabs/tab_group_underline.h"
 #include "chrome/browser/ui/views/tabs/tab_group_views.h"
 #include "chrome/browser/ui/views/tabs/tab_hover_card_bubble_view.h"
+#include "chrome/browser/ui/views/tabs/tab_search_button.h"
 #include "chrome/browser/ui/views/tabs/tab_slot_view.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_layout_helper.h"
@@ -106,12 +108,6 @@
 #include "ui/aura/window.h"
 #endif
 
-#if BUILDFLAG(ENABLE_TAB_SEARCH)
-#include "chrome/browser/ui/ui_features.h"
-#include "chrome/browser/ui/views/tab_search/tab_search_bubble_view.h"
-#include "chrome/browser/ui/views/tabs/tab_search_button.h"
-#endif
-
 namespace {
 
 // Distance from the next/previous stacked before before we consider the tab
@@ -1447,6 +1443,11 @@ void TabStrip::ShiftGroupRight(const tab_groups::TabGroupId& group) {
   ShiftGroupRelative(group, 1);
 }
 
+void TabStrip::OnTabSearchBubbleClosed() {
+  UpdateIdealBounds();
+  AnimateToIdealBounds();
+}
+
 bool TabStrip::ShouldTabBeVisible(const Tab* tab) const {
   // Detached tabs should always be invisible (as they close).
   if (tab->detached())
@@ -2451,7 +2452,6 @@ void TabStrip::Init() {
   new_tab_button_ =
       tab_controls_container_->AddChildView(std::move(new_tab_button));
 
-#if BUILDFLAG(ENABLE_TAB_SEARCH)
   if (base::FeatureList::IsEnabled(features::kTabSearch) &&
       !controller_->GetProfile()->IsIncognitoProfile()) {
     auto tab_search_button = std::make_unique<TabSearchButton>(this, this);
@@ -2466,7 +2466,6 @@ void TabStrip::Init() {
     tab_search_button_ =
         tab_controls_container_->AddChildView(std::move(tab_search_button));
   }
-#endif
 
   UpdateNewTabButtonBorder();
   tab_controls_container_ideal_bounds_.set_size(
@@ -3373,9 +3372,13 @@ void TabStrip::UpdateIdealBounds() {
         base::FeatureList::IsEnabled(features::kScrollableTabStrip)
             ? trailing_x
             : std::min(available_width_for_tabs, trailing_x);
-
-    tab_controls_container_ideal_bounds_.set_origin(
-        gfx::Point(ntb_x_offset + TabToNewTabButtonSpacing(), 0));
+    // We want to lock the |tab_controls_container_| in place if the Tab Search
+    // bubble is open to prevent the UI sliding across the screen as the tab
+    // strip changes.
+    if (!tab_search_button_ || !tab_search_button_->IsBubbleVisible()) {
+      tab_controls_container_ideal_bounds_.set_origin(
+          gfx::Point(ntb_x_offset + TabToNewTabButtonSpacing(), 0));
+    }
   }
 }
 

chrome/browser/ui/views/tabs/tab_strip.h

@@ -45,6 +45,7 @@ class NewTabButton;
 class StackedTabStripLayout;
 class Tab;
 class TabHoverCardBubbleView;
+class TabSearchButton;
 class TabStripController;
 class TabStripObserver;
 class TabStripLayoutHelper;
@@ -201,6 +202,12 @@ class TabStrip : public views::AccessiblePaneView,
   // Attempts to move the specified group to the right.
   void ShiftGroupRight(const tab_groups::TabGroupId& group);
 
+  // While the Tab Search bubble is open, the |tab_controls_container_| is kept
+  // in a fixed position in the tab strip. This is called when the bubble is
+  // closed to allow the |tab_controls_container_| to animate to the correct
+  // position.
+  void OnTabSearchBubbleClosed();
+
   // Returns true if the tab is not partly or fully clipped (due to overflow),
   // and the tab couldn't become partly clipped due to changing the selected tab
   // (for example, if currently the strip has the last tab selected, and
@@ -238,7 +245,7 @@ class TabStrip : public views::AccessiblePaneView,
   NewTabButton* new_tab_button() { return new_tab_button_; }
 
   // Returns the TabSearchButton.
-  NewTabButton* tab_search_button() { return tab_search_button_; }
+  TabSearchButton* tab_search_button() { return tab_search_button_; }
 
   // Returns the index of the specified view in the model coordinate system, or
   // -1 if view is closing or not a tab.
@@ -700,7 +707,7 @@ class TabStrip : public views::AccessiblePaneView,
   NewTabButton* new_tab_button_ = nullptr;
   // |tab_search_button_| will be null if features::kTabSearch is disabled or if
   // the current profile is an incognito profile.
-  NewTabButton* tab_search_button_ = nullptr;
+  TabSearchButton* tab_search_button_ = nullptr;
 
   // Ideal bounds of container holding the tab controls.
   gfx::Rect tab_controls_container_ideal_bounds_;

chrome/browser/ui/views/tabs/tab_strip_browsertest.cc

@@ -6,17 +6,27 @@
 
 #include <vector>
 
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_group_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/tabs/tab_search_button.h"
 #include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/tab_groups/tab_group_id.h"
 #include "content/public/test/browser_test.h"
 #include "url/gurl.h"
 
+namespace {
+ui::MouseEvent GetDummyEvent() {
+  return ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
+                        base::TimeTicks::Now(), 0, 0);
+}
+}  // namespace
+
 // Integration tests for interactions between TabStripModel and TabStrip.
 class TabStripBrowsertest : public InProcessBrowserTest {
  public:
@@ -741,10 +751,7 @@ IN_PROC_BROWSER_TEST_F(TabStripBrowsertest,
   AppendTab();
 
   tab_groups::TabGroupId group = AddTabToNewGroup(0);
-  ui::MouseEvent dummy_event =
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
-                     base::TimeTicks::Now(), 0, 0);
-  tab_strip()->SelectTab(tab_strip()->tab_at(0), dummy_event);
+  tab_strip()->SelectTab(tab_strip()->tab_at(0), GetDummyEvent());
   ASSERT_EQ(0, tab_strip()->controller()->GetActiveIndex());
   ASSERT_FALSE(tab_strip()->controller()->IsGroupCollapsed(group));
   tab_strip()->controller()->ToggleTabGroupCollapsedState(group);
@@ -758,10 +765,7 @@ IN_PROC_BROWSER_TEST_F(TabStripBrowsertest,
   AppendTab();
 
   tab_groups::TabGroupId group = AddTabToNewGroup(1);
-  ui::MouseEvent dummy_event =
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
-                     base::TimeTicks::Now(), 0, 0);
-  tab_strip()->SelectTab(tab_strip()->tab_at(1), dummy_event);
+  tab_strip()->SelectTab(tab_strip()->tab_at(1), GetDummyEvent());
   ASSERT_EQ(1, tab_strip()->controller()->GetActiveIndex());
   ASSERT_FALSE(tab_strip()->controller()->IsGroupCollapsed(group));
   tab_strip()->controller()->ToggleTabGroupCollapsedState(group);
@@ -776,10 +780,7 @@ IN_PROC_BROWSER_TEST_F(
   AppendTab();
 
   tab_groups::TabGroupId group = AddTabToNewGroup(0);
-  ui::MouseEvent dummy_event =
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
-                     base::TimeTicks::Now(), 0, 0);
-  tab_strip()->SelectTab(tab_strip()->tab_at(1), dummy_event);
+  tab_strip()->SelectTab(tab_strip()->tab_at(1), GetDummyEvent());
   ASSERT_EQ(1, tab_strip()->controller()->GetActiveIndex());
   ASSERT_FALSE(tab_strip()->controller()->IsGroupCollapsed(group));
   tab_strip()->controller()->ToggleTabGroupCollapsedState(group);
@@ -809,9 +810,31 @@ IN_PROC_BROWSER_TEST_F(TabStripBrowsertest,
   ASSERT_TRUE(tab_strip()->controller()->IsGroupCollapsed(group));
   ASSERT_EQ(1, tab_strip()->controller()->GetActiveIndex());
 
-  ui::MouseEvent dummy_event =
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
-                     base::TimeTicks::Now(), 0, 0);
-  tab_strip()->SelectTab(tab_strip()->tab_at(0), dummy_event);
+  tab_strip()->SelectTab(tab_strip()->tab_at(0), GetDummyEvent());
   EXPECT_FALSE(tab_strip()->controller()->IsGroupCollapsed(group));
 }
+
+class TabSearchButtonTest : public TabStripBrowsertest {
+ public:
+  void SetUp() override {
+    scoped_feature_list_.InitWithFeatureState(features::kTabSearch, true);
+    TabStripBrowsertest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(TabSearchButtonTest, TabSearchBubble_CreateAndClose) {
+  TabSearchButton* tab_search_button = tab_strip()->tab_search_button();
+
+  DCHECK_EQ(nullptr, tab_search_button->bubble_for_testing());
+  tab_search_button->ButtonPressed(tab_search_button, GetDummyEvent());
+  DCHECK_NE(nullptr, tab_search_button->bubble_for_testing());
+
+  // Close the tab search bubble widget, the bubble should be cleared from the
+  // TabSearchButton.
+  tab_search_button->bubble_for_testing()->CloseWithReason(
+      views::Widget::ClosedReason::kUnspecified);
+  DCHECK_EQ(nullptr, tab_search_button->bubble_for_testing());
+}