Skip to content

Commit

Permalink
[RDSG] Use Feature Engagement to trigger Default-on message
Browse files Browse the repository at this point in the history
Added the following events:
- desktop_site_settings_page_opened
If the user has opened the desktop site settings page.
- request_desktop_site_default_on_iph_trigger
If the message has been shown.
- desktop_site_default_on_primary_action
If the user has accepted the message.
- desktop_site_default_on_gesture
If the user has explicitly dismissed the message.

The initial client side config matches current product behavior.
Will make adjustment via server side config in follow-up CL.

Bug: 1392606
Change-Id: I1fd95a49346e6adbb1af54b80b9ea98b172dac29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4050352
Commit-Queue: Shu Yang <shuyng@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1077574}
  • Loading branch information
Shu Yang authored and Chromium LUCI CQ committed Nov 30, 2022
1 parent 68c4355 commit 38ccbde
Show file tree
Hide file tree
Showing 17 changed files with 100 additions and 53 deletions.
Expand Up @@ -26,6 +26,7 @@
import org.chromium.chrome.browser.settings.ChromeManagedPreferenceDelegate;
import org.chromium.chrome.browser.settings.FaviconLoader;
import org.chromium.chrome.browser.settings.SettingsLauncherImpl;
import org.chromium.chrome.browser.tab.RequestDesktopUtils;
import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
import org.chromium.chrome.browser.webapps.WebappRegistry;
import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
Expand Down Expand Up @@ -262,4 +263,11 @@ public void launchClearBrowsingDataDialog(Activity currentActivity) {
new SettingsLauncherImpl().launchSettingsActivity(
currentActivity, ClearBrowsingDataTabsFragment.class);
}

@Override
// TODO(crbug.com/1393116): Look into a more scalable pattern like
// notifyPageOpened(String className).
public void notifyRequestDesktopSiteSettingsPageOpened() {
RequestDesktopUtils.notifyRequestDesktopSiteSettingsPageOpened();
}
}
Expand Up @@ -35,8 +35,10 @@
import org.chromium.components.browser_ui.util.ConversionUtils;
import org.chromium.components.content_settings.ContentSettingValues;
import org.chromium.components.content_settings.ContentSettingsType;
import org.chromium.components.feature_engagement.EventConstants;
import org.chromium.components.feature_engagement.FeatureConstants;
import org.chromium.components.feature_engagement.Tracker;
import org.chromium.components.messages.DismissReason;
import org.chromium.components.messages.MessageBannerProperties;
import org.chromium.components.messages.MessageDispatcher;
import org.chromium.components.messages.MessageIdentifier;
Expand Down Expand Up @@ -398,12 +400,6 @@ public static boolean maybeDefaultEnableGlobalSetting(
profile, ContentSettingsType.REQUEST_DESKTOP_SITE, true);
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING, true);
// This key will be added only once, since this method will be invoked only once on a
// device. Once the corresponding message is shown, the key will be removed since the
// message will also be shown at most once.
SharedPreferencesManager.getInstance().writeBoolean(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE,
true);
return true;
}

Expand Down Expand Up @@ -433,8 +429,6 @@ public static boolean maybeDisableGlobalSetting(Profile profile) {
// Remove SharedPreferences keys that were added when the feature was supported.
sharedPreferencesManager.removeKey(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING);
sharedPreferencesManager.removeKey(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE);
sharedPreferencesManager.removeKey(
ChromePreferenceKeys.DEFAULT_ENABLE_DESKTOP_SITE_GLOBAL_SETTING_COHORT);

Expand Down Expand Up @@ -463,8 +457,7 @@ public static boolean maybeShowDefaultEnableGlobalSettingMessage(

// Present the message only if the global setting has been default-enabled.
if (!SharedPreferencesManager.getInstance().contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE)) {
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING)) {
return false;
}

Expand All @@ -473,8 +466,6 @@ public static boolean maybeShowDefaultEnableGlobalSettingMessage(
// setting. Present the message only if the setting is enabled.
if (!WebsitePreferenceBridge.isCategoryEnabled(
profile, ContentSettingsType.REQUEST_DESKTOP_SITE)) {
SharedPreferencesManager.getInstance().removeKey(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE);
return false;
}

Expand All @@ -485,6 +476,12 @@ public static boolean maybeShowDefaultEnableGlobalSettingMessage(
return false;
}

Tracker tracker = TrackerFactory.getTrackerForProfile(profile);
if (!tracker.shouldTriggerHelpUI(
FeatureConstants.REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE)) {
return false;
}

Resources resources = context.getResources();
PropertyModel message =
new PropertyModel.Builder(MessageBannerProperties.ALL_KEYS)
Expand All @@ -500,13 +497,23 @@ public static boolean maybeShowDefaultEnableGlobalSettingMessage(
() -> {
SiteSettingsHelper.showCategorySettings(context,
SiteSettingsCategory.Type.REQUEST_DESKTOP_SITE);
tracker.notifyEvent(
EventConstants.DESKTOP_SITE_DEFAULT_ON_PRIMARY_ACTION);
return PrimaryActionClickBehavior.DISMISS_IMMEDIATELY;
})
.with(MessageBannerProperties.ON_DISMISSED,
(dismissReason) -> {
if (dismissReason == DismissReason.GESTURE) {
tracker.notifyEvent(
EventConstants.DESKTOP_SITE_DEFAULT_ON_GESTURE);
}
tracker.dismissed(
FeatureConstants
.REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE);
})
.build();

messageDispatcher.enqueueWindowScopedMessage(message, false);
SharedPreferencesManager.getInstance().removeKey(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE);
return true;
}

Expand Down Expand Up @@ -688,6 +695,14 @@ public void onDismiss(PropertyModel model, int dismissalCause) {
return true;
}

/**
* Record event for feature engagement on desktop site settings page open.
*/
public static void notifyRequestDesktopSiteSettingsPageOpened() {
TrackerFactory.getTrackerForProfile(Profile.getLastUsedRegularProfile())
.notifyEvent(EventConstants.DESKTOP_SITE_SETTINGS_PAGE_OPENED);
}

/**
* Updates the desktop site content setting on user request.
* @param profile The current {@link Profile}.
Expand Down
Expand Up @@ -510,15 +510,6 @@ public void testMaybeDefaultEnableGlobalSetting() {
&& mSharedPreferencesManager.readBoolean(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING,
false));
Assert.assertTrue(
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE should be true.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE)
&& mSharedPreferencesManager.readBoolean(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE,
false));
Assert.assertTrue(
"SharedPreference DEFAULT_ENABLE_DESKTOP_SITE_GLOBAL_SETTING_COHORT should be true.",
mSharedPreferencesManager.contains(
Expand Down Expand Up @@ -553,6 +544,8 @@ public void testMaybeDefaultEnableGlobalSetting_DoNotEnableOnOptInEnabled() {

@Test
public void testMaybeShowDefaultEnableGlobalSettingMessage() {
when(mTracker.shouldTriggerHelpUI(FeatureConstants.REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE))
.thenReturn(true);
enableFeatureWithParams(ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS, null, true);

// Default-enable the global setting before the message is shown.
Expand All @@ -579,21 +572,17 @@ public void testMaybeShowDefaultEnableGlobalSettingMessage() {
message.getValue().get(MessageBannerProperties.PRIMARY_BUTTON_TEXT));
Assert.assertEquals("Message icon resource ID should match.", R.drawable.ic_desktop_windows,
message.getValue().get(MessageBannerProperties.ICON_RESOURCE_ID));
Assert.assertFalse(
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE));
}

@Test
public void testMaybeShowDefaultEnableGlobalSettingMessage_DoNotShowIfSettingIsDisabled() {
when(mTracker.shouldTriggerHelpUI(FeatureConstants.REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE))
.thenReturn(true);
enableFeatureWithParams(ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS, null, true);

// Preference is set when the setting is default-enabled.
mSharedPreferencesManager.writeBoolean(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE,
true);
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING, true);

// Simulate disabling of the setting by the user before the message is shown.
when(mWebsitePreferenceBridgeJniMock.isContentSettingEnabled(
Expand All @@ -604,11 +593,6 @@ public void testMaybeShowDefaultEnableGlobalSettingMessage_DoNotShowIfSettingIsD
mProfile, mMessageDispatcher, mActivity);
Assert.assertFalse(
"Message should not be shown if the content setting is disabled.", shown);
Assert.assertFalse(
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE));
}

@Test
Expand All @@ -633,11 +617,6 @@ public void testMaybeDisableGlobalSetting() {
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING));
Assert.assertFalse(
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE));
Assert.assertFalse(
"SharedPreference DEFAULT_ENABLE_DESKTOP_SITE_GLOBAL_SETTING_COHORT should be removed.",
mSharedPreferencesManager.contains(
Expand Down Expand Up @@ -679,11 +658,6 @@ public void testMaybeDisableGlobalSetting_UserUpdatedSetting() {
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING));
Assert.assertFalse(
"SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE should be removed.",
mSharedPreferencesManager.contains(
ChromePreferenceKeys
.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE));
}

@Test
Expand Down
Expand Up @@ -256,12 +256,6 @@ public final class ChromePreferenceKeys {
*/
public static final String DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING =
"Chrome.RequestDesktopSiteGlobalSetting.DefaultEnabled";
/**
* Indicates whether an opt-out message should be shown after the desktop site global setting
* was enabled by default for a device.
*/
public static final String DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE =
"Chrome.RequestDesktopSiteGlobalSetting.DefaultEnabledShowMessage";
/**
* Indicates whether the device qualifies for default-enabling the desktop site global setting.
*/
Expand Down Expand Up @@ -1079,7 +1073,6 @@ static List<String> getKeysInUse() {
DEFAULT_BROWSER_PROMO_PROMOED_COUNT,
DEFAULT_BROWSER_PROMO_SESSION_COUNT,
DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING,
DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_SHOW_MESSAGE,
DEFAULT_ENABLE_DESKTOP_SITE_GLOBAL_SETTING_COHORT,
DESKTOP_SITE_EXCEPTIONS_DOWNGRADE_GLOBAL_SETTING_ENABLED,
DESKTOP_SITE_EXCEPTIONS_DOWNGRADE_TAB_SETTING_SET,
Expand Down
Expand Up @@ -35,8 +35,9 @@ static List<String> getKeysForTesting() {
"Chrome.OfflineMeasurements.IsRoaming",
"Chrome.OfflineMeasurements.TimeBetweenChecksMillisList",
"Chrome.OfflineMeasurements.UserStateList",
"Chrome.SigninPromo.NTPImpressions",
"Chrome.PriceTracking.PriceDropAlerts",
"Chrome.RequestDesktopSiteGlobalSetting.DefaultEnabledShowMessage",
"Chrome.SigninPromo.NTPImpressions",
"PersistedNotificationId",
"PhysicalWeb.ActivityReferral",
"PhysicalWeb.HasDeferredMetrics",
Expand Down
Expand Up @@ -419,6 +419,7 @@ public void onActivityCreated(Bundle savedInstanceState) {
configureGlobalToggles();
if (mCategory.getType() == SiteSettingsCategory.Type.REQUEST_DESKTOP_SITE) {
RecordUserAction.record("DesktopSiteContentSetting.SettingsPage.Entered");
getSiteSettingsDelegate().notifyRequestDesktopSiteSettingsPageOpened();
}

setHasOptionsMenu(true);
Expand Down
Expand Up @@ -174,6 +174,11 @@ public interface SiteSettingsDelegate {
*/
void launchClearBrowsingDataDialog(Activity currentActivity);

/**
* Called when the desktop site settings page is opened.
*/
void notifyRequestDesktopSiteSettingsPageOpened();

/**
* Called when the view this delegate is assigned to gets destroyed.
*/
Expand Down
Expand Up @@ -299,6 +299,17 @@ public final class EventConstants {
public static final String APP_MENU_DESKTOP_SITE_FOR_TAB_CLICKED =
"app_menu_desktop_site_for_tab_clicked";

/** Desktop site settings page opened. */
public static final String DESKTOP_SITE_SETTINGS_PAGE_OPENED =
"desktop_site_settings_page_opened";

/** Desktop site default-on message primary action event. */
public static final String DESKTOP_SITE_DEFAULT_ON_PRIMARY_ACTION =
"desktop_site_default_on_primary_action";

/** Desktop site default-on message gesture event. */
public static final String DESKTOP_SITE_DEFAULT_ON_GESTURE = "desktop_site_default_on_gesture";

/**
* Do not instantiate.
*/
Expand Down
Expand Up @@ -65,6 +65,7 @@
FeatureConstants.READ_LATER_BOTTOM_SHEET_FEATURE,
FeatureConstants.READ_LATER_CONTEXT_MENU_FEATURE,
FeatureConstants.REQUEST_DESKTOP_SITE_APP_MENU_FEATURE,
FeatureConstants.REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE,
FeatureConstants.IPH_MIC_TOOLBAR_FEATURE, FeatureConstants.IPH_SHARE_SCREENSHOT_FEATURE,
FeatureConstants.IPH_SHARING_HUB_LINK_TOGGLE_FEATURE,
FeatureConstants.IPH_WEB_FEED_FOLLOW_FEATURE,
Expand Down Expand Up @@ -119,6 +120,7 @@
String READ_LATER_APP_MENU_BOOKMARKS_FEATURE = "IPH_ReadLaterAppMenuBookmarks";
String READ_LATER_BOTTOM_SHEET_FEATURE = "IPH_ReadLaterBottomSheet";
String REQUEST_DESKTOP_SITE_APP_MENU_FEATURE = "IPH_RequestDesktopSiteAppMenu";
String REQUEST_DESKTOP_SITE_DEFAULT_ON_FEATURE = "IPH_RequestDesktopSiteDefaultOn";

/**
* An IPH feature indicating to users that there are settings for downloads and they are
Expand Down
21 changes: 21 additions & 0 deletions components/feature_engagement/public/feature_configurations.cc
Expand Up @@ -985,6 +985,27 @@ absl::optional<FeatureConfig> GetClientSideFeatureConfig(
Comparator(EQUAL, 0), 180, 180);
return config;
}

if (kIPHRequestDesktopSiteDefaultOnFeature.name == feature->name) {
// A config that allows the RDS default-on message to be shown:
// * If the message has never been shown before.
// * If the user has never accepted the message.
// * If the user has never explicitly dismissed the message.
absl::optional<FeatureConfig> config = FeatureConfig();
config->valid = true;
config->availability = Comparator(ANY, 0);
config->session_rate = Comparator(ANY, 0);
config->used = EventConfig("desktop_site_settings_page_opened",
Comparator(ANY, 0), 360, 360);
config->trigger = EventConfig("request_desktop_site_default_on_iph_trigger",
Comparator(EQUAL, 0), 360, 360);
config->event_configs.insert(
EventConfig("desktop_site_default_on_primary_action",
Comparator(EQUAL, 0), 360, 360));
config->event_configs.insert(EventConfig("desktop_site_default_on_gesture",
Comparator(EQUAL, 0), 360, 360));
return config;
}
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_APPLE) || BUILDFLAG(IS_LINUX) || \
Expand Down
3 changes: 3 additions & 0 deletions components/feature_engagement/public/feature_constants.cc
Expand Up @@ -195,6 +195,9 @@ BASE_FEATURE(kIPHReadLaterBottomSheetFeature,
BASE_FEATURE(kIPHRequestDesktopSiteAppMenuFeature,
"IPH_RequestDesktopSiteAppMenu",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kIPHRequestDesktopSiteDefaultOnFeature,
"IPH_RequestDesktopSiteDefaultOn",
base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kIPHShoppingListSaveFlowFeature,
"IPH_ShoppingListSaveFlow",
base::FEATURE_ENABLED_BY_DEFAULT);
Expand Down
1 change: 1 addition & 0 deletions components/feature_engagement/public/feature_constants.h
Expand Up @@ -134,6 +134,7 @@ BASE_DECLARE_FEATURE(kIPHReadLaterAppMenuBookmarkThisPageFeature);
BASE_DECLARE_FEATURE(kIPHReadLaterAppMenuBookmarksFeature);
BASE_DECLARE_FEATURE(kIPHReadLaterBottomSheetFeature);
BASE_DECLARE_FEATURE(kIPHRequestDesktopSiteAppMenuFeature);
BASE_DECLARE_FEATURE(kIPHRequestDesktopSiteDefaultOnFeature);
BASE_DECLARE_FEATURE(kIPHShoppingListMenuItemFeature);
BASE_DECLARE_FEATURE(kIPHShoppingListSaveFlowFeature);
BASE_DECLARE_FEATURE(kIPHTabGroupsQuicklyComparePagesFeature);
Expand Down
1 change: 1 addition & 0 deletions components/feature_engagement/public/feature_list.cc
Expand Up @@ -77,6 +77,7 @@ const base::Feature* const kAllFeatures[] = {
&kIPHReadLaterAppMenuBookmarksFeature,
&kIPHReadLaterBottomSheetFeature,
&kIPHRequestDesktopSiteAppMenuFeature,
&kIPHRequestDesktopSiteDefaultOnFeature,
&kIPHShoppingListMenuItemFeature,
&kIPHShoppingListSaveFlowFeature,
&kIPHTabGroupsQuicklyComparePagesFeature,
Expand Down
3 changes: 3 additions & 0 deletions components/feature_engagement/public/feature_list.h
Expand Up @@ -150,6 +150,8 @@ DEFINE_VARIATION_PARAM(kIPHReadLaterBottomSheetFeature,
"IPH_ReadLaterBottomSheet");
DEFINE_VARIATION_PARAM(kIPHRequestDesktopSiteAppMenuFeature,
"IPH_RequestDesktopSiteAppMenu");
DEFINE_VARIATION_PARAM(kIPHRequestDesktopSiteDefaultOnFeature,
"IPH_RequestDesktopSiteDefaultOn");
DEFINE_VARIATION_PARAM(kIPHShoppingListMenuItemFeature,
"IPH_ShoppingListMenuItem");
DEFINE_VARIATION_PARAM(kIPHShoppingListSaveFlowFeature,
Expand Down Expand Up @@ -329,6 +331,7 @@ constexpr flags_ui::FeatureEntry::FeatureVariation
VARIATION_ENTRY(kIPHReadLaterAppMenuBookmarksFeature),
VARIATION_ENTRY(kIPHReadLaterBottomSheetFeature),
VARIATION_ENTRY(kIPHRequestDesktopSiteAppMenuFeature),
VARIATION_ENTRY(kIPHRequestDesktopSiteDefaultOnFeature),
VARIATION_ENTRY(kIPHShoppingListMenuItemFeature),
VARIATION_ENTRY(kIPHShoppingListSaveFlowFeature),
VARIATION_ENTRY(kIPHTabGroupsQuicklyComparePagesFeature),
Expand Down

0 comments on commit 38ccbde

Please sign in to comment.