Default-enable global desktop site setting for premium tablets - part 1

- Add SharedPreferences keys to track global setting default and user updates
- Default-enable the global setting on app start if it has not been attempted before or if the user has not directly updated the setting
- Add Finch-configurable boolean param to disable default-on on low end devices

Bug: 1343916
Change-Id: I01f0bc18f3c53bd32244409904e12fb294d7dbba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3794846
Commit-Queue: Aishwarya Rajesh <aishwaryarj@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Shu Yang <shuyng@google.com>
Cr-Commit-Position: refs/heads/main@{#1032909}

chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java

@@ -1716,6 +1716,14 @@ public void finishNativeInitialization() {
 
         super.finishNativeInitialization();
 
+        if (RequestDesktopUtils.maybeDefaultEnableGlobalSetting(
+                    getPrimaryDisplaySizeInInches(), Profile.getLastUsedRegularProfile())) {
+            // TODO(crbug.com/1350274): Remove this explicit load when this bug is addressed.
+            if (getActivityTab() != null) {
+                getActivityTab().loadIfNeeded(LoadIfNeededCaller.OTHER);
+            }
+        }
+
         mManualFillingComponentSupplier.get().initialize(getWindowAndroid(),
                 mRootUiCoordinator.getBottomSheetController(),
                 (ChromeKeyboardVisibilityDelegate) getWindowAndroid().getKeyboardDelegate(),

chrome/android/java/src/org/chromium/chrome/browser/tab/RequestDesktopUtils.java

@@ -8,9 +8,14 @@
 import androidx.annotation.IntDef;
 import androidx.annotation.Nullable;
 
+import org.chromium.base.SysUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.components.browser_ui.site_settings.SingleCategorySettings;
 import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge;
 import org.chromium.components.content_settings.ContentSettingValues;
 import org.chromium.components.content_settings.ContentSettingsType;
@@ -26,13 +31,15 @@
  * Utilities for requesting desktop sites support.
  */
 public class RequestDesktopUtils {
-    private static final String PARAM_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES =
-            "default_on_display_size_threshold_inches";
-    private static final double DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES =
-            12.0;
     private static final String ANY_SUBDOMAIN_PATTERN = "[*.]";
     private static final String SITE_WILDCARD = "*";
 
+    static final String PARAM_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES =
+            "default_on_display_size_threshold_inches";
+    static final double DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES = 12.0;
+    static final String PARAM_GLOBAL_SETTING_DEFAULT_ON_ON_LOW_END_DEVICES =
+            "default_on_on_low_end_devices";
+
     // Note: these values must match the UserAgentRequestType enum in enums.xml.
     @IntDef({UserAgentRequestType.REQUEST_DESKTOP, UserAgentRequestType.REQUEST_MOBILE})
     @Retention(RetentionPolicy.SOURCE)
@@ -142,12 +149,43 @@ public static boolean shouldDefaultEnableGlobalSetting(double displaySizeInInche
             return false;
         }
 
-        // TODO(crbug.com/1343916): Also check new SharedPreferences for REQUEST_DESKTOP_SITE to
-        // determine if the setting should be updated. Also nice to have a Finch configurable
-        // boolean to disable default-enabling this setting on low RAM devices.
-        return displaySizeInInches >= ChromeFeatureList.getFieldTrialParamByFeatureAsDouble(
-                       ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS,
-                       PARAM_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES,
-                       DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES);
+        // Check whether default-on for low end devices is disabled.
+        if (!ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
+                    ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS,
+                    PARAM_GLOBAL_SETTING_DEFAULT_ON_ON_LOW_END_DEVICES, true)
+                && SysUtils.isLowEndDevice()) {
+            return false;
+        }
+
+        boolean previouslyDefaultEnabled = SharedPreferencesManager.getInstance().readBoolean(
+                ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING, false);
+        boolean previouslyUpdatedByUser = SharedPreferencesManager.getInstance().contains(
+                SingleCategorySettings.USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY);
+
+        return !previouslyDefaultEnabled && !previouslyUpdatedByUser
+                && displaySizeInInches >= ChromeFeatureList.getFieldTrialParamByFeatureAsDouble(
+                           ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS,
+                           PARAM_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES,
+                           DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES);
+    }
+
+    /**
+     * Default-enables the desktop site global setting if {@code shouldDefaultEnableGlobalSetting}
+     * returns true.
+     * @param displaySizeInInches The device primary display size, in inches.
+     * @param profile The current {@link Profile}.
+     * @return Whether the desktop site global setting was default-enabled.
+     */
+    public static boolean maybeDefaultEnableGlobalSetting(
+            double displaySizeInInches, Profile profile) {
+        if (!shouldDefaultEnableGlobalSetting(displaySizeInInches)) {
+            return false;
+        }
+
+        WebsitePreferenceBridge.setCategoryEnabled(
+                profile, ContentSettingsType.REQUEST_DESKTOP_SITE, true);
+        SharedPreferencesManager.getInstance().writeBoolean(
+                ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING, true);
+        return true;
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/site_settings/SiteSettingsTest.java

@@ -30,6 +30,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.ContextUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.test.util.Batch;
 import org.chromium.base.test.util.CallbackHelper;
@@ -152,6 +153,11 @@ public void tearDown() throws TimeoutException {
         LocationProviderOverrider.setLocationProviderImpl(null);
         NfcSystemLevelSetting.resetNfcForTesting();
         IncognitoUtils.setEnabledForTesting(null);
+        ContextUtils.getAppSharedPreferences()
+                .edit()
+                .remove(SingleCategorySettings
+                                .USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY)
+                .apply();
     }
 
     @AfterClass
@@ -1082,6 +1088,11 @@ public void testOnlyExpectedPreferencesProtectedMedia() {
     public void testOnlyExpectedPreferencesRequestDesktopSite() {
         testExpectedPreferences(
                 SiteSettingsCategory.Type.REQUEST_DESKTOP_SITE, BINARY_TOGGLE, BINARY_TOGGLE);
+        Assert.assertTrue(
+                "SharedPreference USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY should be updated.",
+                ContextUtils.getAppSharedPreferences().contains(
+                        SingleCategorySettings
+                                .USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY));
     }
 
     @Test

chrome/android/junit/src/org/chromium/chrome/browser/tab/RequestDesktopUtilsUnitTest.java

@@ -11,17 +11,30 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
 
+import org.chromium.base.FeatureList;
+import org.chromium.base.FeatureList.TestValues;
+import org.chromium.base.SysUtils;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
+import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
+import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.RequestDesktopUtilsUnitTest.ShadowSysUtils;
+import org.chromium.components.browser_ui.site_settings.SingleCategorySettings;
 import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge;
 import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridgeJni;
 import org.chromium.components.content_settings.ContentSettingValues;
@@ -35,12 +48,13 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Map.Entry;
 
 /**
  * Unit tests for {@link RequestDesktopUtils}.
  */
 @RunWith(BaseRobolectricTestRunner.class)
-@Config(manifest = Config.NONE, shadows = {ShadowGURL.class})
+@Config(manifest = Config.NONE, shadows = {ShadowGURL.class, ShadowSysUtils.class})
 public class RequestDesktopUtilsUnitTest {
     @Rule
     public JniMocker mJniMocker = new JniMocker();
@@ -53,9 +67,28 @@ public class RequestDesktopUtilsUnitTest {
     private BrowserContextHandle mBrowserContextHandleMock;
 
     private @ContentSettingValues int mRdsDefaultValue;
+    private SharedPreferencesManager mSharedPreferencesManager;
+
     private final Map<String, Integer> mContentSettingMap = new HashMap<>();
     private final GURL mGoogleUrl = new GURL(JUnitTestGURLs.GOOGLE_URL);
     private final GURL mMapsUrl = new GURL(JUnitTestGURLs.MAPS_URL);
+
+    private final TestValues mTestValues = new TestValues();
+
+    @Implements(SysUtils.class)
+    static class ShadowSysUtils {
+        private static boolean sLowEndDevice;
+
+        public static void setLowEndDevice(boolean lowEndDevice) {
+            sLowEndDevice = lowEndDevice;
+        }
+
+        @Implementation
+        public static boolean isLowEndDevice() {
+            return sLowEndDevice;
+        }
+    }
+
     private static final String GOOGLE_COM = "[*.]google.com/";
 
     @Before
@@ -68,6 +101,16 @@ public void setup() {
         doAnswer(invocation -> mRdsDefaultValue)
                 .when(mWebsitePreferenceBridgeJniMock)
                 .getDefaultContentSetting(any(), eq(ContentSettingsType.REQUEST_DESKTOP_SITE));
+
+        doAnswer(invocation -> {
+            mRdsDefaultValue = invocation.getArgument(2) ? ContentSettingValues.ALLOW
+                                                         : ContentSettingValues.BLOCK;
+            return null;
+        })
+                .when(mWebsitePreferenceBridgeJniMock)
+                .setContentSettingEnabled(
+                        any(), eq(ContentSettingsType.REQUEST_DESKTOP_SITE), anyBoolean());
+
         doAnswer(invocation -> {
             mContentSettingMap.put(invocation.getArgument(2), invocation.getArgument(4));
             return null;
@@ -78,6 +121,19 @@ public void setup() {
         doAnswer(invocation -> getDomainAndRegistry(invocation.getArgument(0)))
                 .when(mUrlUtilitiesJniMock)
                 .getDomainAndRegistry(anyString(), anyBoolean());
+
+        mSharedPreferencesManager = SharedPreferencesManager.getInstance();
+        mSharedPreferencesManager.disableKeyCheckerForTesting();
+    }
+
+    @After
+    public void tearDown() {
+        FeatureList.setTestValues(null);
+        ShadowSysUtils.setLowEndDevice(false);
+        mSharedPreferencesManager.removeKey(
+                ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING);
+        mSharedPreferencesManager.removeKey(
+                SingleCategorySettings.USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY);
     }
 
     @Test
@@ -166,4 +222,87 @@ public void testSetRequestDesktopSiteContentSettingsForUrl_DefaultAllow_SiteBloc
     private String getDomainAndRegistry(String origin) {
         return origin.replaceAll(".*\\.(.+\\.[^.]+$)", "$1");
     }
+
+    @Test
+    public void testShouldDefaultEnableGlobalSetting_DisableOnLowEndDevice() {
+        Map<String, String> params = new HashMap<>();
+        params.put(RequestDesktopUtils.PARAM_GLOBAL_SETTING_DEFAULT_ON_ON_LOW_END_DEVICES, "false");
+        enableFeatureRequestDesktopSiteDefaults(params);
+        ShadowSysUtils.setLowEndDevice(true);
+        boolean shouldDefaultEnable = RequestDesktopUtils.shouldDefaultEnableGlobalSetting(
+                RequestDesktopUtils
+                        .DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES);
+        Assert.assertFalse(
+                "Desktop site global setting should not be default-enabled on low memory devices.",
+                shouldDefaultEnable);
+    }
+
+    @Test
+    public void testShouldDefaultEnableGlobalSetting_CustomScreenSizeThreshold() {
+        Map<String, String> params = new HashMap<>();
+        params.put(
+                RequestDesktopUtils.PARAM_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES,
+                "10.0");
+        enableFeatureRequestDesktopSiteDefaults(params);
+        boolean shouldDefaultEnable = RequestDesktopUtils.shouldDefaultEnableGlobalSetting(11.0);
+        Assert.assertTrue("Desktop site global setting should be default-enabled on 10\"+ devices.",
+                shouldDefaultEnable);
+    }
+
+    @Test
+    public void testShouldDefaultEnableGlobalSetting_UserPreviouslyUpdatedSetting() {
+        enableFeatureRequestDesktopSiteDefaults(null);
+        // This SharedPreference key will ideally be updated when the user explicitly requests for
+        // an update to the desktop site global setting.
+        mSharedPreferencesManager.writeBoolean(
+                SingleCategorySettings.USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY,
+                true);
+        boolean shouldDefaultEnable = RequestDesktopUtils.shouldDefaultEnableGlobalSetting(
+                RequestDesktopUtils
+                        .DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES);
+        Assert.assertFalse(
+                "Desktop site global setting should not be default-enabled if it has been previously updated by the user.",
+                shouldDefaultEnable);
+    }
+
+    @Test
+    public void testMaybeDefaultEnableGlobalSetting() {
+        enableFeatureRequestDesktopSiteDefaults(null);
+        boolean didDefaultEnable = RequestDesktopUtils.maybeDefaultEnableGlobalSetting(
+                RequestDesktopUtils.DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES,
+                Mockito.mock(Profile.class));
+        Assert.assertTrue(
+                "Desktop site global setting should be default-enabled on big screen devices.",
+                didDefaultEnable);
+        Assert.assertEquals("Desktop site content setting should be set correctly.",
+                ContentSettingValues.ALLOW, mRdsDefaultValue);
+        Assert.assertTrue(
+                "SharedPreference DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING should be true.",
+                mSharedPreferencesManager.contains(
+                        ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING)
+                        && mSharedPreferencesManager.readBoolean(
+                                ChromePreferenceKeys.DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING,
+                                false));
+
+        // Check whether the desktop site global setting should be default-enabled after the first
+        // attempt.
+        boolean shouldDefaultEnable = RequestDesktopUtils.shouldDefaultEnableGlobalSetting(
+                RequestDesktopUtils
+                        .DEFAULT_GLOBAL_SETTING_DEFAULT_ON_DISPLAY_SIZE_THRESHOLD_INCHES);
+        Assert.assertFalse(
+                "Desktop site global setting should not be default-enabled more than once.",
+                shouldDefaultEnable);
+    }
+
+    private void enableFeatureRequestDesktopSiteDefaults(Map<String, String> params) {
+        mTestValues.addFeatureFlagOverride(ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS, true);
+        if (params != null) {
+            for (Entry<String, String> param : params.entrySet()) {
+                mTestValues.addFieldTrialParamOverride(
+                        ChromeFeatureList.REQUEST_DESKTOP_SITE_DEFAULTS, param.getKey(),
+                        param.getValue());
+            }
+        }
+        FeatureList.setTestValues(mTestValues);
+    }
 }

chrome/browser/preferences/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceKeys.java

@@ -237,6 +237,12 @@ public final class ChromePreferenceKeys {
     public static final String DEFAULT_BROWSER_PROMO_PROMOED_BY_SYSTEM_SETTINGS =
             "Chrome.DefaultBrowserPromo.PromoedBySystemSettings";
 
+    /**
+     * Indicates whether the desktop site global setting was enabled by default for a device.
+     */
+    public static final String DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING =
+            "Chrome.RequestDesktopSiteGlobalSetting.DefaultEnabled";
+
     public static final String DOWNLOAD_AUTO_RESUMPTION_ATTEMPT_LEFT = "ResumptionAttemptLeft";
     public static final String DOWNLOAD_FOREGROUND_SERVICE_OBSERVERS = "ForegroundServiceObservers";
     public static final String DOWNLOAD_IS_DOWNLOAD_HOME_ENABLED =
@@ -1012,6 +1018,7 @@ static List<String> getKeysInUse() {
                 DEFAULT_BROWSER_PROMO_PROMOED_BY_SYSTEM_SETTINGS,
                 DEFAULT_BROWSER_PROMO_PROMOED_COUNT,
                 DEFAULT_BROWSER_PROMO_SESSION_COUNT,
+                DEFAULT_ENABLED_DESKTOP_SITE_GLOBAL_SETTING,
                 DOWNLOAD_INTERSTITIAL_DOWNLOAD_PENDING_REMOVAL,
                 EXPLORE_OFFLINE_CONTENT_AVAILABILITY_STATUS,
                 FEED_ARTICLES_LIST_VISIBLE,

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SingleCategorySettings.java

@@ -14,6 +14,7 @@
 
 import android.content.Context;
 import android.content.Intent;
+import android.content.SharedPreferences;
 import android.os.Build;
 import android.os.Bundle;
 import android.provider.Settings;
@@ -39,6 +40,7 @@
 import androidx.recyclerview.widget.RecyclerView;
 import androidx.vectordrawable.graphics.drawable.VectorDrawableCompat;
 
+import org.chromium.base.ContextUtils;
 import org.chromium.base.annotations.UsedByReflection;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.base.metrics.RecordUserAction;
@@ -97,6 +99,13 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
      */
     public static final String EXTRA_SELECTED_DOMAINS = "selected_domains";
 
+    /**
+     * {@link SharedPreferences} key that indicates whether the desktop site global setting was
+     * enabled by the user.
+     */
+    public static final String USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY =
+            "Chrome.RequestDesktopSiteGlobalSetting.UserEnabled";
+
     // The list that contains preferences.
     private RecyclerView mListView;
     // The item for searching the list of items.
@@ -487,6 +496,12 @@ public boolean onPreferenceChange(Preference preference, Object newValue) {
                 } else if (type == SiteSettingsCategory.Type.REQUEST_DESKTOP_SITE) {
                     recordSiteLayoutChanged((boolean) newValue);
                     updateDesktopSiteSecondaryControls();
+                    // TODO(crbug.com/1069897): Use SharedPreferencesManager if it is componentized.
+                    ContextUtils.getAppSharedPreferences()
+                            .edit()
+                            .putBoolean(USER_ENABLED_DESKTOP_SITE_GLOBAL_SETTING_PREFERENCE_KEY,
+                                    (boolean) newValue)
+                            .apply();
                 }
                 break;
             }