Skip to content

Commit

Permalink
[Merge to M111][RDSC] Add Finch params and metric for per-site settin…
Browse files Browse the repository at this point in the history 
…gs IPH

- Add a metric to record an app menu icon click when the IPH is
dismissed.
- Add Finch params to set screen size and RAM thresholds for
the arm 2 IPH.

(cherry picked from commit b9adee3)

Bug: 1402566
Change-Id: Ibf398858c991a903ab4d63e9e91c91f963d130f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198185
Commit-Queue: Aishwarya Rajesh <aishwaryarj@google.com>
Reviewed-by: Sinan Sahin <sinansahin@google.com>
Reviewed-by: Shu Yang <shuyng@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1098242}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4206216
Cr-Commit-Position: refs/branch-heads/5563@{#59}
Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
  • Loading branch information
Aishwarya Rajesh authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent 31132ca commit e76952e
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 63 deletions.
2 changes: 1 addition & 1 deletion ...droid/java/src/org/chromium/chrome/browser/customtabs/BaseCustomTabRootUiCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ void onDeferredStartup() {
mDesktopSiteSettingsIPHController = DesktopSiteSettingsIPHController.create(mActivity,
mWindowAndroid, mActivityTabProvider, Profile.getLastUsedRegularProfile(),
getToolbarManager().getMenuButtonView(),
mAppMenuCoordinator.getAppMenuHandler());
mAppMenuCoordinator.getAppMenuHandler(), getPrimaryDisplaySizeInInches());
}
}
}
63 changes: 49 additions & 14 deletions ...d/java/src/org/chromium/chrome/browser/desktop_site/DesktopSiteSettingsIPHController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import androidx.annotation.VisibleForTesting;

import org.chromium.base.Callback;
import org.chromium.base.SysUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ActivityTabProvider.ActivityTabTabObserver;
Expand All @@ -25,6 +27,7 @@
import org.chromium.chrome.browser.user_education.IPHCommandBuilder;
import org.chromium.chrome.browser.user_education.UserEducationHelper;
import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge;
import org.chromium.components.browser_ui.util.ConversionUtils;
import org.chromium.components.content_settings.ContentSettingsType;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.components.feature_engagement.FeatureConstants;
Expand All @@ -45,6 +48,12 @@ public class DesktopSiteSettingsIPHController {
static final String PARAM_IPH_TYPE_SPECIFIC = "iph_type_specific";
static final String PARAM_SITE_LIST = "site_list";

static final String PARAM_GENERIC_IPH_SCREEN_SIZE_THRESHOLD_INCHES =
"generic_iph_screen_size_threshold_inches";
static final double DEFAULT_GENERIC_IPH_SCREEN_SIZE_THRESHOLD_INCHES = 0.0;
static final String PARAM_GENERIC_IPH_MEMORY_THRESHOLD_MB = "generic_iph_memory_threshold_mb";
static final int DEFAULT_GENERIC_IPH_MEMORY_THRESHOLD_MB = 0;

private final UserEducationHelper mUserEducationHelper;
private final WindowAndroid mWindowAndroid;
private final AppMenuHandler mAppMenuHandler;
Expand All @@ -68,32 +77,33 @@ public class DesktopSiteSettingsIPHController {
* @param profile The current {@link Profile}.
* @param toolbarMenuButton The toolbar menu button to which the IPH will be anchored.
* @param appMenuHandler The app menu handler.
* @param screenSizeInInches The device primary display size in inches.
*/
public static @Nullable DesktopSiteSettingsIPHController create(Activity activity,
WindowAndroid windowAndroid, ActivityTabProvider activityTabProvider, Profile profile,
View toolbarMenuButton, AppMenuHandler appMenuHandler) {
View toolbarMenuButton, AppMenuHandler appMenuHandler, double screenSizeInInches) {
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH)) {
return null;
}

return new DesktopSiteSettingsIPHController(windowAndroid, activityTabProvider, profile,
toolbarMenuButton, appMenuHandler,
new UserEducationHelper(activity, new Handler(Looper.getMainLooper())),
new WebsitePreferenceBridge());
new WebsitePreferenceBridge(), screenSizeInInches);
}

DesktopSiteSettingsIPHController(WindowAndroid windowAndroid,
ActivityTabProvider activityTabProvider, Profile profile, View toolbarMenuButton,
AppMenuHandler appMenuHandler, UserEducationHelper userEducationHelper,
WebsitePreferenceBridge websitePreferenceBridge) {
WebsitePreferenceBridge websitePreferenceBridge, double screenSizeInInches) {
mWindowAndroid = windowAndroid;
mToolbarMenuButton = toolbarMenuButton;
mAppMenuHandler = appMenuHandler;
mUserEducationHelper = userEducationHelper;
mActivityTabProvider = activityTabProvider;
mWebsitePreferenceBridge = websitePreferenceBridge;

maybeRegisterTabObserverForPerSiteIPH(profile);
maybeCreateTabObserverForPerSiteIPH(profile, screenSizeInInches);
}

public void destroy() {
Expand Down Expand Up @@ -127,11 +137,6 @@ void showSpecificIPH(@NonNull Tab tab, Profile profile) {

@VisibleForTesting
void showGenericIPH(@NonNull Tab tab, Profile profile) {
// Return early if the device is not a tablet.
if (!DeviceFormFactor.isWindowOnTablet(mWindowAndroid)) {
return;
}

Tracker tracker = TrackerFactory.getTrackerForProfile(profile);
String featureName = FeatureConstants.REQUEST_DESKTOP_SITE_EXCEPTIONS_GENERIC_FEATURE;
if (perSiteIPHPreChecksFailed(tab, tracker, featureName)) return;
Expand Down Expand Up @@ -182,18 +187,47 @@ boolean perSiteIPHPreChecksFailed(@NonNull Tab tab, Tracker tracker, String feat
return UrlUtilities.isInternalScheme(url) || tab.getWebContents() == null;
}

private boolean genericIPHDevicePreChecksFailed(double screenSizeInInches) {
// Return early if the device is not a tablet.
if (!DeviceFormFactor.isWindowOnTablet(mWindowAndroid)) {
return true;
}

// Return early if the device does not satisfy screen size requirements.
double screenSizeThresholdInInches = ChromeFeatureList.getFieldTrialParamByFeatureAsDouble(
ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH,
PARAM_GENERIC_IPH_SCREEN_SIZE_THRESHOLD_INCHES,
DEFAULT_GENERIC_IPH_SCREEN_SIZE_THRESHOLD_INCHES);
if (screenSizeInInches < screenSizeThresholdInInches) {
return true;
}

// Return early if the device does not satisfy memory requirements.
int memoryThresholdInMb = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH,
PARAM_GENERIC_IPH_MEMORY_THRESHOLD_MB, DEFAULT_GENERIC_IPH_MEMORY_THRESHOLD_MB);
return memoryThresholdInMb != 0
&& SysUtils.amountOfPhysicalMemoryKB()
< memoryThresholdInMb * ConversionUtils.KILOBYTES_PER_MEGABYTE;
}

private void requestShowPerSiteIPH(String featureName, int textId, Object[] textArgs) {
mUserEducationHelper.requestShowIPH(
new IPHCommandBuilder(mToolbarMenuButton.getContext().getResources(), featureName,
textId, textArgs, textId, textArgs)
.setAnchorView(mToolbarMenuButton)
.setOnShowCallback(
() -> turnOnHighlightForMenuItem(R.id.request_desktop_site_id))
.setOnDismissCallback(this::turnOffHighlightForMenuItem)
.setOnDismissCallback(() -> {
turnOffHighlightForMenuItem();
RecordHistogram.recordBooleanHistogram(
"Android.RequestDesktopSite.PerSiteIphDismissed.AppMenuOpened",
mAppMenuHandler.isAppMenuShowing());
})
.build());
}

private void maybeRegisterTabObserverForPerSiteIPH(Profile profile) {
private void maybeCreateTabObserverForPerSiteIPH(Profile profile, double screenSizeInInches) {
boolean showGenericIPH = ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH, PARAM_IPH_TYPE_GENERIC, false);
boolean showSpecificIPH = ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
Expand All @@ -209,20 +243,21 @@ private void maybeRegisterTabObserverForPerSiteIPH(Profile profile) {
.split(","));
createActivityTabTabObserver(tab -> showSpecificIPH(tab, profile));
} else if (showGenericIPH) {
if (genericIPHDevicePreChecksFailed(screenSizeInInches)) return;
createActivityTabTabObserver(tab -> showGenericIPH(tab, profile));
}
}

private void createActivityTabTabObserver(Callback<Tab> callback) {
private void createActivityTabTabObserver(Callback<Tab> showIPHCallback) {
mActivityTabTabObserver = new ActivityTabTabObserver(mActivityTabProvider) {
@Override
protected void onObservingDifferentTab(Tab tab, boolean hint) {
callback.onResult(tab);
showIPHCallback.onResult(tab);
}

@Override
public void onPageLoadFinished(Tab tab, GURL url) {
callback.onResult(tab);
showIPHCallback.onResult(tab);
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ mActivity, new SettingsLauncherImpl(),
}
DesktopSiteSettingsIPHController.create(mActivity, mWindowAndroid, mActivityTabProvider,
Profile.getLastUsedRegularProfile(), getToolbarManager().getMenuButtonView(),
mAppMenuCoordinator.getAppMenuHandler());
mAppMenuCoordinator.getAppMenuHandler(), getPrimaryDisplaySizeInInches());
}
mPromoShownOneshotSupplier.set(didTriggerPromo);

Expand Down
76 changes: 65 additions & 11 deletions ...rc/org/chromium/chrome/browser/desktop_site/DesktopSiteSettingsIPHControllerUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

import org.chromium.base.FeatureList;
import org.chromium.base.FeatureList.TestValues;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.Features.JUnitProcessor;
import org.chromium.base.test.util.JniMocker;
Expand All @@ -43,6 +44,7 @@
import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.RequestDesktopUtilsUnitTest.ShadowSysUtils;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.ui.appmenu.AppMenuHandler;
import org.chromium.chrome.browser.user_education.IPHCommand;
Expand Down Expand Up @@ -70,7 +72,7 @@

/** Unit tests for {@link DesktopSiteSettingsIPHController}. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE, shadows = {ShadowUrlUtilities.class})
@Config(manifest = Config.NONE, shadows = {ShadowUrlUtilities.class, ShadowSysUtils.class})
public class DesktopSiteSettingsIPHControllerUnitTest {
@Rule
public TestRule mFeaturesProcessor = new JUnitProcessor();
Expand Down Expand Up @@ -113,6 +115,7 @@ public class DesktopSiteSettingsIPHControllerUnitTest {
private Map<String, String> mTopDesktopSitesDomainMap;
private boolean mDesktopSiteGloballyEnabled;
private final TestValues mTestValues = new TestValues();
private double mDeviceScreenSizeInInches;

@Before
public void setUp() {
Expand Down Expand Up @@ -163,19 +166,23 @@ public String getDomainAndRegistry(String uri, boolean includePrivateRegistries)
}
});

mDeviceScreenSizeInInches = 10.0;
ShadowSysUtils.setMemoryInMB(2048);

initializeController();
}

@After
public void tearDown() {
TrackerFactory.setTrackerForTests(null);
FeatureList.setTestValues(null);
ShadowSysUtils.setMemoryInMB(0);
ShadowUrlUtilities.reset();
mDesktopSiteGloballyEnabled = false;
}

@Test
public void testRegisterTabObserverForPerSiteIPH_Specific() {
public void testCreateTabObserver_SpecificIPH() {
// Re-instantiate the controller to re-register the ActivityTabTabObserver with applicable
// fieldtrial params set.
mController.destroy();
Expand All @@ -194,7 +201,7 @@ public void testRegisterTabObserverForPerSiteIPH_Specific() {

@Test
@Config(qualifiers = "sw600dp")
public void testRegisterTabObserverForPerSiteIPH_Generic() {
public void testCreateTabObserver_GenericIPH() {
// Re-instantiate the controller to re-register the ActivityTabTabObserver with applicable
// fieldtrial params set.
mController.destroy();
Expand All @@ -209,6 +216,54 @@ public void testRegisterTabObserverForPerSiteIPH_Generic() {
verify(mUserEducationHelper).requestShowIPH(mIPHCommandCaptor.capture());
}

@Test
@Config(qualifiers = "sw320dp")
public void testCreateTabObserver_GenericIPH_NonTabletDevice() {
// Re-instantiate the controller to re-register the ActivityTabTabObserver with applicable
// fieldtrial params set.
mController.destroy();
var params = new HashMap<String, String>();
params.put(DesktopSiteSettingsIPHController.PARAM_IPH_TYPE_GENERIC, "true");
enableFeatureWithParams(ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH, params);
initializeController();
Assert.assertNull(
"ActivityTabTabObserver should be created for the generic IPH when the device is a tablet.",
mController.getActiveTabObserverForTesting());
}

@Test
@Config(qualifiers = "sw600dp")
public void testCreateTabObserver_GenericIPH_InvalidDeviceScreenSize() {
// Re-instantiate the controller to re-register the ActivityTabTabObserver with applicable
// fieldtrial params set.
mController.destroy();
var params = new HashMap<String, String>();
params.put(DesktopSiteSettingsIPHController.PARAM_IPH_TYPE_GENERIC, "true");
params.put(DesktopSiteSettingsIPHController.PARAM_GENERIC_IPH_SCREEN_SIZE_THRESHOLD_INCHES,
"12.0");
enableFeatureWithParams(ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH, params);
initializeController();
Assert.assertNull(
"ActivityTabTabObserver should be created for the generic IPH when the device size is valid.",
mController.getActiveTabObserverForTesting());
}

@Test
@Config(qualifiers = "sw600dp")
public void testCreateTabObserver_GenericIPH_InvalidDeviceMemory() {
// Re-instantiate the controller to re-register the ActivityTabTabObserver with applicable
// fieldtrial params set.
mController.destroy();
var params = new HashMap<String, String>();
params.put(DesktopSiteSettingsIPHController.PARAM_IPH_TYPE_GENERIC, "true");
params.put(DesktopSiteSettingsIPHController.PARAM_GENERIC_IPH_MEMORY_THRESHOLD_MB, "4096");
enableFeatureWithParams(ChromeFeatureList.REQUEST_DESKTOP_SITE_PER_SITE_IPH, params);
initializeController();
Assert.assertNull(
"ActivityTabTabObserver should be created for the generic IPH when the device memory is valid.",
mController.getActiveTabObserverForTesting());
}

@Test
@Config(qualifiers = "sw600dp")
public void testPerSiteIPHPreChecksFailed_TrackerWouldNotTrigger() {
Expand Down Expand Up @@ -329,13 +384,6 @@ public void testSpecificIPH_NotShown_SiteNotEligible() {
verify(mUserEducationHelper, never()).requestShowIPH(mIPHCommandCaptor.capture());
}

@Test
@Config(qualifiers = "sw320dp")
public void testGenericIPH_NotShown_NonTabletDevice() {
mController.showGenericIPH(mTab, mProfile);
verify(mUserEducationHelper, never()).requestShowIPH(mIPHCommandCaptor.capture());
}

@Test
@Config(qualifiers = "sw600dp")
public void testGenericIPH_NotShown_SettingUsed() {
Expand Down Expand Up @@ -369,6 +417,12 @@ private void testShowGenericIPH(boolean switchToDesktop) {

command.onDismissCallback.run();
verify(mAppMenuHandler).clearMenuHighlight();

Assert.assertEquals(
"<Android.RequestDesktopSite.PerSiteIphDismissed.AppMenuOpened> should be recorded when the IPH is dismissed.",
1,
RecordHistogram.getHistogramTotalCountForTesting(
"Android.RequestDesktopSite.PerSiteIphDismissed.AppMenuOpened"));
}

private void enableFeatureWithParams(String featureName, Map<String, String> params) {
Expand All @@ -385,6 +439,6 @@ private void enableFeatureWithParams(String featureName, Map<String, String> par
private void initializeController() {
mController = new DesktopSiteSettingsIPHController(mWindowAndroid, mActivityTabProvider,
mProfile, mToolbarMenuButton, mAppMenuHandler, mUserEducationHelper,
mWebsitePreferenceBridge);
mWebsitePreferenceBridge, mDeviceScreenSizeInInches);
}
}

0 comments on commit e76952e

Please sign in to comment.