Tab Search: Remove the non-fixed Tab Search entry point.

The tab search feature will only support a fixed entry point in the
tab strip region view going forward. This change removes the
implementation of the tab search button that sits flush with the new
tab button in the tab strip.

This reverts most of the changes introduced in the following CLs:
https://crrev.com/c/2333341
https://crrev.com/c/2377550

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

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

@@ -736,16 +736,9 @@ int BrowserView::GetTabStripHeight() const {
 }
 
 TabSearchButton* BrowserView::GetTabSearchButton() {
-  if (!base::FeatureList::IsEnabled(features::kTabSearch))
-    return nullptr;
-
-  // If kTabSearchFixedEntrypoint is enabled then the tab search button is
-  // defined in the tab strip region view.
-  // TODO(tluk): Consolidate these once Tab Scrolling successfully moves the
-  // tab controls container to the tab strip region view.
-  return base::FeatureList::IsEnabled(features::kTabSearchFixedEntrypoint)
+  return base::FeatureList::IsEnabled(features::kTabSearch)
              ? tab_strip_region_view_->tab_search_button()
-             : tabstrip_->tab_search_button();
+             : nullptr;
 }
 
 bool BrowserView::IsTabStripVisible() const {

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

@@ -107,7 +107,6 @@ TabStripRegionView::TabStripRegionView(std::unique_ptr<TabStrip> tab_strip) {
                                views::MaximumFlexSizeRule::kUnbounded));
 
   if (base::FeatureList::IsEnabled(features::kTabSearch) &&
-      base::FeatureList::IsEnabled(features::kTabSearchFixedEntrypoint) &&
       !tab_strip_->controller()->GetProfile()->IsIncognitoProfile()) {
     auto tab_search_button = std::make_unique<TabSearchButton>(tab_strip_);
     tab_search_button->SetTooltipText(

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

@@ -28,11 +28,9 @@ class TabStripRegionViewBrowserTest
   void SetUp() override {
     // Run the test with both kTabSearchFixedEntrypoint enabled and disabled.
     if (GetParam()) {
-      scoped_feature_list_.InitWithFeatures(
-          {features::kTabSearch, features::kTabSearchFixedEntrypoint}, {});
+      scoped_feature_list_.InitWithFeatures({features::kTabSearch}, {});
     } else {
-      scoped_feature_list_.InitWithFeatures(
-          {}, {features::kTabSearch, features::kTabSearchFixedEntrypoint});
+      scoped_feature_list_.InitWithFeatures({}, {features::kTabSearch});
     }
     InProcessBrowserTest::SetUp();
   }
@@ -178,8 +176,7 @@ IN_PROC_BROWSER_TEST_P(TabStripRegionViewBrowserTest, TestBeginEndFocus) {
 
 IN_PROC_BROWSER_TEST_P(TabStripRegionViewBrowserTest,
                        TestSearchButtonIsEndAligned) {
-  if (base::FeatureList::IsEnabled(features::kTabSearch) &&
-      base::FeatureList::IsEnabled(features::kTabSearchFixedEntrypoint)) {
+  if (base::FeatureList::IsEnabled(features::kTabSearch)) {
     EXPECT_EQ(tab_strip_region_view()->GetLocalBounds().right(),
               tab_search_button()->bounds().right());
   }

chrome/browser/ui/views/tab_search/tab_search_bubble_view_browsertest.cc

@@ -65,10 +65,7 @@ IN_PROC_BROWSER_TEST_F(TabSearchBubbleBrowserTest, ShowTriggersTimer) {
 class TabSearchBubbleBrowserUITest : public DialogBrowserTest {
  public:
   TabSearchBubbleBrowserUITest() {
-    feature_list_.InitWithFeatures(
-        /*enabled_features=*/{features::kTabSearch,
-                              features::kTabSearchFixedEntrypoint},
-        /*disabled_features=*/{});
+    feature_list_.InitAndEnableFeature(features::kTabSearch);
   }
   TabSearchBubbleBrowserUITest(TabSearchBubbleBrowserUITest&) = delete;
   TabSearchBubbleBrowserUITest& operator=(TabSearchBubbleBrowserUITest&) =

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

@@ -58,7 +58,6 @@ void TabSearchButton::OnWidgetClosing(views::Widget* widget) {
   observed_bubble_widget_.Remove(bubble_);
   bubble_ = nullptr;
   pressed_lock_.reset();
-  tab_strip()->OnTabSearchBubbleClosed();
 }
 
 bool TabSearchButton::ShowTabSearchBubble() {

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

@@ -25,22 +25,11 @@ ui::MouseEvent GetDummyEvent() {
 }
 }  // namespace
 
-class TabSearchButtonBrowserTest : public InProcessBrowserTest,
-                                   public ::testing::WithParamInterface<bool> {
+class TabSearchButtonBrowserTest : public InProcessBrowserTest {
  public:
   // InProcessBrowserTest:
   void SetUp() override {
-    // Run the test with both kTabSearchFixedEntrypoint enabled and disabled.
-    if (GetParam()) {
-      scoped_feature_list_.InitWithFeatures(
-          /*enabled_features=*/{features::kTabSearch,
-                                features::kTabSearchFixedEntrypoint},
-          /*disabled_features=*/{});
-    } else {
-      scoped_feature_list_.InitWithFeatures(
-          /*enabled_features=*/{features::kTabSearch},
-          /*disabled_features=*/{features::kTabSearchFixedEntrypoint});
-    }
+    scoped_feature_list_.InitAndEnableFeature(features::kTabSearch);
     InProcessBrowserTest::SetUp();
   }
 
@@ -56,7 +45,7 @@ class TabSearchButtonBrowserTest : public InProcessBrowserTest,
   base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, CreateAndClose) {
+IN_PROC_BROWSER_TEST_F(TabSearchButtonBrowserTest, CreateAndClose) {
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
   views::test::ButtonTestApi(tab_search_button()).NotifyClick(GetDummyEvent());
   ASSERT_NE(nullptr, tab_search_button()->bubble_for_testing());
@@ -68,7 +57,7 @@ IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, CreateAndClose) {
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
 }
 
-IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleVisible) {
+IN_PROC_BROWSER_TEST_F(TabSearchButtonBrowserTest, TestBubbleVisible) {
   EXPECT_FALSE(tab_search_button()->IsBubbleVisible());
 
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
@@ -92,7 +81,7 @@ IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleVisible) {
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
 }
 
-IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, BubbleNotVisibleIncognito) {
+IN_PROC_BROWSER_TEST_F(TabSearchButtonBrowserTest, BubbleNotVisibleIncognito) {
   Browser* incognito_browser = CreateIncognitoBrowser();
   BrowserView* incognito_browser_view =
       BrowserView::GetBrowserViewForBrowser(incognito_browser);
@@ -103,7 +92,7 @@ IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, BubbleNotVisibleIncognito) {
 
 // On macOS, most accelerators are handled by CommandDispatcher.
 #if !defined(OS_MAC)
-IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleKeyboardShortcut) {
+IN_PROC_BROWSER_TEST_F(TabSearchButtonBrowserTest, TestBubbleKeyboardShortcut) {
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
 
   auto accelerator = ui::Accelerator(
@@ -120,7 +109,3 @@ IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleKeyboardShortcut) {
   ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
 }
 #endif
-
-INSTANTIATE_TEST_SUITE_P(All,
-                         TabSearchButtonBrowserTest,
-                         ::testing::Values(true, false));

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

@@ -44,7 +44,6 @@ class NewTabButton;
 class StackedTabStripLayout;
 class Tab;
 class TabHoverCardBubbleView;
-class TabSearchButton;
 class TabStripController;
 class TabStripObserver;
 class TabStripLayoutHelper;
@@ -146,9 +145,9 @@ class TabStrip : public views::View,
   // Sets |stacked_layout_| and animates if necessary.
   void SetStackedLayout(bool stacked_layout);
 
-  // Returns the ideal bounds of the tab controls container.
-  gfx::Rect tab_controls_container_ideal_bounds() const {
-    return tab_controls_container_ideal_bounds_;
+  // Returns the ideal bounds of the new tab button.
+  const gfx::Rect& new_tab_button_ideal_bounds() const {
+    return new_tab_button_ideal_bounds_;
   }
 
   // Adds a tab at the specified index.
@@ -205,12 +204,6 @@ class TabStrip : public views::View,
   // 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
@@ -247,9 +240,6 @@ class TabStrip : public views::View,
   // Returns the NewTabButton.
   NewTabButton* new_tab_button() { return new_tab_button_; }
 
-  // Returns the TabSearchButton.
-  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.
   int GetModelIndexOf(const TabSlotView* view) const;
@@ -709,15 +699,11 @@ class TabStrip : public views::View,
   // Responsible for animating tabs in response to model changes.
   views::BoundsAnimator bounds_animator_{this};
 
-  // Container that holds the |new_tab_button_| and the |tab_search_button_|.
-  views::View* tab_controls_container_ = nullptr;
+  // The "New Tab" button.
   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.
-  TabSearchButton* tab_search_button_ = nullptr;
 
-  // Ideal bounds of container holding the tab controls.
-  gfx::Rect tab_controls_container_ideal_bounds_;
+  // Ideal bounds of the new tab button.
+  gfx::Rect new_tab_button_ideal_bounds_;
 
   // If this value is defined, it is used as the width to lay out tabs
   // (instead of GetTabAreaWidth()). It is defined when closing tabs with the

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

@@ -138,10 +138,6 @@ class TabStripTest : public ChromeViewsTestBase,
     return tab->icon_->ShowingAttentionIndicator();
   }
 
-  views::View* tab_controls_container() {
-    return tab_strip_->tab_controls_container_;
-  }
-
   // Checks whether |tab| contains |point_in_tabstrip_coords|, where the point
   // is in |tab_strip_| coordinates.
   bool IsPointInTab(Tab* tab, const gfx::Point& point_in_tabstrip_coords) {
@@ -863,8 +859,7 @@ TEST_P(TabStripTest, NewTabButtonStaysVisible) {
 
   CompleteAnimationAndLayout();
 
-  EXPECT_LE(tab_strip_->tab_controls_container_ideal_bounds().right(),
-            kTabStripWidth);
+  EXPECT_LE(tab_strip_->new_tab_button_ideal_bounds().right(), kTabStripWidth);
 }
 
 TEST_P(TabStripTest, NewTabButtonRightOfTabs) {
@@ -875,7 +870,7 @@ TEST_P(TabStripTest, NewTabButtonRightOfTabs) {
 
   AnimateToIdealBounds();
 
-  EXPECT_EQ(tab_strip_->tab_controls_container_ideal_bounds().x(),
+  EXPECT_EQ(tab_strip_->new_tab_button_ideal_bounds().x(),
             tab_strip_->ideal_bounds(0).right() + TabToNewTabButtonSpacing());
 }
 
@@ -1351,20 +1346,10 @@ TEST_P(TabStripTest, NewTabButtonFlushWithTopOfTabStrip) {
 
   AnimateToIdealBounds();
 
-  // |tab_controls_container_| should sit flush with the top of the tab strip.
-  EXPECT_EQ(0, tab_strip_->tab_controls_container_ideal_bounds().y());
-
   // The new tab button should sit flush with the top of the
-  // |tab_controls_container_|.
+  // |tab_strip_|.
+  EXPECT_EQ(tab_strip_, tab_strip_->new_tab_button()->parent());
   EXPECT_EQ(0, tab_strip_->new_tab_button()->bounds().y());
-
-  // The new tab button should be positioned flush with the top of the tab
-  // strip.
-  gfx::RectF ntb_in_child_coords_f(tab_strip_->new_tab_button()->bounds());
-  views::View::ConvertRectToTarget(tab_controls_container(), tab_strip_,
-                                   &ntb_in_child_coords_f);
-  gfx::Rect ntb_in_child_coords = gfx::ToEnclosingRect(ntb_in_child_coords_f);
-  EXPECT_EQ(0, ntb_in_child_coords.y());
 }
 
 INSTANTIATE_TEST_SUITE_P(All, TabStripTest, ::testing::Values(false, true));