Reland "Tab Search: Update TabSearchButton tests"

This is a reland of 0d88901a


Diff against patchset 1 for reland diff.

Added call to close widget and assert that the button is no
longer tracking.

Original change's description:
> Tab Search: Update TabSearchButton tests
>
> This CL creates a new file for TabSearchButton browser tests. These
> tests run for both the fixed and regular entrypoints.
>
> Existing tests for the TabSearchButton have been moved from the tab
> strip into the TabSearchButton browser tests.
>
> Bug: 1099917
> Change-Id: I0b1d57826768705316b9bed814dac7af96ce52e8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422730
> Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#809376}

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

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

@@ -699,6 +699,19 @@ int BrowserView::GetTabStripHeight() const {
   return IsTabStripVisible() ? tabstrip_->GetPreferredSize().height() : 0;
 }
 
+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)
+             ? tab_strip_region_view_->tab_search_button()
+             : tabstrip_->tab_search_button();
+}
+
 bool BrowserView::IsTabStripVisible() const {
   // Return false if this window does not normally display a tabstrip.
   if (!browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP))
@@ -2611,13 +2624,8 @@ void BrowserView::CreateTabSearchBubble() {
     return;
 #endif  // BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
 
-  // 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.
-  if ((base::FeatureList::IsEnabled(features::kTabSearchFixedEntrypoint) &&
-       tab_strip_region_view_->tab_search_button()->ShowTabSearchBubble()) ||
-      tabstrip_->tab_search_button()->ShowTabSearchBubble()) {
+  DCHECK(GetTabSearchButton());
+  if (GetTabSearchButton()->ShowTabSearchBubble()) {
     // Only log the open action if it resulted in creating a new instance of the
     // Tab Search bubble.
     base::UmaHistogramEnumeration("Tabs.TabSearch.OpenAction",

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

@@ -66,6 +66,7 @@ class FullscreenControlHost;
 class InfoBarContainerView;
 class LocationBarView;
 class StatusBubbleViews;
+class TabSearchButton;
 class TabStrip;
 class TabStripRegionView;
 class ToolbarButtonProvider;
@@ -219,6 +220,9 @@ class BrowserView : public BrowserWindow,
   // Accessor for the contents WebView.
   views::WebView* contents_web_view() { return contents_web_view_; }
 
+  // Accessor for the BrowserView's TabSearchButton instance.
+  TabSearchButton* GetTabSearchButton();
+
   // Returns true if various window components are visible.
   bool IsTabStripVisible() const;
 

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/tabs/tab_search_button.h"
+
+#include <vector>
+
+#include "base/feature_list.h"
+#include "base/test/scoped_feature_list.h"
+#include "build/build_config.h"
+#include "chrome/browser/ui/browser.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/test/base/in_process_browser_test.h"
+#include "content/public/test/browser_test.h"
+#include "ui/base/accelerators/accelerator.h"
+
+namespace {
+ui::MouseEvent GetDummyEvent() {
+  return ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::PointF(), gfx::PointF(),
+                        base::TimeTicks::Now(), 0, 0);
+}
+}  // namespace
+
+class TabSearchButtonBrowserTest : public InProcessBrowserTest,
+                                   public ::testing::WithParamInterface<bool> {
+ 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});
+    }
+    InProcessBrowserTest::SetUp();
+  }
+
+  BrowserView* browser_view() {
+    return BrowserView::GetBrowserViewForBrowser(browser());
+  }
+
+  TabSearchButton* tab_search_button() {
+    return browser_view()->GetTabSearchButton();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, CreateAndClose) {
+  ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
+  tab_search_button()->ButtonPressed(tab_search_button(), GetDummyEvent());
+  ASSERT_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);
+  ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
+}
+
+IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleVisible) {
+  EXPECT_FALSE(tab_search_button()->IsBubbleVisible());
+
+  ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
+  tab_search_button()->ButtonPressed(tab_search_button(), GetDummyEvent());
+  ASSERT_NE(nullptr, tab_search_button()->bubble_for_testing());
+
+  // The bubble should not be visible initially since the UI must notify the
+  // bubble it is ready before the bubble is shown.
+  EXPECT_FALSE(tab_search_button()->IsBubbleVisible());
+
+  // Trigger showing the bubble.
+  tab_search_button()->bubble_for_testing()->Show();
+
+  // The bubble should be visible after being shown.
+  EXPECT_TRUE(tab_search_button()->IsBubbleVisible());
+
+  // 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);
+  ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
+}
+
+// On macOS, most accelerators are handled by CommandDispatcher.
+#if !defined(OS_MAC)
+IN_PROC_BROWSER_TEST_P(TabSearchButtonBrowserTest, TestBubbleKeyboardShortcut) {
+  ASSERT_EQ(nullptr, tab_search_button()->bubble_for_testing());
+
+  auto accelerator = ui::Accelerator(
+      ui::VKEY_A, ui::EF_SHIFT_DOWN | ui::EF_PLATFORM_ACCELERATOR);
+  browser_view()->AcceleratorPressed(accelerator);
+
+  // Accelerator keys should have created the tab search bubble.
+  ASSERT_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);
+  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_browsertest.cc

@@ -13,7 +13,6 @@
 #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"
@@ -813,28 +812,3 @@ IN_PROC_BROWSER_TEST_F(TabStripBrowsertest,
   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());
-}

chrome/test/BUILD.gn

@@ -1454,6 +1454,7 @@ if (!is_android) {
       "../browser/ui/views/sharing/shared_clipboard_browsertest.cc",
       "../browser/ui/views/sharing/sharing_browsertest.cc",
       "../browser/ui/views/sharing/sharing_browsertest.h",
+      "../browser/ui/views/tabs/tab_search_button_browsertest.cc",
       "../browser/ui/views/tabs/tab_strip_browsertest.cc",
       "../browser/ui/views/try_chrome_dialog_win/try_chrome_dialog_browsertest.cc",
       "../browser/ui/views/web_apps/web_app_frame_toolbar_browsertest.cc",