From 725c7b9da4666e596052169fcf9b0b6c82d73b79 Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Wed, 18 Mar 2020 17:33:39 -0400 Subject: [PATCH] Remove WebView fallback logic migration code. The WebView fallback mechanism was removed in Q and replaced with code to migrate the state of upgraded devices to the new configuration. This is no longer necessary; remove the logic to simplify the code. The notion of a WebView provider being designated as the fallback is kept, as this is still used by the code which checks if there is no valid provider at all at boot time and tries to fix the situation by re-enabling the fallback provider for all users - this was added for b/132815849 and should suffice for any devices which haven't executed the fallback migration code. Bug: 129470358 Test: atest WebViewUpdateServiceTest Change-Id: I4de777b650fb793903f6df74a7ff639190d8b0a9 --- core/java/android/provider/Settings.java | 8 -- .../settings/SettingsProtoDumpUtil.java | 3 - .../android/provider/SettingsBackupTest.java | 1 - .../com/android/server/webkit/SystemImpl.java | 13 ---- .../server/webkit/SystemInterface.java | 3 - .../webkit/WebViewUpdateServiceImpl.java | 27 +------ .../android/server/webkit/TestSystemImpl.java | 16 +--- .../webkit/WebViewUpdateServiceTest.java | 76 ++++++------------- 8 files changed, 28 insertions(+), 119 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 8ad76692db042..88cfd143593b2 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -10442,14 +10442,6 @@ public final class Settings { public static final String WEBVIEW_DATA_REDUCTION_PROXY_KEY = "webview_data_reduction_proxy_key"; - /** - * Whether or not the WebView fallback mechanism should be enabled. - * 0=disabled, 1=enabled. - * @hide - */ - public static final String WEBVIEW_FALLBACK_LOGIC_ENABLED = - "webview_fallback_logic_enabled"; - /** * Name of the package used as WebView provider (if unset the provider is instead determined * by the system). diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java index f1fb527245a13..c60052a628f0a 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java @@ -1493,9 +1493,6 @@ class SettingsProtoDumpUtil { dumpSetting(s, p, Settings.Global.WEBVIEW_DATA_REDUCTION_PROXY_KEY, GlobalSettingsProto.Webview.DATA_REDUCTION_PROXY_KEY); - dumpSetting(s, p, - Settings.Global.WEBVIEW_FALLBACK_LOGIC_ENABLED, - GlobalSettingsProto.Webview.FALLBACK_LOGIC_ENABLED); dumpSetting(s, p, Settings.Global.WEBVIEW_PROVIDER, GlobalSettingsProto.Webview.PROVIDER); diff --git a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java index 69be14413583e..d3ed7addd279a 100644 --- a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java +++ b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java @@ -524,7 +524,6 @@ public class SettingsBackupTest { Settings.Global.NETWORK_ACCESS_TIMEOUT_MS, Settings.Global.WARNING_TEMPERATURE, Settings.Global.WEBVIEW_DATA_REDUCTION_PROXY_KEY, - Settings.Global.WEBVIEW_FALLBACK_LOGIC_ENABLED, Settings.Global.WEBVIEW_MULTIPROCESS, Settings.Global.WEBVIEW_PROVIDER, Settings.Global.WFC_IMS_ENABLED, diff --git a/services/core/java/com/android/server/webkit/SystemImpl.java b/services/core/java/com/android/server/webkit/SystemImpl.java index 201894049b061..68f554cb2758b 100644 --- a/services/core/java/com/android/server/webkit/SystemImpl.java +++ b/services/core/java/com/android/server/webkit/SystemImpl.java @@ -196,19 +196,6 @@ public class SystemImpl implements SystemInterface { } } - @Override - public boolean isFallbackLogicEnabled() { - // Note that this is enabled by default (i.e. if the setting hasn't been set). - return Settings.Global.getInt(AppGlobals.getInitialApplication().getContentResolver(), - Settings.Global.WEBVIEW_FALLBACK_LOGIC_ENABLED, 1) == 1; - } - - @Override - public void enableFallbackLogic(boolean enable) { - Settings.Global.putInt(AppGlobals.getInitialApplication().getContentResolver(), - Settings.Global.WEBVIEW_FALLBACK_LOGIC_ENABLED, enable ? 1 : 0); - } - @Override public void enablePackageForAllUsers(Context context, String packageName, boolean enable) { UserManager userManager = (UserManager)context.getSystemService(Context.USER_SERVICE); diff --git a/services/core/java/com/android/server/webkit/SystemInterface.java b/services/core/java/com/android/server/webkit/SystemInterface.java index 743740d277ba2..09c23a7229ad2 100644 --- a/services/core/java/com/android/server/webkit/SystemInterface.java +++ b/services/core/java/com/android/server/webkit/SystemInterface.java @@ -41,9 +41,6 @@ public interface SystemInterface { public void updateUserSetting(Context context, String newProviderName); public void killPackageDependents(String packageName); - public boolean isFallbackLogicEnabled(); - public void enableFallbackLogic(boolean enable); - public void enablePackageForAllUsers(Context context, String packageName, boolean enable); public boolean systemIsDebuggable(); diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java index 11fd7953e08f5..55697d39fb99d 100644 --- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java +++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java @@ -43,8 +43,7 @@ import java.io.PrintWriter; * as the WebView preparation class. * 2. The SystemServer calls WebViewUpdateService.prepareWebViewInSystemServer. This happens at boot * and the WebViewUpdateService should not have been accessed before this call. In this call we - * migrate away from the old fallback logic if necessary and then choose WebView implementation for - * the first time. + * choose WebView implementation for the first time. * 3. The update service listens for Intents related to package installs and removals. These intents * are received and processed on the UI thread. Each intent can result in changing WebView * implementation. @@ -80,7 +79,6 @@ public class WebViewUpdateServiceImpl { } void prepareWebViewInSystemServer() { - migrateFallbackStateOnBoot(); mWebViewUpdater.prepareWebViewInSystemServer(); if (getCurrentWebViewPackage() == null) { // We didn't find a valid WebView implementation. Try explicitly re-enabling the @@ -158,27 +156,6 @@ public class WebViewUpdateServiceImpl { return mWebViewUpdater.getCurrentWebViewPackage(); } - /** - * If the fallback logic is enabled, re-enable any fallback package for all users, then - * disable the fallback logic. - * - * This migrates away from the old fallback mechanism to the new state where packages are never - * automatically enableenableisabled. - */ - private void migrateFallbackStateOnBoot() { - if (!mSystemInterface.isFallbackLogicEnabled()) return; - - WebViewProviderInfo[] webviewProviders = mSystemInterface.getWebViewPackages(); - WebViewProviderInfo fallbackProvider = getFallbackProvider(webviewProviders); - if (fallbackProvider != null) { - Slog.i(TAG, "One-time migration: enabling " + fallbackProvider.packageName); - mSystemInterface.enablePackageForAllUsers(mContext, fallbackProvider.packageName, true); - } else { - Slog.i(TAG, "Skipping one-time migration: no fallback provider"); - } - mSystemInterface.enableFallbackLogic(false); - } - /** * Returns the only fallback provider in the set of given packages, or null if there is none. */ @@ -217,8 +194,6 @@ public class WebViewUpdateServiceImpl { */ void dumpState(PrintWriter pw) { pw.println("Current WebView Update Service state"); - pw.println(String.format(" Fallback logic enabled: %b", - mSystemInterface.isFallbackLogicEnabled())); pw.println(String.format(" Multiprocess enabled: %b", isMultiProcessEnabled())); mWebViewUpdater.dumpState(pw); } 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 8cba69f6ddcbb..3530e38ef67c5 100644 --- a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java +++ b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java @@ -34,7 +34,6 @@ public class TestSystemImpl implements SystemInterface { List mUsers = new ArrayList<>(); // Package -> [user, package] Map> mPackages = new HashMap(); - private boolean mFallbackLogicEnabled; private final int mNumRelros; private final boolean mIsDebuggable; private int mMultiProcessSetting; @@ -42,10 +41,9 @@ public class TestSystemImpl implements SystemInterface { public static final int PRIMARY_USER_ID = 0; - public TestSystemImpl(WebViewProviderInfo[] packageConfigs, boolean fallbackLogicEnabled, - int numRelros, boolean isDebuggable, boolean multiProcessDefault) { + public TestSystemImpl(WebViewProviderInfo[] packageConfigs, int numRelros, boolean isDebuggable, + boolean multiProcessDefault) { mPackageConfigs = packageConfigs; - mFallbackLogicEnabled = fallbackLogicEnabled; mNumRelros = numRelros; mIsDebuggable = isDebuggable; mUsers.add(PRIMARY_USER_ID); @@ -77,16 +75,6 @@ public class TestSystemImpl implements SystemInterface { @Override public void killPackageDependents(String packageName) {} - @Override - public boolean isFallbackLogicEnabled() { - return mFallbackLogicEnabled; - } - - @Override - public void enableFallbackLogic(boolean enable) { - mFallbackLogicEnabled = enable; - } - @Override public void enablePackageForAllUsers(Context context, String packageName, boolean enable) { for(int userId : mUsers) { 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 bbfc5ab42d61c..ebe45a6fa1e8a 100644 --- a/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java @@ -67,36 +67,30 @@ public class WebViewUpdateServiceTest { } private void setupWithPackages(WebViewProviderInfo[] packages) { - setupWithAllParameters(packages, false /* fallbackLogicEnabled */, 1 /* numRelros */, - true /* isDebuggable */, false /* multiProcessDefault */); - } - - private void setupWithPackagesAndFallbackLogic(WebViewProviderInfo[] packages) { - setupWithAllParameters(packages, true /* fallbackLogicEnabled */, 1 /* numRelros */, - true /* isDebuggable */, false /* multiProcessDefault */); + setupWithAllParameters(packages, 1 /* numRelros */, true /* isDebuggable */, + false /* multiProcessDefault */); } private void setupWithPackagesAndRelroCount(WebViewProviderInfo[] packages, int numRelros) { - setupWithAllParameters(packages, false /* fallbackLogicEnabled */, numRelros, - true /* isDebuggable */, false /* multiProcessDefault */); + setupWithAllParameters(packages, numRelros, true /* isDebuggable */, + false /* multiProcessDefault */); } private void setupWithPackagesNonDebuggable(WebViewProviderInfo[] packages) { - setupWithAllParameters(packages, false /* fallbackLogicEnabled */, 1 /* numRelros */, - false /* isDebuggable */, false /* multiProcessDefault */); + setupWithAllParameters(packages, 1 /* numRelros */, false /* isDebuggable */, + false /* multiProcessDefault */); } private void setupWithPackagesAndMultiProcess(WebViewProviderInfo[] packages, boolean multiProcessDefault) { - setupWithAllParameters(packages, false /* fallbackLogicEnabled */, 1 /* numRelros */, - true /* isDebuggable */, multiProcessDefault); + setupWithAllParameters(packages, 1 /* numRelros */, true /* isDebuggable */, + multiProcessDefault); } - private void setupWithAllParameters(WebViewProviderInfo[] packages, - boolean fallbackLogicEnabled, int numRelros, boolean isDebuggable, - boolean multiProcessDefault) { - TestSystemImpl testing = new TestSystemImpl(packages, fallbackLogicEnabled, numRelros, - isDebuggable, multiProcessDefault); + private void setupWithAllParameters(WebViewProviderInfo[] packages, int numRelros, + boolean isDebuggable, boolean multiProcessDefault) { + TestSystemImpl testing = new TestSystemImpl(packages, numRelros, isDebuggable, + multiProcessDefault); mTestSystemImpl = Mockito.spy(testing); mWebViewUpdateServiceImpl = new WebViewUpdateServiceImpl(null /*Context*/, mTestSystemImpl); @@ -514,49 +508,29 @@ public class WebViewUpdateServiceTest { } /** - * Scenario for testing migrating away from the fallback logic. - * We start with a primary package that's a disabled fallback, and an enabled secondary, - * so that the fallback being re-enabled will cause a provider switch, as that covers - * the most complex case. + * Scenario for testing re-enabling a fallback package. */ @Test - public void testFallbackLogicMigration() { - String primaryPackage = "primary"; - String secondaryPackage = "secondary"; + public void testFallbackPackageEnabling() { + String testPackage = "testFallback"; WebViewProviderInfo[] packages = new WebViewProviderInfo[] { new WebViewProviderInfo( - primaryPackage, "", true /* default available */, true /* fallback */, null), - new WebViewProviderInfo( - secondaryPackage, "", true /* default available */, false /* fallback */, - null)}; - setupWithPackagesAndFallbackLogic(packages); + testPackage, "", true /* default available */, true /* fallback */, null)}; + setupWithPackages(packages); mTestSystemImpl.setPackageInfo( - createPackageInfo(primaryPackage, false /* enabled */ , true /* valid */, - true /* installed */)); - mTestSystemImpl.setPackageInfo( - createPackageInfo(secondaryPackage, true /* enabled */ , true /* valid */, + createPackageInfo(testPackage, false /* enabled */ , true /* valid */, true /* installed */)); - // Check that the boot time logic re-enables and chooses the primary, and disables the - // fallback logic. + // Check that the boot time logic re-enables the fallback package. runWebViewBootPreparationOnMainSync(); Mockito.verify(mTestSystemImpl).enablePackageForAllUsers( - Matchers.anyObject(), Mockito.eq(primaryPackage), Mockito.eq(true)); - checkPreparationPhasesForPackage(primaryPackage, 1); - assertFalse(mTestSystemImpl.isFallbackLogicEnabled()); + Matchers.anyObject(), Mockito.eq(testPackage), Mockito.eq(true)); - // Disable primary again - mTestSystemImpl.setPackageInfo(createPackageInfo(primaryPackage, false /* enabled */, - true /* valid */, true /* installed */)); - mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage, - WebViewUpdateService.PACKAGE_CHANGED, TestSystemImpl.PRIMARY_USER_ID); - checkPreparationPhasesForPackage(secondaryPackage, 1); - - // Run boot logic again and check that we didn't re-enable the primary a second time. - runWebViewBootPreparationOnMainSync(); - Mockito.verify(mTestSystemImpl, Mockito.times(1)).enablePackageForAllUsers( - Matchers.anyObject(), Mockito.eq(primaryPackage), Mockito.eq(true)); - checkPreparationPhasesForPackage(secondaryPackage, 2); + // Fake the message about the enabling having changed the package state, + // and check we now use that package. + mWebViewUpdateServiceImpl.packageStateChanged( + testPackage, WebViewUpdateService.PACKAGE_CHANGED, TestSystemImpl.PRIMARY_USER_ID); + checkPreparationPhasesForPackage(testPackage, 1); } /**