Commit 12c756d2 authored by Sammie Quon's avatar Sammie Quon Committed by Chromium LUCI CQ

Revert "Fix flakiness in ChromeLabsButtonTest.ShowAndHideChromeLabsBubbleOnPress"

This reverts commit 9cc64477.

Reason for revert: Breaks ChromeOS builder. See https://crbug.com/1165349.

Original change's description:
> 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/+/2618656
> Reviewed-by: Collin Baker <collinbaker@chromium.org>
> Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#842150}

TBR=davidbienvenu@chromium.org,collinbaker@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I5cd05c29bb299b97fe8b491d865a9925c642a5f5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 956719
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622519Reviewed-by: default avatarSammie Quon <sammiequon@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842174}
parent 2f6cfe5c
...@@ -23,12 +23,6 @@ ChromeLabsBubbleViewModel::ChromeLabsBubbleViewModel() { ...@@ -23,12 +23,6 @@ ChromeLabsBubbleViewModel::ChromeLabsBubbleViewModel() {
SetUpLabs(); SetUpLabs();
} }
ChromeLabsBubbleViewModel::ChromeLabsBubbleViewModel(
const std::vector<LabInfo>& lab_info)
: lab_info_(lab_info) {
SetUpLabs();
}
ChromeLabsBubbleViewModel::~ChromeLabsBubbleViewModel() = default; ChromeLabsBubbleViewModel::~ChromeLabsBubbleViewModel() = default;
const std::vector<LabInfo>& ChromeLabsBubbleViewModel::GetLabInfo() const { const std::vector<LabInfo>& ChromeLabsBubbleViewModel::GetLabInfo() const {
......
...@@ -32,8 +32,6 @@ struct LabInfo { ...@@ -32,8 +32,6 @@ struct LabInfo {
class ChromeLabsBubbleViewModel { class ChromeLabsBubbleViewModel {
public: public:
ChromeLabsBubbleViewModel(); ChromeLabsBubbleViewModel();
// |lab_info_| will have `lab_info`, and whatever SetUpLabs appends to it.
explicit ChromeLabsBubbleViewModel(const std::vector<LabInfo>& lab_info);
~ChromeLabsBubbleViewModel(); ~ChromeLabsBubbleViewModel();
const std::vector<LabInfo>& GetLabInfo() const; const std::vector<LabInfo>& GetLabInfo() const;
......
...@@ -15,8 +15,6 @@ ChromeLabsButton::ChromeLabsButton() ...@@ -15,8 +15,6 @@ ChromeLabsButton::ChromeLabsButton()
views::ButtonController::NotifyAction::kOnPress); views::ButtonController::NotifyAction::kOnPress);
} }
ChromeLabsButton::~ChromeLabsButton() = default;
void ChromeLabsButton::UpdateIcon() { void ChromeLabsButton::UpdateIcon() {
UpdateIconsWithStandardColors(kChromeLabsIcon); UpdateIconsWithStandardColors(kChromeLabsIcon);
} }
...@@ -25,17 +23,11 @@ const char* ChromeLabsButton::GetClassName() const { ...@@ -25,17 +23,11 @@ const char* ChromeLabsButton::GetClassName() const {
return "ChromeLabsButton"; return "ChromeLabsButton";
} }
void ChromeLabsButton::SetLabInfoForTesting(
const std::vector<LabInfo>& test_lab_info) {
test_lab_info_ = test_lab_info;
}
void ChromeLabsButton::ButtonPressed() { void ChromeLabsButton::ButtonPressed() {
if (ChromeLabsBubbleView::IsShowing()) { if (ChromeLabsBubbleView::IsShowing()) {
ChromeLabsBubbleView::Hide(); ChromeLabsBubbleView::Hide();
return; return;
} }
std::unique_ptr<ChromeLabsBubbleViewModel> model = ChromeLabsBubbleView::Show(this,
std::make_unique<ChromeLabsBubbleViewModel>(test_lab_info_); std::make_unique<ChromeLabsBubbleViewModel>());
ChromeLabsBubbleView::Show(this, std::move(model));
} }
...@@ -5,13 +5,11 @@ ...@@ -5,13 +5,11 @@
#ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_ #ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_
#define 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" #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
class ChromeLabsButton : public ToolbarButton { class ChromeLabsButton : public ToolbarButton {
public: public:
ChromeLabsButton(); ChromeLabsButton();
~ChromeLabsButton() override;
// ToolbarButton: // ToolbarButton:
void UpdateIcon() override; void UpdateIcon() override;
...@@ -19,14 +17,8 @@ class ChromeLabsButton : public ToolbarButton { ...@@ -19,14 +17,8 @@ class ChromeLabsButton : public ToolbarButton {
// views::View: // views::View:
const char* GetClassName() const override; const char* GetClassName() const override;
void SetLabInfoForTesting(const std::vector<LabInfo>& test_lab_info);
private: private:
void ButtonPressed(); 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_ #endif // CHROME_BROWSER_UI_VIEWS_TOOLBAR_CHROME_LABS_BUTTON_H_
...@@ -4,21 +4,15 @@ ...@@ -4,21 +4,15 @@
#include "chrome/browser/ui/views/toolbar/chrome_labs_button.h" #include "chrome/browser/ui/views/toolbar/chrome_labs_button.h"
#include "base/test/scoped_feature_list.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/ui_features.h"
#include "chrome/browser/ui/views/frame/browser_view.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/frame/test_with_browser_view.h"
#include "chrome/browser/ui/views/toolbar/chrome_labs_bubble_view.h" #include "chrome/browser/ui/views/toolbar/chrome_labs_bubble_view.h"
#include "chrome/browser/ui/views/toolbar/toolbar_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/events/event_utils.h"
#include "ui/views/test/button_test_api.h" #include "ui/views/test/button_test_api.h"
#include "ui/views/test/widget_test.h" #include "ui/views/test/widget_test.h"
namespace {
const char kFirstTestFeatureId[] = "feature-1";
} // namespace
class ChromeLabsButtonTest : public TestWithBrowserView { class ChromeLabsButtonTest : public TestWithBrowserView {
public: public:
void SetUp() override { void SetUp() override {
...@@ -34,22 +28,6 @@ TEST_F(ChromeLabsButtonTest, ShowAndHideChromeLabsBubbleOnPress) { ...@@ -34,22 +28,6 @@ TEST_F(ChromeLabsButtonTest, ShowAndHideChromeLabsBubbleOnPress) {
ChromeLabsButton* labs_button = ChromeLabsButton* labs_button =
browser_view()->toolbar()->chrome_labs_button(); browser_view()->toolbar()->chrome_labs_button();
EXPECT_FALSE(ChromeLabsBubbleView::IsShowing()); 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::MouseEvent e(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), 0, 0); ui::EventTimeForNow(), 0, 0);
views::test::ButtonTestApi test_api(labs_button); views::test::ButtonTestApi test_api(labs_button);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment