From dc375072c4fc95d612ccd3c60ff0519eb75b804c Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Wed, 11 Jan 2017 15:48:13 +0000 Subject: [PATCH 1/2] Prepare to enable multiprocess by default. Implement logic to track the version on which the user last manually enabled/disabled multiprocess (by storing the version code in the setting, negated for disabled). This enables us to have the following logic: 1) If multiprocess is enabled by default, re-enable it for users who turned it off on a previous version, but leave it off for users who turned it off on the current version. This lets us collect fresh bug reports after fixing previous issues. 2) If multiprocess is disabled by default, leave it on for users who have enabled it manually, but turn it back off for users who were just relying on the default. This CL leaves it set to "disabled by default" so there is no immediate behavioural change, but this enables us to switch the "enabled by default" flag back and forth freely later. Bug: 21643067 Test: run WebViewUpdateServiceTest Test: manually upgrade/downgrade webview, verify behaviour of setting Change-Id: Ibbecac7e5ee75e9c4a1b6efb84253ace98d1eb1a --- .../com/android/server/webkit/SystemImpl.java | 5 + .../server/webkit/SystemInterface.java | 1 + .../webkit/WebViewUpdateServiceImpl.java | 23 ++++- .../android/server/webkit/TestSystemImpl.java | 9 +- .../webkit/WebViewUpdateServiceTest.java | 95 ++++++++++++++++++- 5 files changed, 129 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/webkit/SystemImpl.java b/services/core/java/com/android/server/webkit/SystemImpl.java index 4fd51b2274dfd..cc87fdae2ad8f 100644 --- a/services/core/java/com/android/server/webkit/SystemImpl.java +++ b/services/core/java/com/android/server/webkit/SystemImpl.java @@ -294,6 +294,11 @@ public class SystemImpl implements SystemInterface { WebViewZygote.setMultiprocessEnabled(enableMultiProcess); } + @Override + public boolean isMultiProcessDefaultEnabled() { + return false; + } + // flags declaring we want extra info from the package manager for webview providers private final static int PACKAGE_FLAGS = PackageManager.GET_META_DATA | PackageManager.GET_SIGNATURES | PackageManager.MATCH_DEBUG_TRIAGED_MISSING diff --git a/services/core/java/com/android/server/webkit/SystemInterface.java b/services/core/java/com/android/server/webkit/SystemInterface.java index b06f829712476..d57edcd4c44c9 100644 --- a/services/core/java/com/android/server/webkit/SystemInterface.java +++ b/services/core/java/com/android/server/webkit/SystemInterface.java @@ -64,4 +64,5 @@ public interface SystemInterface { public int getMultiProcessSetting(Context context); public void setMultiProcessSetting(Context context, int value); public void notifyZygote(boolean enableMultiProcess); + public boolean isMultiProcessDefaultEnabled(); } diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java index f016830a26735..fedd55a9b5b68 100644 --- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java +++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java @@ -240,12 +240,31 @@ public class WebViewUpdateServiceImpl { } boolean isMultiProcessEnabled() { - return mSystemInterface.getMultiProcessSetting(mContext) != 0; + 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; + } else { + // Multiprocess should not be enabled, unless the user has turned it on manually for + // any version. + return settingValue > 0; + } } 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(); - mSystemInterface.setMultiProcessSetting(mContext, enable ? 1 : 0); + int currentVersion = current != null ? current.versionCode : 1; + mSystemInterface.setMultiProcessSetting(mContext, + enable ? currentVersion : -currentVersion); mSystemInterface.notifyZygote(enable); if (current != null) { mSystemInterface.killPackageDependents(current.packageName); diff --git a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java index 67e78d4a2108e..cd3ae1ae8b03a 100644 --- a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java +++ b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java @@ -38,16 +38,18 @@ public class TestSystemImpl implements SystemInterface { private final int mNumRelros; private final boolean mIsDebuggable; private int mMultiProcessSetting; + private final boolean mMultiProcessDefault; public static final int PRIMARY_USER_ID = 0; public TestSystemImpl(WebViewProviderInfo[] packageConfigs, boolean fallbackLogicEnabled, - int numRelros, boolean isDebuggable) { + int numRelros, boolean isDebuggable, boolean multiProcessDefault) { mPackageConfigs = packageConfigs; mFallbackLogicEnabled = fallbackLogicEnabled; mNumRelros = numRelros; mIsDebuggable = isDebuggable; mUsers.add(PRIMARY_USER_ID); + mMultiProcessDefault = multiProcessDefault; } public void addUser(int userId) { @@ -187,4 +189,9 @@ public class TestSystemImpl implements SystemInterface { @Override public void notifyZygote(boolean enableMultiProcess) {} + + @Override + public boolean isMultiProcessDefaultEnabled() { + return mMultiProcessDefault; + } } 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 33cedfa41e517..4c0f0424cf188 100644 --- a/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java @@ -18,6 +18,7 @@ package com.android.server.webkit; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import android.content.Context; import android.content.pm.ApplicationInfo; @@ -84,8 +85,15 @@ public class WebViewUpdateServiceTest { private void setupWithPackages(WebViewProviderInfo[] packages, boolean fallbackLogicEnabled, int numRelros, boolean isDebuggable) { + setupWithPackages(packages, fallbackLogicEnabled, numRelros, isDebuggable, + false /* multiProcessDefault */); + } + + private void setupWithPackages(WebViewProviderInfo[] packages, + boolean fallbackLogicEnabled, int numRelros, boolean isDebuggable, + boolean multiProcessDefault) { TestSystemImpl testing = new TestSystemImpl(packages, fallbackLogicEnabled, numRelros, - isDebuggable); + isDebuggable, multiProcessDefault); mTestSystemImpl = Mockito.spy(testing); mWebViewUpdateServiceImpl = new WebViewUpdateServiceImpl(null /*Context*/, mTestSystemImpl); @@ -1521,4 +1529,89 @@ public class WebViewUpdateServiceTest { assertEquals(firstPackage.versionName, mWebViewUpdateServiceImpl.getCurrentWebViewPackage().versionName); } + + @Test + public void testMultiProcessEnabledByDefault() { + 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 */); + mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */, + true /* valid */, true /* installed */, null /* signatures */, + 10 /* lastUpdateTime*/, false /* not hidden */, 1000 /* versionCode */, + false /* isSystemApp */)); + + runWebViewBootPreparationOnMainSync(); + checkPreparationPhasesForPackage(primaryPackage, 1 /* first preparation phase */); + + // Check it's on by default + assertTrue(mWebViewUpdateServiceImpl.isMultiProcessEnabled()); + + // Test toggling it + mWebViewUpdateServiceImpl.enableMultiProcess(false); + assertFalse(mWebViewUpdateServiceImpl.isMultiProcessEnabled()); + mWebViewUpdateServiceImpl.enableMultiProcess(true); + assertTrue(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 testMultiProcessDisabledByDefault() { + 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 */); + mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, true /* enabled */, + true /* valid */, true /* installed */, null /* signatures */, + 10 /* lastUpdateTime*/, false /* not hidden */, 1000 /* versionCode */, + false /* isSystemApp */)); + + 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()); + + // 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()); + } } From db70ba2953bbb6d5c5d3e2a31b4170c287ab0441 Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Tue, 24 Jan 2017 14:09:52 +0000 Subject: [PATCH 2/2] Enable multiprocess WebView by default. Make the multiprocess WebView enabled by default. It can be disabled by the user in developer settings, but will be re-enabled after the WebView implementation is updated to keep people testing it. Bug: 21643067 Test: verify that WebView is enabled by default in developer settings Change-Id: I100dac6b4d3784678a694352faa6e9fb2fc72078 --- services/core/java/com/android/server/webkit/SystemImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/webkit/SystemImpl.java b/services/core/java/com/android/server/webkit/SystemImpl.java index cc87fdae2ad8f..a0381f774c35d 100644 --- a/services/core/java/com/android/server/webkit/SystemImpl.java +++ b/services/core/java/com/android/server/webkit/SystemImpl.java @@ -296,7 +296,7 @@ public class SystemImpl implements SystemInterface { @Override public boolean isMultiProcessDefaultEnabled() { - return false; + return true; } // flags declaring we want extra info from the package manager for webview providers