[M123] Toggle NTP Wallpaper Search button w/ Wallpaper Search setting
1. Toggles visibility of NTP Wallpaper Search button whenever
its setting is changed. *video demo in bug comments
We add SettingsEnabledObserver [1] to new_tab_page_handler.cc, and
communicate changes to settings via Mojo to the renderer.
2. Restructures wallpaper_search_interactive_uitest.cc into two
test suites. *The only "new" test is
|NTPWallpaperSearchButtonVisibilityDependsOnSettings|.
Setting changes are not observed if MockOptimizationGuideKeyedService is
being used [2], so we create a separate suite for tests that rely on
MockOptimizationGuideKeyedService.
[1]https://source.chromium.org/chromium/chromium/src/+/main:components/optimization_guide/core/model_execution/settings_enabled_observer.h
[2]https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/optimization_guide/optimization_guide_keyed_service.cc;drc=7484b91192bc20728f4c653f258694f72ba7c572;l=282
(cherry picked from commit 9387cff6b5e90fad3b7c1598ce224b8c7cf8f547)
Change-Id: If509a7b4c614741f4eddb87a132d05aa11dc29f2
Bug: 328085545
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5345524
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Paul Adedeji <pauladedeji@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1269015}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5361535
Auto-Submit: Paul Adedeji <pauladedeji@google.com>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/branch-heads/6312@{#522}
Cr-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
diff --git a/chrome/browser/resources/new_tab_page/app.ts b/chrome/browser/resources/new_tab_page/app.ts
index 972b3ea4..f44789d 100644
--- a/chrome/browser/resources/new_tab_page/app.ts
+++ b/chrome/browser/resources/new_tab_page/app.ts
@@ -395,6 +395,7 @@
private backgroundManager_: BackgroundManager;
private setThemeListenerId_: number|null = null;
private setCustomizeChromeSidePanelVisibilityListener_: number|null = null;
+ private setWallpaperSearchButtonVisibilityListener_: number|null = null;
private eventTracker_: EventTracker = new EventTracker();
private shouldPrintPerformance_: boolean;
private backgroundImageLoadStartEpoch_: number;
@@ -466,6 +467,11 @@
}
}
});
+ this.setWallpaperSearchButtonVisibilityListener_ =
+ this.callbackRouter_.setWallpaperSearchButtonVisibility.addListener(
+ (visible: boolean) => {
+ this.wallpaperSearchButtonEnabled_ = visible;
+ });
// Open Customize Chrome if there are Customize Chrome URL params.
if (this.showCustomize_) {
@@ -517,6 +523,8 @@
this.callbackRouter_.removeListener(
this.setCustomizeChromeSidePanelVisibilityListener_!);
this.callbackRouter_.removeListener(this.showWebstoreToastListenerId_!);
+ this.callbackRouter_.removeListener(
+ this.setWallpaperSearchButtonVisibilityListener_!);
this.eventTracker_.removeAll();
}
diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom b/chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom
index 2bf3da8..d158e0d7 100644
--- a/chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom
+++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom
@@ -381,4 +381,6 @@
SetPromo(Promo? promo);
// Shows a toast with information about Chrome Webstore themes.
ShowWebstoreToast();
+ // Sets wallpaper search button's visibility to to |visible|.
+ SetWallpaperSearchButtonVisibility(bool visible);
};
diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc
index 911eede..39897ca 100644
--- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc
+++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc
@@ -35,6 +35,7 @@
#include "chrome/browser/new_tab_page/feature_promo_helper/new_tab_page_feature_promo_helper.h"
#include "chrome/browser/new_tab_page/modules/new_tab_page_modules.h"
#include "chrome/browser/new_tab_page/promos/promo_service_factory.h"
+#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/background/ntp_background_service.h"
#include "chrome/browser/search/background/ntp_background_service_factory.h"
@@ -467,7 +468,9 @@
customize_chrome_feature_promo_helper,
const base::Time& ntp_navigation_start_time,
const std::vector<std::pair<const std::string, int>>* module_id_names)
- : ntp_background_service_(
+ : SettingsEnabledObserver(optimization_guide::proto::ModelExecutionFeature::
+ MODEL_EXECUTION_FEATURE_WALLPAPER_SEARCH),
+ ntp_background_service_(
NtpBackgroundServiceFactory::GetForProfile(profile)),
ntp_custom_background_service_(ntp_custom_background_service),
logo_service_(logo_service),
@@ -499,6 +502,14 @@
ntp_custom_background_service_observation_.Observe(
ntp_custom_background_service_.get());
promo_service_observation_.Observe(promo_service_.get());
+ if (customize_chrome::IsWallpaperSearchEnabledForProfile(profile_)) {
+ optimization_guide_keyed_service_ =
+ OptimizationGuideKeyedServiceFactory::GetForProfile(profile_);
+ if (optimization_guide_keyed_service_) {
+ optimization_guide_keyed_service_
+ ->AddModelExecutionSettingsEnabledObserver(this);
+ }
+ }
if (base::FeatureList::IsEnabled(
ntp_features::kNtpBackgroundImageErrorDetection)) {
ntp_custom_background_service_->VerifyCustomBackgroundImageURL();
@@ -539,6 +550,11 @@
if (select_file_dialog_) {
select_file_dialog_->ListenerDestroyed();
}
+ if (optimization_guide_keyed_service_) {
+ optimization_guide_keyed_service_
+ ->RemoveModelExecutionSettingsEnabledObserver(this);
+ optimization_guide_keyed_service_ = nullptr;
+ }
}
// static
@@ -1052,6 +1068,11 @@
promo_service_ = nullptr;
}
+void NewTabPageHandler::OnChangeInFeatureCurrentlyEnabledState(
+ bool is_now_enabled) {
+ page_->SetWallpaperSearchButtonVisibility(is_now_enabled);
+}
+
void NewTabPageHandler::OnAppRendered(double time) {
logger_.LogEvent(NTP_APP_RENDERED,
base::Time::FromMillisecondsSinceUnixEpoch(time) -
diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h
index 9bd9dfe..0464324c 100644
--- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h
+++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h
@@ -19,6 +19,7 @@
#include "chrome/browser/new_tab_page/feature_promo_helper/new_tab_page_feature_promo_helper.h"
#include "chrome/browser/new_tab_page/promos/promo_service.h"
#include "chrome/browser/new_tab_page/promos/promo_service_observer.h"
+#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "chrome/browser/search/background/ntp_background_service_observer.h"
#include "chrome/browser/search/background/ntp_custom_background_service.h"
#include "chrome/browser/search/background/ntp_custom_background_service_observer.h"
@@ -27,6 +28,7 @@
#include "chrome/browser/ui/search/ntp_user_data_logger.h"
#include "chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom.h"
#include "chrome/common/search/ntp_logging_events.h"
+#include "components/optimization_guide/core/model_execution/settings_enabled_observer.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/search_provider_logos/logo_common.h"
#include "content/public/browser/web_contents_observer.h"
@@ -62,7 +64,8 @@
public NtpCustomBackgroundServiceObserver,
public NtpBackgroundServiceObserver,
public ui::SelectFileDialog::Listener,
- public PromoServiceObserver {
+ public PromoServiceObserver,
+ public optimization_guide::SettingsEnabledObserver {
public:
NewTabPageHandler(
mojo::PendingReceiver<new_tab_page::mojom::PageHandler>
@@ -172,6 +175,9 @@
void OnPromoDataUpdated() override;
void OnPromoServiceShuttingDown() override;
+ // SettingsEnabledObserver:
+ void OnChangeInFeatureCurrentlyEnabledState(bool is_now_enabled) override;
+
// SelectFileDialog::Listener:
void FileSelected(const ui::SelectedFileInfo& file,
int index,
@@ -241,6 +247,7 @@
loader_map_;
PrefChangeRegistrar pref_change_registrar_;
raw_ptr<PromoService> promo_service_;
+ raw_ptr<OptimizationGuideKeyedService> optimization_guide_keyed_service_;
base::ScopedObservation<ui::NativeTheme, ui::NativeThemeObserver>
native_theme_observation_{this};
base::ScopedObservation<ThemeService, ThemeServiceObserver>
diff --git a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc
index 50ad55d8..f4f56ea 100644
--- a/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc
+++ b/chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc
@@ -103,6 +103,7 @@
MOCK_METHOD(void, SetCustomizeChromeSidePanelVisibility, (bool));
MOCK_METHOD(void, SetPromo, (new_tab_page::mojom::PromoPtr));
MOCK_METHOD(void, ShowWebstoreToast, ());
+ MOCK_METHOD(void, SetWallpaperSearchButtonVisibility, (bool));
mojo::Receiver<new_tab_page::mojom::Page> receiver_{this};
};
diff --git a/chrome/browser/ui/webui/side_panel/customize_chrome/wallpaper_search/wallpaper_search_interactive_uitest.cc b/chrome/browser/ui/webui/side_panel/customize_chrome/wallpaper_search/wallpaper_search_interactive_uitest.cc
index 6b4f595..692ff38 100644
--- a/chrome/browser/ui/webui/side_panel/customize_chrome/wallpaper_search/wallpaper_search_interactive_uitest.cc
+++ b/chrome/browser/ui/webui/side_panel/customize_chrome/wallpaper_search/wallpaper_search_interactive_uitest.cc
@@ -6,6 +6,7 @@
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/optimization_guide/mock_optimization_guide_keyed_service.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/ui/browser_element_identifiers.h"
#include "chrome/browser/ui/webui/feedback/feedback_dialog.h"
@@ -14,6 +15,7 @@
#include "chrome/test/interaction/interactive_browser_test.h"
#include "chrome/test/interaction/webcontents_interaction_test_util.h"
#include "components/optimization_guide/core/optimization_guide_features.h"
+#include "components/optimization_guide/core/optimization_guide_prefs.h"
#include "components/search/ntp_features.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
@@ -30,16 +32,8 @@
DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kCustomizeChromeElementId);
} // namespace
-// Disabled due to failures; see https://crbug.com/325721633.
-class DISABLED_WallpaperSearchInteractiveTest : public InteractiveBrowserTest {
+class WallpaperSearchInteractiveTest : public InteractiveBrowserTest {
public:
- DISABLED_WallpaperSearchInteractiveTest() {
- subscription_ =
- BrowserContextDependencyManager::GetInstance()
- ->RegisterCreateServicesCallbackForTesting(base::BindRepeating(
- RegisterMockOptimizationGuideKeyedServiceFactory));
- }
- ~DISABLED_WallpaperSearchInteractiveTest() override = default;
void SetUp() override {
scoped_feature_list_.InitWithFeatures(
@@ -59,6 +53,93 @@
IdentityManagerFactory::GetForProfile(browser()->profile());
signin::MakePrimaryAccountAvailable(identity_manager, "user@example.com",
signin::ConsentLevel::kSignin);
+ }
+
+ InteractiveTestApi::MultiStep WaitForElementVisible(
+ const ui::ElementIdentifier& contents_id,
+ const DeepQuery& element) {
+ DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE(kElementVisibleEvent);
+ StateChange element_visible;
+ element_visible.type = StateChange::Type::kExistsAndConditionTrue;
+ element_visible.where = element;
+ element_visible.event = kElementVisibleEvent;
+ element_visible.test_function = "(el) => el.offsetParent !== null";
+
+ return WaitForStateChange(contents_id, element_visible);
+ }
+
+ private:
+ base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(WallpaperSearchInteractiveTest,
+ NTPWallpaperSearchButtonVisibilityDependsOnSettings) {
+ const DeepQuery kWallpaperSearchButtonContainer = {
+ "ntp-app", "#wallpaperSearchButtonContainer"};
+
+ DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE(kElementHiddenEvent);
+ StateChange wallpaper_search_button_hidden;
+ wallpaper_search_button_hidden.type =
+ StateChange::Type::kExistsAndConditionTrue;
+ wallpaper_search_button_hidden.where = kWallpaperSearchButtonContainer;
+ wallpaper_search_button_hidden.event = kElementHiddenEvent;
+ wallpaper_search_button_hidden.test_function =
+ "(el) => el.offsetParent === null";
+
+ RunTestSequence(
+ // 1. Open the NTP.
+ Steps(InstrumentTab(kNewTabPageElementId, 0), Do([=]() {
+ browser()->profile()->GetPrefs()->SetInteger(
+ optimization_guide::prefs::GetSettingEnabledPrefName(
+ optimization_guide::proto::ModelExecutionFeature::
+ MODEL_EXECUTION_FEATURE_WALLPAPER_SEARCH),
+ static_cast<int>(
+ optimization_guide::prefs::FeatureOptInState::kEnabled));
+ }),
+ NavigateWebContents(kNewTabPageElementId,
+ GURL(chrome::kChromeUINewTabPageURL)),
+ WaitForWebContentsReady(kNewTabPageElementId,
+ GURL(chrome::kChromeUINewTabPageURL))),
+ // 2. Ensure the wallpaper search button is visible.
+ WaitForElementVisible(kNewTabPageElementId,
+ kWallpaperSearchButtonContainer),
+ // 3. Turn wallpaper search setting off.
+ Do([=]() {
+ browser()->profile()->GetPrefs()->SetInteger(
+ optimization_guide::prefs::GetSettingEnabledPrefName(
+ optimization_guide::proto::ModelExecutionFeature::
+ MODEL_EXECUTION_FEATURE_WALLPAPER_SEARCH),
+ static_cast<int>(
+ optimization_guide::prefs::FeatureOptInState::kDisabled));
+ }),
+ // 4. Ensure the wallpaper search button is hidden.
+ WaitForStateChange(kNewTabPageElementId, wallpaper_search_button_hidden),
+ // 5. Turn wallpaper search setting on.
+ Do([=]() {
+ browser()->profile()->GetPrefs()->SetInteger(
+ optimization_guide::prefs::GetSettingEnabledPrefName(
+ optimization_guide::proto::ModelExecutionFeature::
+ MODEL_EXECUTION_FEATURE_WALLPAPER_SEARCH),
+ static_cast<int>(
+ optimization_guide::prefs::FeatureOptInState::kEnabled));
+ }),
+ // 6. Ensure the wallpaper search button is visible.
+ WaitForElementVisible(kNewTabPageElementId,
+ kWallpaperSearchButtonContainer));
+}
+
+class WallpaperSearchOptimizationGuideInteractiveTest
+ : public WallpaperSearchInteractiveTest {
+ public:
+ WallpaperSearchOptimizationGuideInteractiveTest() {
+ subscription_ =
+ BrowserContextDependencyManager::GetInstance()
+ ->RegisterCreateServicesCallbackForTesting(base::BindRepeating(
+ RegisterMockOptimizationGuideKeyedServiceFactory));
+ }
+
+ void SetUpOnMainThread() override {
+ WallpaperSearchInteractiveTest::SetUpOnMainThread();
mock_optimization_guide_keyed_service_ =
static_cast<testing::NiceMock<MockOptimizationGuideKeyedService>*>(
OptimizationGuideKeyedServiceFactory::GetForProfile(
@@ -106,31 +187,23 @@
InteractiveTestApi::MultiStep ClickElement(
const ui::ElementIdentifier& contents_id,
const DeepQuery& element) {
- DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE(kElementVisibleEvent);
- StateChange element_visible;
- element_visible.type = StateChange::Type::kExistsAndConditionTrue;
- element_visible.where = element;
- element_visible.event = kElementVisibleEvent;
- element_visible.test_function = "(el) => el.offsetParent !== null";
-
- return Steps(WaitForStateChange(contents_id, element_visible),
+ return Steps(WaitForElementVisible(contents_id, element),
MoveMouseTo(contents_id, element), ClickMouse());
}
InteractiveTestApi::MultiStep OpenNewTabPage() {
- return Steps(InstrumentTab(kNewTabPageElementId, 0),
- NavigateWebContents(kNewTabPageElementId,
- GURL(chrome::kChromeUINewTabPageURL)),
- WaitForWebContentsReady(kNewTabPageElementId,
- GURL(chrome::kChromeUINewTabPageURL)),
- Do([this]() {
+ return Steps(InstrumentTab(kNewTabPageElementId, 0), Do([this]() {
ON_CALL(
mock_optimization_guide_keyed_service(),
ShouldFeatureBeCurrentlyEnabledForUser(
optimization_guide::proto::ModelExecutionFeature::
MODEL_EXECUTION_FEATURE_WALLPAPER_SEARCH))
.WillByDefault(testing::Return(true));
- }));
+ }),
+ NavigateWebContents(kNewTabPageElementId,
+ GURL(chrome::kChromeUINewTabPageURL)),
+ WaitForWebContentsReady(kNewTabPageElementId,
+ GURL(chrome::kChromeUINewTabPageURL)));
}
InteractiveTestApi::MultiStep OpenCustomizeChromeAt(
@@ -203,13 +276,12 @@
}));
}
- base::test::ScopedFeatureList scoped_feature_list_;
- base::CallbackListSubscription subscription_;
raw_ptr<testing::NiceMock<MockOptimizationGuideKeyedService>>
mock_optimization_guide_keyed_service_;
+ base::CallbackListSubscription subscription_;
};
-IN_PROC_BROWSER_TEST_F(DISABLED_WallpaperSearchInteractiveTest,
+IN_PROC_BROWSER_TEST_F(WallpaperSearchOptimizationGuideInteractiveTest,
CustomizeButtonsWorkTogether) {
DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kReopenedCustomizeChromeElementId);
@@ -233,7 +305,7 @@
WaitForHide(kCustomizeChromeSidePanelWebViewElementId)));
}
-IN_PROC_BROWSER_TEST_F(DISABLED_WallpaperSearchInteractiveTest,
+IN_PROC_BROWSER_TEST_F(WallpaperSearchOptimizationGuideInteractiveTest,
SearchesAndSetsNewAndHistoricalResults) {
// Intercept Wallpaper Search descriptor fetches, and respond with data.
std::unique_ptr<content::URLLoaderInterceptor> descriptors_fetch_interceptor =
@@ -318,7 +390,7 @@
// which cannot be easily tested here. LaCrOS has a separate feedback
// browser test which gives us some coverage.
#if !BUILDFLAG(IS_CHROMEOS)
-IN_PROC_BROWSER_TEST_F(DISABLED_WallpaperSearchInteractiveTest,
+IN_PROC_BROWSER_TEST_F(WallpaperSearchOptimizationGuideInteractiveTest,
FeedbackDialogShowsOnThumbsDown) {
EXPECT_CALL(mock_optimization_guide_keyed_service(),
ShouldFeatureBeCurrentlyAllowedForLogging(
diff --git a/chrome/test/data/webui/new_tab_page/app_test.ts b/chrome/test/data/webui/new_tab_page/app_test.ts
index 524c3b3..26e099d 100644
--- a/chrome/test/data/webui/new_tab_page/app_test.ts
+++ b/chrome/test/data/webui/new_tab_page/app_test.ts
@@ -1370,6 +1370,30 @@
32,
$$<HTMLElement>(app, '#customizeButtonContainer')!.offsetWidth);
});
+
+ test(
+ 'button hides/shows in accordance with callback router', async () => {
+ assertNotStyle(
+ $$(app, '#customizeButtonContainer')!, 'display', 'none');
+ assertNotStyle(
+ $$(app, '#wallpaperSearchButtonContainer')!, 'display', 'none');
+
+ callbackRouterRemote.setWallpaperSearchButtonVisibility(false);
+ await callbackRouterRemote.$.flushForTesting();
+
+ assertNotStyle(
+ $$(app, '#customizeButtonContainer')!, 'display', 'none');
+ assertStyle(
+ $$(app, '#wallpaperSearchButtonContainer')!, 'display', 'none');
+
+ callbackRouterRemote.setWallpaperSearchButtonVisibility(true);
+ await callbackRouterRemote.$.flushForTesting();
+
+ assertNotStyle(
+ $$(app, '#customizeButtonContainer')!, 'display', 'none');
+ assertNotStyle(
+ $$(app, '#wallpaperSearchButtonContainer')!, 'display', 'none');
+ });
});
suite('AnimationDisabled', () => {