Add shell command for turning on/off WebView multiprocess.

Now that we are removing the WebView Multiprocess Developer Setting we
instead add a couple of shell commands for debugging purposes:
adb shell cmd webviewupdate enable-multiprocess
adb shell cmd webviewupdate disable-multiprocess

We also ensure that multiprocess for WebView is enabled after this
change - even if the user disabled it earlier.

Bug: 36513859
Test: Run WebViewUpdateServiceTest tests.
Test: Ensure disabling/enabling multiprocess through the new adb
commands causes WebViewZygote to be used accordingly, and kills apps
using WebView.
Test: Disable multiprocess on a previous OC-build, then flash a build
with this change in it, and ensure multiprocess is turned on
(multiprocess is currently on by default for all devices).
Change-Id: I923198781777e33848b671ec44f50bbc65b43e24

diff --git a/packages/SettingsLib/res/values/strings.xml b/packages/SettingsLib/res/values/strings.xml
index 82ffa68..e15df14 100644 (file)
--- a/packages/SettingsLib/res/values/strings.xml
+++ b/packages/SettingsLib/res/values/strings.xml
@@ -733,11 +733,6 @@
     <!-- Services settings screen, setting option summary for the user to go to the screen to view running services  -->
     <string name="runningservices_settings_summary">View and control currently running services</string>
 
-    <!-- Developer settings: enable WebView multiprocess name [CHAR LIMIT=50] -->
-    <string name="enable_webview_multiprocess">Multiprocess WebView</string>
-    <!-- Developer settings: enable WebView multiprocess summary [CHAR LIMIT=60] -->
-    <string name="enable_webview_multiprocess_desc">Run WebView renderers separately</string>
-
     <!-- Developer settings: select WebView provider title [CHAR LIMIT=30] -->
     <string name="select_webview_provider_title">WebView implementation</string>
     <!-- Developer settings: select WebView provider dialog title [CHAR LIMIT=30] -->

diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
index 114c362..b1acbec 100644 (file)
--- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
+++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
@@ -26,6 +26,7 @@ import android.webkit.WebViewProviderInfo;
 import android.webkit.WebViewProviderResponse;
 
 import java.io.PrintWriter;
+import java.lang.Integer;
 import java.util.List;
 
 /**
@@ -72,6 +73,9 @@ public class WebViewUpdateServiceImpl {
     private WebViewUpdater mWebViewUpdater;
     final private Context mContext;
 
+    private final static int MULTIPROCESS_SETTING_ON_VALUE = Integer.MAX_VALUE;
+    private final static int MULTIPROCESS_SETTING_OFF_VALUE = Integer.MIN_VALUE;
+
     public WebViewUpdateServiceImpl(Context context, SystemInterface systemInterface) {
         mContext = context;
         mSystemInterface = systemInterface;
@@ -234,31 +238,20 @@ public class WebViewUpdateServiceImpl {
     }
 
     boolean isMultiProcessEnabled() {
-        PackageInfo current = getCurrentWebViewPackage();
-        if (current == null) return false;
-        int currentVersion = current.versionCode;
         int settingValue = mSystemInterface.getMultiProcessSetting(mContext);
         if (mSystemInterface.isMultiProcessDefaultEnabled()) {
-            // Multiprocess should be enabled unless the user has turned it off manually for this
-            // version or newer, as we want to re-enable it when it's updated to get fresh
-            // bug reports.
-            return settingValue > -currentVersion;
+            // Multiprocess should be enabled unless the user has turned it off manually.
+            return settingValue > MULTIPROCESS_SETTING_OFF_VALUE;
         } else {
-            // Multiprocess should not be enabled, unless the user has turned it on manually for
-            // any version.
-            return settingValue > 0;
+            // Multiprocess should not be enabled, unless the user has turned it on manually.
+            return settingValue >= MULTIPROCESS_SETTING_ON_VALUE;
         }
     }
 
     void enableMultiProcess(boolean enable) {
-        // The value we store for the setting is the version code of the current package, if it's
-        // enabled, or the negation of the version code of the current package, if it's disabled.
-        // Users who have a setting from before this scheme was implemented will have it set to 0 or
-        // 1 instead.
         PackageInfo current = getCurrentWebViewPackage();
-        int currentVersion = current != null ? current.versionCode : 1;
         mSystemInterface.setMultiProcessSetting(mContext,
-                                                enable ? currentVersion : -currentVersion);
+                enable ? MULTIPROCESS_SETTING_ON_VALUE : MULTIPROCESS_SETTING_OFF_VALUE);
         mSystemInterface.notifyZygote(enable);
         if (current != null) {
             mSystemInterface.killPackageDependents(current.packageName);

diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceShellCommand.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceShellCommand.java
index 68448f3..39e28c7 100644 (file)
--- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceShellCommand.java
+++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceShellCommand.java
@@ -43,6 +43,10 @@ class WebViewUpdateServiceShellCommand extends ShellCommand {
                     return enableFallbackLogic(true);
                 case "set-webview-implementation":
                     return setWebViewImplementation();
+                case "enable-multiprocess":
+                    return enableMultiProcess(true);
+                case "disable-multiprocess":
+                    return enableMultiProcess(false);
                 default:
                     return handleDefaultCommands(cmd);
             }
@@ -74,6 +78,13 @@ class WebViewUpdateServiceShellCommand extends ShellCommand {
         }
     }
 
+    private int enableMultiProcess(boolean enable) throws RemoteException {
+        final PrintWriter pw = getOutPrintWriter();
+        mInterface.enableMultiProcess(enable);
+        pw.println("Success");
+        return 0;
+    }
+
     @Override
     public void onHelp() {
         PrintWriter pw = getOutPrintWriter();
@@ -90,6 +101,10 @@ class WebViewUpdateServiceShellCommand extends ShellCommand {
         pw.println("    package is available.");
         pw.println("  set-webview-implementation PACKAGE");
         pw.println("    Set the WebView implementation to the specified package.");
+        pw.println("  enable-multiprocess");
+        pw.println("    Enable multi-process mode for WebView");
+        pw.println("  disable-multiprocess");
+        pw.println("    Disable multi-process mode for WebView");
         pw.println();
     }
 }

diff --git a/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java b/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java
index 9c8007a..05c4853 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java
@@ -42,6 +42,7 @@ import org.mockito.Mockito;
 import org.mockito.Matchers;
 import org.mockito.compat.ArgumentMatcher;
 
+import java.lang.Integer;
 import java.util.concurrent.CountDownLatch;
 
 
@@ -1530,12 +1531,22 @@ public class WebViewUpdateServiceTest {
 
     @Test
     public void testMultiProcessEnabledByDefault() {
+        testMultiProcessByDefault(true /* enabledByDefault */);
+    }
+
+    @Test
+    public void testMultiProcessDisabledByDefault() {
+        testMultiProcessByDefault(false /* enabledByDefault */);
+    }
+
+    private void testMultiProcessByDefault(boolean enabledByDefault) {
         String primaryPackage = "primary";
         WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
             new WebViewProviderInfo(
                     primaryPackage, "", true /* default available */, false /* fallback */, null)};
         setupWithPackages(packages, true /* fallback logic enabled */, 1 /* numRelros */,
-                          true /* debuggable */, true /* multiprocess by default */);
+                          true /* debuggable */,
+                          enabledByDefault /* not multiprocess by default */);
         mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */,
                     true /* valid */, true /* installed */, null /* signatures */,
                     10 /* lastUpdateTime*/, false /* not hidden */, 1000 /* versionCode */,
@@ -1544,35 +1555,54 @@ public class WebViewUpdateServiceTest {
         runWebViewBootPreparationOnMainSync();
         checkPreparationPhasesForPackage(primaryPackage, 1 /* first preparation phase */);
 
-        // Check it's on by default
-        assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+        // Check it's off by default
+        assertEquals(enabledByDefault, mWebViewUpdateServiceImpl.isMultiProcessEnabled());
 
         // Test toggling it
-        mWebViewUpdateServiceImpl.enableMultiProcess(false);
-        assertFalse(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
-        mWebViewUpdateServiceImpl.enableMultiProcess(true);
-        assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+        mWebViewUpdateServiceImpl.enableMultiProcess(!enabledByDefault);
+        assertEquals(!enabledByDefault, mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+        mWebViewUpdateServiceImpl.enableMultiProcess(enabledByDefault);
+        assertEquals(enabledByDefault, mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+    }
 
-        // Disable, then upgrade provider, which should re-enable it
-        mWebViewUpdateServiceImpl.enableMultiProcess(false);
-        mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */,
-                    true /* valid */, true /* installed */, null /* signatures */,
-                    20 /* lastUpdateTime*/, false /* not hidden */, 2000 /* versionCode */,
-                    false /* isSystemApp */));
-        mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
-                WebViewUpdateService.PACKAGE_ADDED_REPLACED, 0);
-        checkPreparationPhasesForPackage(primaryPackage, 2);
-        assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+    @Test
+    public void testMultiProcessEnabledByDefaultWithSettingsValue() {
+        testMultiProcessByDefaultWithSettingsValue(
+                true /* enabledByDefault */, Integer.MIN_VALUE, false /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                true /* enabledByDefault */, -999999, true /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                true /* enabledByDefault */, 0, true /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                true /* enabledByDefault */, 999999, true /* expectEnabled */);
     }
 
     @Test
-    public void testMultiProcessDisabledByDefault() {
+    public void testMultiProcessDisabledByDefaultWithSettingsValue() {
+        testMultiProcessByDefaultWithSettingsValue(
+                false /* enabledByDefault */, Integer.MIN_VALUE, false /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                false /* enabledByDefault */, 0, false /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                false /* enabledByDefault */, 999999, false /* expectEnabled */);
+        testMultiProcessByDefaultWithSettingsValue(
+                false /* enabledByDefault */, Integer.MAX_VALUE, true /* expectEnabled */);
+    }
+
+    /**
+     * Test the logic of the multiprocess setting depending on whether multiprocess is enabled by
+     * default, and what the setting is set to.
+     * @param enabledByDefault whether multiprocess is enabled by default.
+     * @param settingValue value of the multiprocess setting.
+     */
+    private void testMultiProcessByDefaultWithSettingsValue(
+            boolean enabledByDefault, int settingValue, boolean expectEnabled) {
         String primaryPackage = "primary";
         WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
             new WebViewProviderInfo(
                     primaryPackage, "", true /* default available */, false /* fallback */, null)};
         setupWithPackages(packages, true /* fallback logic enabled */, 1 /* numRelros */,
-                          true /* debuggable */, false /* not multiprocess by default */);
+                          true /* debuggable */, enabledByDefault /* multiprocess by default */);
         mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */,
                     true /* valid */, true /* installed */, null /* signatures */,
                     10 /* lastUpdateTime*/, false /* not hidden */, 1000 /* versionCode */,
@@ -1581,38 +1611,12 @@ public class WebViewUpdateServiceTest {
         runWebViewBootPreparationOnMainSync();
         checkPreparationPhasesForPackage(primaryPackage, 1 /* first preparation phase */);
 
-        // Check it's off by default
-        assertFalse(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
-
-        // Test toggling it
-        mWebViewUpdateServiceImpl.enableMultiProcess(true);
-        assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
-        mWebViewUpdateServiceImpl.enableMultiProcess(false);
-        assertFalse(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
-
-        // Disable, then upgrade provider, which should not re-enable it
-        mWebViewUpdateServiceImpl.enableMultiProcess(false);
-        mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */,
-                    true /* valid */, true /* installed */, null /* signatures */,
-                    20 /* lastUpdateTime*/, false /* not hidden */, 2000 /* versionCode */,
-                    false /* isSystemApp */));
-        mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
-                WebViewUpdateService.PACKAGE_ADDED_REPLACED, 0);
-        checkPreparationPhasesForPackage(primaryPackage, 2);
-        assertFalse(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+        mTestSystemImpl.setMultiProcessSetting(null /* context */, settingValue);
 
-        // Enable, then upgrade provider, which should leave it on
-        mWebViewUpdateServiceImpl.enableMultiProcess(true);
-        mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */,
-                    true /* valid */, true /* installed */, null /* signatures */,
-                    30 /* lastUpdateTime*/, false /* not hidden */, 3000 /* versionCode */,
-                    false /* isSystemApp */));
-        mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
-                WebViewUpdateService.PACKAGE_ADDED_REPLACED, 0);
-        checkPreparationPhasesForPackage(primaryPackage, 3);
-        assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled());
+        assertEquals(expectEnabled, mWebViewUpdateServiceImpl.isMultiProcessEnabled());
     }
 
+
     /**
      * Ensure that packages with a targetSdkVersion targeting the current platform are valid, and
      * that packages targeting an older version are not valid.