Skip to content

Commit

Permalink
[CSC] Triggering logic for region search IPH
Browse files Browse the repository at this point in the history
This CL
1. Sets up IPH config for region search IPH. Default config is to show
the very first time.
2. Sends a boolean in the companion url params proto to indicate
whether or not to show the IPH.
2. Notifies IPH system immediately that the bubble was dismissed so
that any subsequent IPH can be shown. This behavior will be modified
as needed further.

(cherry picked from commit 0744b0e)

Bug: b/278267814, 1449021
Change-Id: I80fbfcb33934b5ea2b5e5110a56941baeff8bfa1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4577645
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: Ali Stanfield <stanfield@google.com>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1152699}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4591117
Auto-Submit: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#403}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Shakti Sahu authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent fc40717 commit c261c37
Show file tree
Hide file tree
Showing 18 changed files with 79 additions and 1 deletion.
4 changes: 4 additions & 0 deletions chrome/browser/companion/core/companion_url_builder.cc
Expand Up @@ -128,6 +128,10 @@ std::string CompanionUrlBuilder::BuildCompanionUrlParamProto(GURL page_url) {
promo_state->set_exps_promo_shown_count(
pref_service_->GetInteger(kExpsPromoShownCountPref));

// Set region search IPH state.
promo_state->set_should_show_region_search_iph(
signin_delegate_->ShouldShowRegionSearchIPH());

std::string base64_encoded_proto;
base::Base64Encode(url_params.SerializeAsString(), &base64_encoded_proto);
return base64_encoded_proto;
Expand Down
Expand Up @@ -34,6 +34,7 @@ class MockSigninDelegate : public SigninDelegate {
MOCK_METHOD0(StartSigninFlow, void());
MOCK_METHOD1(EnableMsbb, void(bool));
MOCK_METHOD1(LoadUrlInNewTab, void(const GURL&));
MOCK_METHOD0(ShouldShowRegionSearchIPH, bool());
};

} // namespace
Expand All @@ -54,6 +55,8 @@ class CompanionUrlBuilderTest : public testing::Test {
SetSignInAndMsbbExpectations(/*is_sign_in_allowed=*/true,
/*is_signed_in=*/true,
/*msbb_pref_enabled=*/true);
EXPECT_CALL(signin_delegate_, ShouldShowRegionSearchIPH())
.WillRepeatedly(testing::Return(true));
url_builder_ = std::make_unique<CompanionUrlBuilder>(&pref_service_,
&signin_delegate_);
}
Expand Down Expand Up @@ -212,6 +215,7 @@ TEST_F(CompanionUrlBuilderTest, MsbbOn) {
EXPECT_EQ(0, proto.promo_state().msbb_promo_denial_count());
EXPECT_EQ(0, proto.promo_state().exps_promo_denial_count());
EXPECT_EQ(2, proto.promo_state().exps_promo_shown_count());
EXPECT_TRUE(proto.promo_state().should_show_region_search_iph());
}

TEST_F(CompanionUrlBuilderTest, NonProtobufParams) {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/companion/core/mojom/companion.mojom
Expand Up @@ -59,6 +59,9 @@ enum PromoType {

// Promo to opt into experience api.
kExps = 3,

// IPH about region search capability.
kRegionSearchIPH = 4,
};

// User actions taken on a promo.
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/companion/core/promo_handler.cc
Expand Up @@ -41,6 +41,8 @@ void PromoHandler::OnPromoAction(PromoType promo_type,
case PromoType::kExps:
OnExpsPromo(promo_action, exps_promo_url);
return;
default:
return;
}
}

Expand Down
1 change: 1 addition & 0 deletions chrome/browser/companion/core/promo_handler_unittest.cc
Expand Up @@ -23,6 +23,7 @@ class MockSigninDelegate : public SigninDelegate {
MOCK_METHOD0(StartSigninFlow, void());
MOCK_METHOD1(EnableMsbb, void(bool));
MOCK_METHOD1(LoadUrlInNewTab, void(const GURL&));
MOCK_METHOD0(ShouldShowRegionSearchIPH, bool());
};

} // namespace
Expand Down
Expand Up @@ -12,13 +12,14 @@ package companion.proto;
// Keep this file in sync with
// google3/lens/web/standalone/companion/proto/companion_url_params.proto

// Next ID: 5
// Next ID: 6
message PromoState {
// Number of times the user has denied various promo requests in past.
int32 signin_promo_denial_count = 1;
int32 msbb_promo_denial_count = 2;
int32 exps_promo_denial_count = 3;
int32 exps_promo_shown_count = 4;
bool should_show_region_search_iph = 5;
}

// This proto file is shared between Chrome and Server side and used to pack the
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/companion/core/signin_delegate.h
Expand Up @@ -34,6 +34,9 @@ class SigninDelegate {

// Loads URL in the browser in a new tab.
virtual void LoadUrlInNewTab(const GURL& url) = 0;

// Returns whether region search IPH should be shown.
virtual bool ShouldShowRegionSearchIPH() = 0;
};

} // namespace companion
Expand Down
Expand Up @@ -34,6 +34,7 @@
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/feature_engagement/public/feature_constants.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
Expand Down Expand Up @@ -361,6 +362,8 @@ class CompanionPageBrowserTest : public InProcessBrowserTest {
}

virtual void SetUpFeatureList() {
iph_feature_list_.InitAndEnableFeatures(
{feature_engagement::kIPHCompanionSidePanelRegionSearchFeature});
base::FieldTrialParams params;
params["companion-homepage-url"] =
companion_server_.GetURL("/companion_iframe.html").spec();
Expand Down Expand Up @@ -431,6 +434,7 @@ class CompanionPageBrowserTest : public InProcessBrowserTest {
}

protected:
feature_engagement::test::ScopedIphFeatureList iph_feature_list_;
base::test::ScopedFeatureList feature_list_;
net::EmbeddedTestServer page_url_server_{net::EmbeddedTestServer::TYPE_HTTPS};
net::EmbeddedTestServer companion_server_{
Expand Down
Expand Up @@ -12,6 +12,7 @@
#include "chrome/browser/companion/core/promo_handler.h"
#include "chrome/browser/companion/text_finder/text_finder_manager.h"
#include "chrome/browser/companion/text_finder/text_highlighter_manager.h"
#include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/search.h"
#include "chrome/browser/signin/identity_manager_factory.h"
Expand All @@ -25,6 +26,9 @@
#include "chrome/browser/ui/webui/side_panel/companion/signin_delegate_impl.h"
#include "chrome/browser/unified_consent/unified_consent_service_factory.h"
#include "chrome/common/webui_url_constants.h"
#include "components/feature_engagement/public/event_constants.h"
#include "components/feature_engagement/public/feature_constants.h"
#include "components/feature_engagement/public/tracker.h"
#include "components/lens/buildflags.h"
#include "components/prefs/pref_service.h"
#include "components/unified_consent/pref_names.h"
Expand Down Expand Up @@ -198,6 +202,16 @@ void CompanionPageHandler::OnPromoAction(
side_panel::mojom::PromoType promo_type,
side_panel::mojom::PromoAction promo_action,
const absl::optional<GURL>& exps_promo_url) {
if (promo_type == side_panel::mojom::PromoType::kRegionSearchIPH) {
if (promo_action == side_panel::mojom::PromoAction::kRejected) {
auto* tracker = feature_engagement::TrackerFactory::GetForBrowserContext(
GetProfile());
tracker->Dismissed(
feature_engagement::kIPHCompanionSidePanelRegionSearchFeature);
}
return;
}

promo_handler_->OnPromoAction(promo_type, promo_action, exps_promo_url);
metrics_logger_->OnPromoAction(promo_type, promo_action);
}
Expand All @@ -208,6 +222,8 @@ void CompanionPageHandler::OnRegionSearchClicked() {
helper->StartRegionSearch(web_contents(), /*use_fullscreen_capture=*/false);
metrics_logger_->RecordUiSurfaceClicked(
side_panel::mojom::UiSurface::kRegionSearch, kInvalidPosition);
feature_engagement::TrackerFactory::GetForBrowserContext(GetProfile())
->NotifyEvent("companion_side_panel_region_search_button_clicked");
}

void CompanionPageHandler::OnExpsOptInStatusAvailable(bool is_exps_opted_in) {
Expand Down
Expand Up @@ -5,13 +5,17 @@
#include "chrome/browser/ui/webui/side_panel/companion/signin_delegate_impl.h"

#include "base/functional/callback.h"
#include "chrome/browser/feature_engagement/tracker_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_ui_util.h"
#include "chrome/browser/sync/sync_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/side_panel/companion/companion_side_panel_controller_utils.h"
#include "chrome/browser/unified_consent/unified_consent_service_factory.h"
#include "components/feature_engagement/public/event_constants.h"
#include "components/feature_engagement/public/feature_constants.h"
#include "components/feature_engagement/public/tracker.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/base/consent_level.h"
#include "components/signin/public/base/signin_metrics.h"
Expand Down Expand Up @@ -75,6 +79,13 @@ void SigninDelegateImpl::LoadUrlInNewTab(const GURL& url) {
browser->OpenURL(params);
}

bool SigninDelegateImpl::ShouldShowRegionSearchIPH() {
auto* tracker =
feature_engagement::TrackerFactory::GetForBrowserContext(GetProfile());
return tracker->ShouldTriggerHelpUI(
feature_engagement::kIPHCompanionSidePanelRegionSearchFeature);
}

Profile* SigninDelegateImpl::GetProfile() {
return Profile::FromBrowserContext(webui_contents_->GetBrowserContext());
}
Expand Down
Expand Up @@ -31,6 +31,7 @@ class SigninDelegateImpl : public SigninDelegate {
void StartSigninFlow() override;
void EnableMsbb(bool enable_msbb) override;
void LoadUrlInNewTab(const GURL& url) override;
bool ShouldShowRegionSearchIPH() override;

private:
Profile* GetProfile();
Expand Down
15 changes: 15 additions & 0 deletions components/feature_engagement/public/feature_configurations.cc
Expand Up @@ -265,6 +265,21 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig(
return config;
}

if (kIPHCompanionSidePanelRegionSearchFeature.name == feature->name) {
absl::optional<FeatureConfig> config = FeatureConfig();
config->valid = true;
config->availability = Comparator(ANY, 0);
config->session_rate = Comparator(EQUAL, 0);
// Show the promo up to 1 times a year.
config->trigger =
EventConfig("iph_companion_side_panel_region_search_trigger",
Comparator(LESS_THAN, 1), 360, 360);
config->used =
EventConfig("companion_side_panel_region_search_button_clicked",
Comparator(EQUAL, 0), 360, 360);
return config;
}

if (kIPHDesktopCustomizeChromeFeature.name == feature->name) {
absl::optional<FeatureConfig> config = FeatureConfig();
config->valid = true;
Expand Down
3 changes: 3 additions & 0 deletions components/feature_engagement/public/feature_constants.cc
Expand Up @@ -30,6 +30,9 @@ BASE_FEATURE(kIPHBatterySaverModeFeature,
BASE_FEATURE(kIPHCompanionSidePanelFeature,
"IPH_CompanionSidePanel",
base::FEATURE_DISABLED_BY_DEFAULT);
BASE_FEATURE(kIPHCompanionSidePanelRegionSearchFeature,
"IPH_CompanionSidePanelRegionSearch",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kIPHDesktopSharedHighlightingFeature,
"IPH_DesktopSharedHighlighting",
base::FEATURE_DISABLED_BY_DEFAULT);
Expand Down
1 change: 1 addition & 0 deletions components/feature_engagement/public/feature_constants.h
Expand Up @@ -26,6 +26,7 @@ BASE_DECLARE_FEATURE(kIPHDummyFeature);
BASE_DECLARE_FEATURE(kIPHAutofillFeedbackNewBadgeFeature);
BASE_DECLARE_FEATURE(kIPHBatterySaverModeFeature);
BASE_DECLARE_FEATURE(kIPHCompanionSidePanelFeature);
BASE_DECLARE_FEATURE(kIPHCompanionSidePanelRegionSearchFeature);
BASE_DECLARE_FEATURE(kIPHDesktopSharedHighlightingFeature);
BASE_DECLARE_FEATURE(kIPHDesktopTabGroupsNewGroupFeature);
BASE_DECLARE_FEATURE(kIPHDesktopCustomizeChromeFeature);
Expand Down
1 change: 1 addition & 0 deletions components/feature_engagement/public/feature_list.cc
Expand Up @@ -142,6 +142,7 @@ const base::Feature* const kAllFeatures[] = {
&kIPHAutofillFeedbackNewBadgeFeature,
&kIPHBatterySaverModeFeature,
&kIPHCompanionSidePanelFeature,
&kIPHCompanionSidePanelRegionSearchFeature,
&kIPHDesktopTabGroupsNewGroupFeature,
&kIPHDesktopCustomizeChromeFeature,
&kIPHDownloadToolbarButtonFeature,
Expand Down
3 changes: 3 additions & 0 deletions components/feature_engagement/public/feature_list.h
Expand Up @@ -260,6 +260,8 @@ DEFINE_VARIATION_PARAM(kIPHAutofillFeedbackNewBadgeFeature,
"IPH_AutofillFeedbackNewBadge");
DEFINE_VARIATION_PARAM(kIPHBatterySaverModeFeature, "IPH_BatterySaverMode");
DEFINE_VARIATION_PARAM(kIPHCompanionSidePanelFeature, "IPH_CompanionSidePanel");
DEFINE_VARIATION_PARAM(kIPHCompanionSidePanelRegionSearchFeature,
"IPH_CompanionSidePanelRegionSearch");
DEFINE_VARIATION_PARAM(kIPHDesktopCustomizeChromeFeature,
"IPH_DesktopCustomizeChrome");
DEFINE_VARIATION_PARAM(kIPHDesktopTabGroupsNewGroupFeature,
Expand Down Expand Up @@ -455,6 +457,7 @@ constexpr flags_ui::FeatureEntry::FeatureVariation
VARIATION_ENTRY(kIPHAutofillFeedbackNewBadgeFeature),
VARIATION_ENTRY(kIPHBatterySaverModeFeature),
VARIATION_ENTRY(kIPHCompanionSidePanelFeature),
VARIATION_ENTRY(kIPHCompanionSidePanelRegionSearchFeature),
VARIATION_ENTRY(kIPHDesktopCustomizeChromeFeature),
VARIATION_ENTRY(kIPHDesktopTabGroupsNewGroupFeature),
VARIATION_ENTRY(kIPHDownloadToolbarButtonFeature),
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/actions/actions.xml
Expand Up @@ -37679,6 +37679,8 @@ should be able to be added at any place in this file.
<suffix name="ChromeReengagementNotification3"
label="For ChromeReengagementNotification3 feature."/>
<suffix name="CompanionSidePanel" label="For companion side panel feature."/>
<suffix name="CompanionSidePanelRegionSearch"
label="For companion side panel region search feature."/>
<suffix name="ContextualPageActions_PriceTracking"
label="For Contextual page actions price tracking feature."/>
<suffix name="ContextualPageActions_PriceTrackingActionChip"
Expand Down
Expand Up @@ -85,6 +85,9 @@ chromium-metrics-reviews@google.com.
<variant name="IPH_CompanionSidePanel"
summary="prompting users to view the companion feature in the side
panel"/>
<variant name="IPH_CompanionSidePanelRegionSearch"
summary="prompting users to use the region search feature in the
companion side panel"/>
<variant name="IPH_ContextualPageActions_PriceTracking"
summary="contextual page price tracking action in the top toolbar"/>
<variant name="IPH_ContextualPageActions_PriceTrackingActionChip"
Expand Down

0 comments on commit c261c37

Please sign in to comment.