diff --git a/browser/ui/views/location_bar/BUILD.gn b/browser/ui/views/location_bar/BUILD.gn index f7d5b26598e..3ed9730ad63 100644 --- a/browser/ui/views/location_bar/BUILD.gn +++ b/browser/ui/views/location_bar/BUILD.gn @@ -47,9 +47,11 @@ source_set("location_bar") { "//chrome/browser/ui/color:mixers", "//chrome/browser/ui/omnibox", "//chrome/browser/ui/tabs:tab_strip", + "//chrome/browser/ui/user_education", "//chrome/browser/ui/views/location_bar", "//chrome/browser/ui/views/page_action", "//chrome/browser/ui/views/page_info", + "//components/feature_engagement/public:feature_constants", "//components/image_fetcher/core", "//components/search_engines", "//components/version_info:channel", diff --git a/browser/ui/views/location_bar/brave_location_bar_view.cc b/browser/ui/views/location_bar/brave_location_bar_view.cc index 57298298fbc..c78d9705132 100644 --- a/browser/ui/views/location_bar/brave_location_bar_view.cc +++ b/browser/ui/views/location_bar/brave_location_bar_view.cc @@ -145,8 +145,7 @@ void BraveLocationBarView::Init() { if (page_info::features::IsShowBraveShieldsInPageInfoEnabled()) { shields_page_info_controller_ = - std::make_unique( - browser_->GetTabStripModel(), location_icon_view()); + std::make_unique(location_icon_view()); } // brave action buttons @@ -198,6 +197,10 @@ void BraveLocationBarView::Update(content::WebContents* contents) { } #endif + if (shields_page_info_controller_) { + shields_page_info_controller_->UpdateWebContents(contents); + } + LocationBarView::Update(contents); } diff --git a/browser/ui/views/location_bar/brave_shields_page_info_controller.cc b/browser/ui/views/location_bar/brave_shields_page_info_controller.cc index 0ac8b133911..a0735ee3dcd 100644 --- a/browser/ui/views/location_bar/brave_shields_page_info_controller.cc +++ b/browser/ui/views/location_bar/brave_shields_page_info_controller.cc @@ -8,14 +8,18 @@ #include "base/check_deref.h" #include "base/task/sequenced_task_runner.h" #include "brave/browser/ui/views/page_info/brave_page_info_bubble_view.h" -#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/user_education/browser_user_education_interface.h" #include "chrome/browser/ui/views/location_bar/location_icon_view.h" #include "chrome/browser/ui/views/page_info/page_info_bubble_specification.h" #include "chrome/browser/ui/views/page_info/page_info_bubble_view.h" +#include "components/content_settings/core/browser/content_settings_utils.h" +#include "components/feature_engagement/public/feature_constants.h" +#include "components/page_info/page_info.h" #include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" #include "ui/views/view_utils.h" +#include "url/gurl.h" namespace { @@ -30,36 +34,37 @@ brave_shields::BraveShieldsTabHelper* GetShieldsHelper( } // namespace BraveShieldsPageInfoController::BraveShieldsPageInfoController( - TabStripModel* tab_strip_model, LocationIconView* location_icon_view) - : tab_strip_model_(CHECK_DEREF(tab_strip_model)), - location_icon_view_(CHECK_DEREF(location_icon_view)) { - tab_strip_model_->AddObserver(this); - auto* web_contents = tab_strip_model_->GetActiveWebContents(); - if (auto* shields_helper = GetShieldsHelper(web_contents)) { - shields_helper->AddObserver(this); - } -} + : location_icon_view_(CHECK_DEREF(location_icon_view)) {} BraveShieldsPageInfoController::~BraveShieldsPageInfoController() { - auto* web_contents = tab_strip_model_->GetActiveWebContents(); - if (auto* shields_helper = GetShieldsHelper(web_contents)) { + if (auto* shields_helper = GetShieldsHelper(web_contents())) { shields_helper->RemoveObserver(this); } } -void BraveShieldsPageInfoController::OnTabStripModelChanged( - TabStripModel* tab_strip_model, - const TabStripModelChange& change, - const TabStripSelectionChange& selection) { - if (selection.active_tab_changed()) { - if (auto* shields_helper = GetShieldsHelper(selection.old_contents)) { - shields_helper->RemoveObserver(this); - } - if (auto* shields_helper = GetShieldsHelper(selection.new_contents)) { - shields_helper->AddObserver(this); - } +void BraveShieldsPageInfoController::UpdateWebContents( + content::WebContents* contents) { + if (!contents || contents == web_contents()) { + return; } + + // Stop observing the shields tab helper for the old WebContents. + if (auto* shields_helper = GetShieldsHelper(web_contents())) { + shields_helper->RemoveObserver(this); + } + + // Start observing the new WebContents and its shields helper. + Observe(contents); + if (auto* shields_helper = GetShieldsHelper(contents)) { + shields_helper->AddObserver(this); + } + + MaybeShowShieldsIPH(); +} + +void BraveShieldsPageInfoController::PrimaryPageChanged(content::Page& page) { + MaybeShowShieldsIPH(); } void BraveShieldsPageInfoController::OnResourcesChanged() {} @@ -76,14 +81,40 @@ void BraveShieldsPageInfoController::OnRepeatedReloadsDetected() { weak_factory_.GetWeakPtr())); } -void BraveShieldsPageInfoController::ShowBubbleForRepeatedReloads() { - auto* web_contents = tab_strip_model_->GetActiveWebContents(); - if (!web_contents) { +void BraveShieldsPageInfoController::MaybeShowShieldsIPH() { + if (!web_contents()) { return; } content::NavigationEntry* entry = - web_contents->GetController().GetVisibleEntry(); + web_contents()->GetController().GetVisibleEntry(); + if (entry->IsInitialEntry()) { + return; + } + + // Only attempt to show the IPH if the "normal" PageInfo bubble will be + // displayed for this URL. + GURL url = entry->GetVirtualURL(); + if (PageInfo::IsFileOrInternalPage(url) || + url.SchemeIs(content_settings::kExtensionScheme)) { + return; + } + + if (auto* user_education = + BrowserUserEducationInterface::MaybeGetForWebContentsInTab( + web_contents())) { + user_education->MaybeShowFeaturePromo( + feature_engagement::kIPHBraveShieldsInPageInfoFeature); + } +} + +void BraveShieldsPageInfoController::ShowBubbleForRepeatedReloads() { + if (!web_contents()) { + return; + } + + content::NavigationEntry* entry = + web_contents()->GetController().GetVisibleEntry(); if (!entry || entry->IsInitialEntry()) { return; } @@ -91,7 +122,7 @@ void BraveShieldsPageInfoController::ShowBubbleForRepeatedReloads() { std::unique_ptr specification = PageInfoBubbleSpecification::Builder( &location_icon_view_.get(), - location_icon_view_->GetWidget()->GetNativeWindow(), web_contents, + location_icon_view_->GetWidget()->GetNativeWindow(), web_contents(), entry->GetVirtualURL()) .Build(); diff --git a/browser/ui/views/location_bar/brave_shields_page_info_controller.h b/browser/ui/views/location_bar/brave_shields_page_info_controller.h index 0035662764b..53f64db2b94 100644 --- a/browser/ui/views/location_bar/brave_shields_page_info_controller.h +++ b/browser/ui/views/location_bar/brave_shields_page_info_controller.h @@ -9,19 +9,17 @@ #include "base/memory/raw_ref.h" #include "base/memory/weak_ptr.h" #include "brave/browser/brave_shields/brave_shields_tab_helper.h" -#include "chrome/browser/ui/tabs/tab_strip_model_observer.h" +#include "content/public/browser/web_contents_observer.h" class LocationIconView; -class TabStripModel; // Controller that listens for Brave Shields events for the current active tab // and opens the Page Info bubble when appropriate. class BraveShieldsPageInfoController : public brave_shields::BraveShieldsTabHelper::Observer, - public TabStripModelObserver { + public content::WebContentsObserver { public: - BraveShieldsPageInfoController(TabStripModel* tab_strip_model, - LocationIconView* location_icon_view); + explicit BraveShieldsPageInfoController(LocationIconView* location_icon_view); BraveShieldsPageInfoController(const BraveShieldsPageInfoController&) = delete; @@ -30,20 +28,19 @@ class BraveShieldsPageInfoController ~BraveShieldsPageInfoController() override; - // TabStripModelObserver: - void OnTabStripModelChanged( - TabStripModel* tab_strip_model, - const TabStripModelChange& change, - const TabStripSelectionChange& selection) override; + void UpdateWebContents(content::WebContents* web_contents); + + // content::WebContentsObserver: + void PrimaryPageChanged(content::Page& page) override; // brave_shields::BraveShieldsTabHelper::Observer: void OnResourcesChanged() override; void OnRepeatedReloadsDetected() override; private: + void MaybeShowShieldsIPH(); void ShowBubbleForRepeatedReloads(); - raw_ref tab_strip_model_; raw_ref location_icon_view_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/chromium_src/chrome/browser/ui/views/user_education/browser_user_education_service.cc b/chromium_src/chrome/browser/ui/views/user_education/browser_user_education_service.cc new file mode 100644 index 00000000000..bf4d2b19dc6 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/user_education/browser_user_education_service.cc @@ -0,0 +1,53 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/ui/views/user_education/browser_user_education_service.h" + +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser_element_identifiers.h" +#include "components/feature_engagement/public/feature_constants.h" +#include "components/grit/brave_components_strings.h" +#include "components/user_education/common/feature_promo/feature_promo_registry.h" +#include "components/user_education/common/feature_promo/feature_promo_specification.h" + +// This override allows Brave to register its own in-product help features with +// the user education system's feature promo registry. + +namespace { + +// Registers Brave-specific in-product help features. +void MaybeRegisterBraveFeaturePromos( + user_education::FeaturePromoRegistry& registry, + Profile* profile) { + using user_education::FeaturePromoSpecification; + using user_education::HelpBubbleArrow; + + // IPH for Brave Shields in Page Info. + registry.RegisterFeature(std::move( + FeaturePromoSpecification::CreateForToastPromo( + feature_engagement::kIPHBraveShieldsInPageInfoFeature, + kLocationIconElementId, IDS_BRAVE_SHIELDS_PAGE_INFO_IPH_TEXT, + IDS_BRAVE_SHIELDS_PAGE_INFO_IPH_SCREENREADER_TEXT, + FeaturePromoSpecification::AcceleratorInfo()) + .SetBubbleArrow(HelpBubbleArrow::kBottomCenter) + .SetMetadata(144, "ksmith@brave.com", + "Shown when user visits a site for the first time after " + "the Shields UI was moved to Page Info. Educates users " + "about the new location."))); +} + +} // namespace + +// BRAVE_MAYBE_REGISTER_CHROME_FEATURE_PROMOS is patched near the beginning of +// MaybeRegisterChromeFeaturePromos, and allows us to register Brave-specific +// in-product help features. Patching is necessary because the token +// "MaybeRegisterChromeFeaturePromos" appears several times in the source file +// in different contexts. +#define BRAVE_MAYBE_REGISTER_CHROME_FEATURE_PROMOS \ + MaybeRegisterBraveFeaturePromos(registry, profile); + +#include + +#undef BRAVE_MAYBE_REGISTER_CHROME_FEATURE_PROMOS diff --git a/chromium_src/components/feature_engagement/public/feature_constants.cc b/chromium_src/components/feature_engagement/public/feature_constants.cc index a8e9be2be35..a137a931de9 100644 --- a/chromium_src/components/feature_engagement/public/feature_constants.cc +++ b/chromium_src/components/feature_engagement/public/feature_constants.cc @@ -5,10 +5,19 @@ #include "base/feature_override.h" +// This override allows Brave to define its own IPH features, and to modify the +// IPH features defined by Chromium. + #include namespace feature_engagement { +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) +BASE_FEATURE(kIPHBraveShieldsInPageInfoFeature, + "IPH_BraveShieldsInPageInfo", + base::FEATURE_ENABLED_BY_DEFAULT); +#endif + OVERRIDE_FEATURE_DEFAULT_STATES({{ #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_APPLE) || BUILDFLAG(IS_LINUX) {kIPHGMCCastStartStopFeature, base::FEATURE_DISABLED_BY_DEFAULT}, diff --git a/chromium_src/components/feature_engagement/public/feature_constants.h b/chromium_src/components/feature_engagement/public/feature_constants.h new file mode 100644 index 00000000000..097beb899d3 --- /dev/null +++ b/chromium_src/components/feature_engagement/public/feature_constants.h @@ -0,0 +1,23 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_CONSTANTS_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_CONSTANTS_H_ + +// This override allows Brave to declare its own IPH features. Features declared +// here must be defined in "feature_constants.cc". + +#include // IWYU pragma: export + +namespace feature_engagement { + +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) +// IPH for notifying users that Brave Shields settings have moved to Page Info. +FEATURE_CONSTANTS_DECLARE_FEATURE(kIPHBraveShieldsInPageInfoFeature); +#endif + +} // namespace feature_engagement + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_CONSTANTS_H_ diff --git a/chromium_src/components/feature_engagement/public/feature_list.cc b/chromium_src/components/feature_engagement/public/feature_list.cc new file mode 100644 index 00000000000..58eeb4159f4 --- /dev/null +++ b/chromium_src/components/feature_engagement/public/feature_list.cc @@ -0,0 +1,19 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#include "components/feature_engagement/public/feature_list.h" + +// This override adds Brave-specific IPH features to the kAllFeatures array, +// which is used by the feature engagement tracker and GetAllFeatures(). + +// Replaces the first entry in the kAllFeatures array with that entry, plus any +// additional entries for Brave-specific IPH features. +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) +#define kIPHDummyFeature kIPHDummyFeature, &kIPHBraveShieldsInPageInfoFeature +#endif + +#include + +#undef kIPHDummyFeature diff --git a/chromium_src/components/feature_engagement/public/feature_list.h b/chromium_src/components/feature_engagement/public/feature_list.h new file mode 100644 index 00000000000..f8af5c0fdcc --- /dev/null +++ b/chromium_src/components/feature_engagement/public/feature_list.h @@ -0,0 +1,41 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_LIST_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_LIST_H_ + +#include "build/build_config.h" + +// This override allows Brave to display Brave-specific in-product help features +// on the brave://flags page. + +// BRAVE_FEATURE_ENGAGEMENT_VARIATION_PARAMS is patched in after Chromium's +// variation params are defined, and allows us to define Brave-specific IPH +// variation params. A patch is necessary because the DEFINE_VARIATION_PARAM +// macro is undef'ed at the end of the header file. +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) +#define BRAVE_FEATURE_ENGAGEMENT_VARIATION_PARAMS \ + DEFINE_VARIATION_PARAM(kIPHBraveShieldsInPageInfoFeature, \ + "IPH_BraveShieldsInPageInfo"); +#else +#define BRAVE_FEATURE_ENGAGEMENT_VARIATION_PARAMS +#endif + +// BRAVE_FEATURE_ENGAGEMENT_VARIATION_ENTRIES is patched in at the start of the +// kIPHDemoModeChoiceVariations array, and allows us to add Brave-specific IPH +// variation entries to the array. +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) +#define BRAVE_FEATURE_ENGAGEMENT_VARIATION_ENTRIES \ + VARIATION_ENTRY(kIPHBraveShieldsInPageInfoFeature), +#else +#define BRAVE_FEATURE_ENGAGEMENT_VARIATION_ENTRIES +#endif + +#include // IWYU pragma: export + +#undef BRAVE_FEATURE_ENGAGEMENT_VARIATION_PARAMS +#undef BRAVE_FEATURE_ENGAGEMENT_VARIATION_ENTRIES + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_FEATURE_ENGAGEMENT_PUBLIC_FEATURE_LIST_H_ diff --git a/components/resources/brave_shields_strings.grdp b/components/resources/brave_shields_strings.grdp index dc805da36ea..53c2a360658 100644 --- a/components/resources/brave_shields_strings.grdp +++ b/components/resources/brave_shields_strings.grdp @@ -311,4 +311,13 @@ Site Info + + + Brave Shields has a new home and design. It now integrates with site information for a one-stop experience. + + + + Brave Shields has a new home and design. Access it by clicking the site icon in the address bar. + + diff --git a/patches/chrome-browser-ui-views-user_education-browser_user_education_service.cc.patch b/patches/chrome-browser-ui-views-user_education-browser_user_education_service.cc.patch new file mode 100644 index 00000000000..3acb9bededa --- /dev/null +++ b/patches/chrome-browser-ui-views-user_education-browser_user_education_service.cc.patch @@ -0,0 +1,12 @@ +diff --git a/chrome/browser/ui/views/user_education/browser_user_education_service.cc b/chrome/browser/ui/views/user_education/browser_user_education_service.cc +index 5327bdb635e01bfe9a27da72f51dae9920d9d887..6e52d9f395731d2d746c0039107f12e6fb0f1326 100644 +--- a/chrome/browser/ui/views/user_education/browser_user_education_service.cc ++++ b/chrome/browser/ui/views/user_education/browser_user_education_service.cc +@@ -268,6 +268,7 @@ void MaybeRegisterChromeFeaturePromos( + feature_engagement::kIPHWebUiHelpBubbleTestFeature)) { + return; + } ++ BRAVE_MAYBE_REGISTER_CHROME_FEATURE_PROMOS + + // kIPHAutofillCreditCardBenefitFeature: + registry.RegisterFeature(std::move( diff --git a/patches/components-feature_engagement-public-feature_list.h.patch b/patches/components-feature_engagement-public-feature_list.h.patch new file mode 100644 index 00000000000..db84f42018b --- /dev/null +++ b/patches/components-feature_engagement-public-feature_list.h.patch @@ -0,0 +1,19 @@ +diff --git a/components/feature_engagement/public/feature_list.h b/components/feature_engagement/public/feature_list.h +index 000a1d75f5b84b9fcd62c25df22eb8375f3b291b..7a2e62f2e3d63995ef0b6c9c6e7928babcbbe654 100644 +--- a/components/feature_engagement/public/feature_list.h ++++ b/components/feature_engagement/public/feature_list.h +@@ -607,12 +607,14 @@ DEFINE_VARIATION_PARAM(kIPHiOSLensPromoDesktopFeature, + DEFINE_VARIATION_PARAM(kIPHiOSEnhancedBrowsingDesktopFeature, + "IPH_iOSEnhancedBrowsingDesktop"); + #endif // !BUILDFLAG(IS_ANDROID) ++BRAVE_FEATURE_ENGAGEMENT_VARIATION_PARAMS + + // Defines the array of which features should be listed in the chrome://flags + // UI to be able to select them alone for demo-mode. The features listed here + // are possible to enable on their own in demo mode. + inline constexpr flags_ui::FeatureEntry::FeatureVariation + kIPHDemoModeChoiceVariations[] = { ++ BRAVE_FEATURE_ENGAGEMENT_VARIATION_ENTRIES + #if BUILDFLAG(IS_ANDROID) + VARIATION_ENTRY(kIPHAccountSettingsHistorySync), + VARIATION_ENTRY( diff --git a/test/filters/unit_tests.filter b/test/filters/unit_tests.filter index 4d695f31cf7..65e3112d2c7 100644 --- a/test/filters/unit_tests.filter +++ b/test/filters/unit_tests.filter @@ -694,6 +694,12 @@ # These tests fail because we don't show the history sync promo -BrowsingHistoryHandlerHistorySyncPromoTest.ShouldShowHistoryPageHistorySyncPromo*/SignedIn +# These tests fail because we add IPH features but do not add associated UMA +# histogram/actions XML entries (which are not needed for Brave since we don't +# use Google's UMA for IPH metrics). +-BrowserUserEducationServiceTest.CheckFeaturePromoActions +-BrowserUserEducationServiceTest.CheckFeaturePromoHistograms + # Tests below this point have not been diagnosed or had issues created yet. -AboutFlagsHistogramTest.* -AboutFlagsTest.EveryFlagHasMetadata