Fix flakiness in ChromeLabsButtonTest.ShowAndHideChromeLabsBubbleOnPress

This test is very flaky, I believe because it relies on global state,
instead of explicitly setting up the LabInfo used to populate the labs
button. For example, if there were no current labs in SetUpLabs, the
test would fail. So this CL explicitly sets up the LabInfo for the labs
button. It's a bit messy that it requires a change to the production
code, but I didn't see any other way of plumbing that information
through to the model.

Bug: 956719
Change-Id: I8078c211e37fc1eeff50a44043eca760cdc4850a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2618656Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842150}

chrome/browser/ui/views/toolbar/chrome_labs_bubble_view_model.cc

@@ -10,6 +10,12 @@ ChromeLabsBubbleViewModel::ChromeLabsBubbleViewModel() {
   SetUpLabs();
 }
 
+ChromeLabsBubbleViewModel::ChromeLabsBubbleViewModel(
+    const std::vector<LabInfo>& lab_info)
+    : lab_info_(lab_info) {
+  SetUpLabs();
+}
+
 ChromeLabsBubbleViewModel::~ChromeLabsBubbleViewModel() = default;
 
 const std::vector<LabInfo>& ChromeLabsBubbleViewModel::GetLabInfo() const {

chrome/browser/ui/views/toolbar/chrome_labs_bubble_view_model.h

@@ -27,6 +27,8 @@ struct LabInfo {
 class ChromeLabsBubbleViewModel {
  public:
   ChromeLabsBubbleViewModel();
+  // |lab_info_| will have `lab_info`, and whatever SetUpLabs appends to it.
+  explicit ChromeLabsBubbleViewModel(const std::vector<LabInfo>& lab_info);
   ~ChromeLabsBubbleViewModel();
 
   const std::vector<LabInfo>& GetLabInfo() const;

chrome/browser/ui/views/toolbar/chrome_labs_button.cc

@@ -15,6 +15,8 @@ ChromeLabsButton::ChromeLabsButton()
       views::ButtonController::NotifyAction::kOnPress);
 }
 
+ChromeLabsButton::~ChromeLabsButton() = default;
+
 void ChromeLabsButton::UpdateIcon() {
   UpdateIconsWithStandardColors(kChromeLabsIcon);
 }
@@ -23,11 +25,17 @@ const char* ChromeLabsButton::GetClassName() const {
   return "ChromeLabsButton";
 }
 
+void ChromeLabsButton::SetLabInfoForTesting(
+    const std::vector<LabInfo>& test_lab_info) {
+  test_lab_info_ = test_lab_info;
+}
+
 void ChromeLabsButton::ButtonPressed() {
   if (ChromeLabsBubbleView::IsShowing()) {
     ChromeLabsBubbleView::Hide();
     return;
   }
-  ChromeLabsBubbleView::Show(this,
-                             std::make_unique<ChromeLabsBubbleViewModel>());
+  std::unique_ptr<ChromeLabsBubbleViewModel> model =
+      std::make_unique<ChromeLabsBubbleViewModel>(test_lab_info_);
+  ChromeLabsBubbleView::Show(this, std::move(model));
 }

chrome/browser/ui/views/toolbar/chrome_labs_button.h

@@ -5,11 +5,13 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_
 #define CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_
 
+#include "chrome/browser/ui/views/toolbar/chrome_labs_bubble_view_model.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 
 class ChromeLabsButton : public ToolbarButton {
  public:
   ChromeLabsButton();
+  ~ChromeLabsButton() override;
 
   // ToolbarButton:
   void UpdateIcon() override;
@@ -17,8 +19,14 @@ class ChromeLabsButton : public ToolbarButton {
   // views::View:
   const char* GetClassName() const override;
 
+  void SetLabInfoForTesting(const std::vector<LabInfo>& test_lab_info);
+
  private:
   void ButtonPressed();
+
+  // Used by tests to customize the LabInfo used to populate the button's menu.
+  // This will be empty in production code.
+  std::vector<LabInfo> test_lab_info_;
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_

chrome/browser/ui/views/toolbar/chrome_labs_button_unittest.cc

@@ -4,15 +4,21 @@
 
 #include "chrome/browser/ui/views/toolbar/chrome_labs_button.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/about_flags.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/test_with_browser_view.h"
 #include "chrome/browser/ui/views/toolbar/chrome_labs_bubble_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "components/flags_ui/feature_entry_macros.h"
 #include "ui/events/event_utils.h"
 #include "ui/views/test/button_test_api.h"
 #include "ui/views/test/widget_test.h"
 
+namespace {
+const char kFirstTestFeatureId[] = "feature-1";
+}  // namespace
+
 class ChromeLabsButtonTest : public TestWithBrowserView {
  public:
   void SetUp() override {
@@ -28,6 +34,22 @@ TEST_F(ChromeLabsButtonTest, ShowAndHideChromeLabsBubbleOnPress) {
   ChromeLabsButton* labs_button =
       browser_view()->toolbar()->chrome_labs_button();
   EXPECT_FALSE(ChromeLabsBubbleView::IsShowing());
+
+  // Explicitly set up the feature flags and LabInfo for the button instead of
+  // relying on ChromeLabsBubbleViewModel::SetUpLabs().
+  const base::Feature kTestFeature1{"FeatureName1",
+                                    base::FEATURE_ENABLED_BY_DEFAULT};
+
+  std::vector<flags_ui::FeatureEntry> entries = {
+      {kFirstTestFeatureId, "", "", flags_ui::FlagsState::GetCurrentPlatform(),
+       FEATURE_VALUE_TYPE(kTestFeature1)}};
+  about_flags::testing::SetFeatureEntries(entries);
+
+  std::vector<LabInfo> test_feature_info = {
+      {kFirstTestFeatureId, base::ASCIIToUTF16(""), base::ASCIIToUTF16("")}};
+
+  labs_button->SetLabInfoForTesting(test_feature_info);
+
   ui::MouseEvent e(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
                    ui::EventTimeForNow(), 0, 0);
   views::test::ButtonTestApi test_api(labs_button);