0

Revert "[headless] Use --disable-infobars switch in headless Chrome"

This reverts commit 70c0208350.

Reason for revert: breaking linux-cft compile e.g. https://ci.chromium.org/ui/p/chrome/builders/ci/linux-cft/b8745477625053030737/overview

Original change's description:
> [headless] Use --disable-infobars switch in headless Chrome
>
> Disabling infobars in headless mode unconditionally has proven
> to be too aggressive. This CL changes infobars behavior in headless
> to be the same as in Chrome for Testing, which is: hide infobars
> only when --disable-infobars switch is present and only if the infobar
> has no buttons.
>
> Bug: 341947684, 333945848
> Change-Id: I8c544116d7650b3ebb7824406f328b6289494876
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5606442
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1313099}

Bug: 341947684, 333945848
Change-Id: I1fe1b09896badc62c3629bf28fb7c4623ae57252
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5618571
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Darren Shen <shend@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Darren Shen <shend@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1313119}
This commit is contained in:
Darren Shen
2024-06-10 23:16:42 +00:00
committed by Chromium LUCI CQ
parent 0aa706158f
commit 72b16e51a7
5 changed files with 43 additions and 170 deletions

@@ -25,20 +25,17 @@
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/headless/headless_mode_browsertest_utils.h"
#include "chrome/browser/headless/headless_mode_util.h"
#include "chrome/browser/infobars/confirm_infobar_creator.h"
#include "chrome/browser/process_singleton.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/page_info/page_info_infobar_delegate.h"
#include "chrome/browser/ui/views/frame/app_menu_button.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/common/chrome_switches.h"
#include "components/headless/clipboard/headless_clipboard.h" // nogncheck
#include "components/infobars/content/content_infobar_manager.h" // nogncheck
#include "components/infobars/core/confirm_infobar_delegate.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobars_switches.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
@@ -62,6 +59,8 @@
#include "ui/views/widget/widget.h"
#include "url/gurl.h"
using testing::Ge;
using testing::SizeIs;
using views::Widget;
namespace headless {
@@ -161,74 +160,17 @@ IN_PROC_BROWSER_TEST_F(HeadlessModeBrowserTest, BrowserWindowIsActive) {
EXPECT_TRUE(browser()->window()->IsActive());
}
// Infobar tests -------------------------------------------------------------
class TestInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
TestInfoBarDelegate(const TestInfoBarDelegate&) = delete;
TestInfoBarDelegate& operator=(const TestInfoBarDelegate&) = delete;
static void Create(infobars::ContentInfoBarManager* infobar_manager,
int buttons) {
infobar_manager->AddInfoBar(
CreateConfirmInfoBar(std::unique_ptr<ConfirmInfoBarDelegate>(
new TestInfoBarDelegate(buttons))));
}
// ConfirmInfoBarDelegate:
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override {
return TEST_INFOBAR;
}
std::u16string GetMessageText() const override {
return buttons_ ? u"BUTTON" : u"";
}
int GetButtons() const override { return buttons_; }
private:
explicit TestInfoBarDelegate(int buttons) : buttons_(buttons) {}
~TestInfoBarDelegate() override = default;
int buttons_;
};
class HeadlessModeInfobarBrowserTest
: public HeadlessModeBrowserTest,
public testing::WithParamInterface<bool> {
public:
HeadlessModeInfobarBrowserTest() = default;
~HeadlessModeInfobarBrowserTest() override = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
HeadlessModeBrowserTest::SetUpCommandLine(command_line);
if (disable_infobars()) {
command_line->AppendSwitch(::switches::kDisableInfoBars);
}
}
bool disable_infobars() const { return GetParam(); }
};
INSTANTIATE_TEST_SUITE_P(/* no prefix */,
HeadlessModeInfobarBrowserTest,
::testing::Bool());
IN_PROC_BROWSER_TEST_P(HeadlessModeInfobarBrowserTest, InfoBarsCanBeDisabled) {
IN_PROC_BROWSER_TEST_F(HeadlessModeBrowserTest, NoInfoBarInHeadless) {
content::WebContents* web_contents = GetActiveWebContents();
ASSERT_TRUE(web_contents);
infobars::ContentInfoBarManager* infobar_manager =
infobars::ContentInfoBarManager::FromWebContents(web_contents);
ASSERT_TRUE(infobar_manager);
ASSERT_THAT(infobar_manager->infobars(), testing::IsEmpty());
TestInfoBarDelegate::Create(infobar_manager,
ConfirmInfoBarDelegate::BUTTON_NONE);
TestInfoBarDelegate::Create(infobar_manager,
ConfirmInfoBarDelegate::BUTTON_OK);
PageInfoInfoBarDelegate::Create(infobar_manager);
// The infobar with a button should appear even if infobars are disabled.
EXPECT_THAT(infobar_manager->infobars(),
testing::SizeIs(disable_infobars() ? 1 : 2));
EXPECT_THAT(infobar_manager->infobars(), testing::IsEmpty());
}
// UserAgent tests -----------------------------------------------------------

@@ -10,22 +10,35 @@
#include "base/observer_list.h"
#include "base/ranges/algorithm.h"
#include "build/branding_buildflags.h"
#include "components/infobars/core/confirm_infobar_delegate.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobars_switches.h"
#include "ui/gfx/switches.h"
#if BUILDFLAG(CHROME_FOR_TESTING)
#include "components/infobars/core/confirm_infobar_delegate.h"
#include "components/infobars/core/infobars_switches.h"
#endif
namespace infobars {
namespace {
bool DisableInfoBars() {
const auto* const command_line = base::CommandLine::ForCurrentProcess();
// Infobars can only be disabled when Chrome is running in headless mode and
// in Chrome for Testing.
return command_line->HasSwitch(::switches::kDisableInfoBars) &&
(BUILDFLAG(CHROME_FOR_TESTING) ||
command_line->HasSwitch(::switches::kHeadless));
bool ShouldEnableInfoBars() {
// In headless mode info bars are not visible and cause unexpected layout
// changes which are often very confusing for headless users.
const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(::switches::kHeadless)) {
return false;
}
#if BUILDFLAG(CHROME_FOR_TESTING)
// Chrome for Testing users are allowed to disable info bars with a switch.
if (command_line->HasSwitch(switches::kDisableInfoBars)) {
return false;
}
#endif
return true;
}
} // namespace
@@ -123,7 +136,7 @@ void InfoBarManager::RemoveObserver(Observer* obs) {
observer_list_.RemoveObserver(obs);
}
InfoBarManager::InfoBarManager() : infobars_enabled_(!DisableInfoBars()) {}
InfoBarManager::InfoBarManager() : infobars_enabled_(ShouldEnableInfoBars()) {}
InfoBarManager::~InfoBarManager() = default;
@@ -175,12 +188,19 @@ bool InfoBarManager::ShouldShowInfoBar(const InfoBar* infobar) const {
return true;
}
// Only buttonless infobars should be disabled. The ones with buttons are
// semantically message boxes and must be shown because certain functionality
// depends on them, see crbug.com/333945848 and crbug.com/341947684.
const auto* const delegate = infobar->delegate()->AsConfirmInfoBarDelegate();
return delegate &&
delegate->GetButtons() != ConfirmInfoBarDelegate::BUTTON_NONE;
// Chrome for Testing can hide infobars that do not require confirmation using
// --disable-infobars command line switch.
#if BUILDFLAG(CHROME_FOR_TESTING)
ConfirmInfoBarDelegate* delegate =
infobar->delegate()->AsConfirmInfoBarDelegate();
if (delegate &&
delegate->GetButtons() != ConfirmInfoBarDelegate::BUTTON_NONE) {
return true;
}
#endif
// Headless mode and non confirmational Chrome for Testing are not shown.
return false;
}
} // namespace infobars

@@ -690,8 +690,6 @@ test("headless_browsertests") {
"//components/embedder_support:embedder_support",
"//components/headless/select_file_dialog",
"//components/headless/test",
"//components/infobars/content",
"//components/infobars/core",
"//components/policy/core/browser",
"//components/security_state/content",
"//components/version_info",

@@ -18,8 +18,6 @@ include_rules = [
]
specific_include_rules = {
"headless_browser_browsertest.*": [
"+components/infobars/core",
"+components/infobars/content",
"+storage/browser",
"+ui/shell_dialogs",
],

@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "headless/public/headless_browser.h"
#include <memory>
#include <string>
#include <tuple>
@@ -22,10 +20,6 @@
#include "build/build_config.h"
#include "components/devtools/simple_devtools_protocol_client/simple_devtools_protocol_client.h"
#include "components/headless/select_file_dialog/headless_select_file_dialog.h"
#include "components/infobars/content/content_infobar_manager.h"
#include "components/infobars/core/confirm_infobar_delegate.h"
#include "components/infobars/core/infobar.h"
#include "components/infobars/core/infobars_switches.h"
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/permission_controller_delegate.h"
@@ -39,6 +33,7 @@
#include "headless/lib/browser/headless_browser_context_impl.h"
#include "headless/lib/browser/headless_browser_impl.h"
#include "headless/lib/browser/headless_web_contents_impl.h"
#include "headless/public/headless_browser.h"
#include "headless/public/headless_web_contents.h"
#include "headless/public/switches.h"
#include "headless/test/headless_browser_test.h"
@@ -76,8 +71,6 @@ using testing::UnorderedElementsAre;
namespace headless {
namespace {
IN_PROC_BROWSER_TEST_F(HeadlessBrowserTest, CreateAndDestroyBrowserContext) {
HeadlessBrowserContext* browser_context =
browser()->CreateBrowserContextBuilder().Build();
@@ -884,82 +877,4 @@ IN_PROC_BROWSER_TEST_F(HeadlessBrowserTest, DISABLED_NetworkServiceCrash) {
} while (wc->GetController().GetLastCommittedEntry()->GetURL() != new_url);
}
// Infobar tests -------------------------------------------------------------
class TestInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
explicit TestInfoBarDelegate(int buttons) : buttons_(buttons) {}
TestInfoBarDelegate(const TestInfoBarDelegate&) = delete;
TestInfoBarDelegate& operator=(const TestInfoBarDelegate&) = delete;
~TestInfoBarDelegate() override = default;
static void Create(infobars::ContentInfoBarManager* infobar_manager,
int buttons) {
infobar_manager->AddInfoBar(std::make_unique<infobars::InfoBar>(
std::make_unique<TestInfoBarDelegate>(buttons)));
}
// ConfirmInfoBarDelegate:
infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override {
return TEST_INFOBAR;
}
std::u16string GetMessageText() const override {
return buttons_ ? u"BUTTON" : u"";
}
int GetButtons() const override { return buttons_; }
private:
int buttons_;
};
class HeadlessInfobarBrowserTest : public HeadlessBrowserTest,
public testing::WithParamInterface<bool> {
public:
HeadlessInfobarBrowserTest() = default;
~HeadlessInfobarBrowserTest() override = default;
void SetUpCommandLine(base::CommandLine* command_line) override {
HeadlessBrowserTest::SetUpCommandLine(command_line);
if (disable_infobars()) {
command_line->AppendSwitch(::switches::kDisableInfoBars);
}
}
bool disable_infobars() const { return GetParam(); }
};
INSTANTIATE_TEST_SUITE_P(/* no prefix */,
HeadlessInfobarBrowserTest,
::testing::Bool());
IN_PROC_BROWSER_TEST_P(HeadlessInfobarBrowserTest, InfoBarsCanBeDisabled) {
HeadlessBrowserContext* browser_context =
browser()->CreateBrowserContextBuilder().Build();
HeadlessWebContents* headless_web_contents =
browser_context->CreateWebContentsBuilder().Build();
ASSERT_TRUE(WaitForLoad(headless_web_contents));
content::WebContents* web_contents =
HeadlessWebContentsImpl::From(headless_web_contents)->web_contents();
ASSERT_TRUE(web_contents);
auto infobar_manager =
std::make_unique<infobars::ContentInfoBarManager>(web_contents);
ASSERT_THAT(infobar_manager->infobars(), testing::IsEmpty());
TestInfoBarDelegate::Create(infobar_manager.get(),
ConfirmInfoBarDelegate::BUTTON_NONE);
TestInfoBarDelegate::Create(infobar_manager.get(),
ConfirmInfoBarDelegate::BUTTON_OK);
// The infobar with a button should appear even if infobars are disabled.
EXPECT_THAT(infobar_manager->infobars(),
testing::SizeIs(disable_infobars() ? 1 : 2));
}
} // namespace
} // namespace headless