Merge "Fix the behaviour for explicitly switching WebView provider" into nyc-dev
diff --git a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
index 8c01c4c..7bd1f63 100644
--- a/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
+++ b/services/core/java/com/android/server/webkit/WebViewUpdateServiceImpl.java
@@ -308,38 +308,42 @@
/**
* Change WebView provider and provider setting and kill packages using the old provider.
- * Return the new provider (in case we are in the middle of creating relro files this new
- * provider will not be in use directly, but will when the relros are done).
+ * Return the new provider (in case we are in the middle of creating relro files, or
+ * replacing that provider it will not be in use directly, but will be used when the relros
+ * or the replacement are done).
*/
public String changeProviderAndSetting(String newProviderName) {
PackageInfo oldPackage = null;
PackageInfo newPackage = null;
+ boolean providerChanged = false;
synchronized(mLock) {
oldPackage = mCurrentWebViewPackage;
mSystemInterface.updateUserSetting(mContext, newProviderName);
try {
newPackage = findPreferredWebViewPackage();
- if (oldPackage != null
- && newPackage.packageName.equals(oldPackage.packageName)) {
- // If we don't perform the user change, revert the settings change.
- mSystemInterface.updateUserSetting(mContext, newPackage.packageName);
- return newPackage.packageName;
- }
+ providerChanged = (oldPackage == null)
+ || !newPackage.packageName.equals(oldPackage.packageName);
} catch (WebViewFactory.MissingWebViewPackageException e) {
Slog.e(TAG, "Tried to change WebView provider but failed to fetch WebView " +
"package " + e);
// If we don't perform the user change but don't have an installed WebView
// package, we will have changed the setting and it will be used when a package
// is available.
- return newProviderName;
+ return "";
}
- onWebViewProviderChanged(newPackage);
+ // Perform the provider change if we chose a new provider
+ if (providerChanged) {
+ onWebViewProviderChanged(newPackage);
+ }
}
- // Kill apps using the old provider
- if (oldPackage != null) {
+ // Kill apps using the old provider only if we changed provider
+ if (providerChanged && oldPackage != null) {
mSystemInterface.killPackageDependents(oldPackage.packageName);
}
+ // Return the new provider, this is not necessarily the one we were asked to switch to
+ // But the persistent setting will now be pointing to the provider we were asked to
+ // switch to anyway
return newPackage.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 26b87c5..e33be40 100644
--- a/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java
+++ b/services/tests/servicestests/src/com/android/server/webkit/TestSystemImpl.java
@@ -24,7 +24,7 @@
import java.util.HashMap;
public class TestSystemImpl implements SystemInterface {
- private String mUserProvider = "";
+ private String mUserProvider = null;
private final WebViewProviderInfo[] mPackageConfigs;
HashMap<String, PackageInfo> mPackages = new HashMap();
private boolean mFallbackLogicEnabled;
@@ -105,6 +105,10 @@
mPackages.put(pi.packageName, pi);
}
+ public void removePackageInfo(String packageName) {
+ mPackages.remove(packageName);
+ }
+
@Override
public int getFactoryPackageVersion(String packageName) {
return 0;
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 7d404c9..c03324a 100644
--- a/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/webkit/WebViewUpdateServiceTest.java
@@ -269,14 +269,27 @@
}
public void testFailListingInvalidWebviewPackage() {
- WebViewProviderInfo wpi = new WebViewProviderInfo("", "", true, true, null);
+ WebViewProviderInfo wpi = new WebViewProviderInfo("package", "", true, true, null);
WebViewProviderInfo[] packages = new WebViewProviderInfo[] {wpi};
setupWithPackages(packages);
- mTestSystemImpl.setPackageInfo(createPackageInfo(wpi.packageName, true, false));
+ mTestSystemImpl.setPackageInfo(
+ createPackageInfo(wpi.packageName, true /* enabled */, false /* valid */));
mWebViewUpdateServiceImpl.prepareWebViewInSystemServer();
+
+ Mockito.verify(mTestSystemImpl, Mockito.never()).onWebViewProviderChanged(
+ Matchers.anyObject());
+
WebViewProviderResponse response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
assertEquals(WebViewFactory.LIBLOAD_FAILED_LISTING_WEBVIEW_PACKAGES, response.status);
+
+ // Verify that we can recover from failing to list webview packages.
+ mTestSystemImpl.setPackageInfo(
+ createPackageInfo(wpi.packageName, true /* enabled */, true /* valid */));
+ mWebViewUpdateServiceImpl.packageStateChanged(wpi.packageName,
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
+
+ checkPreparationPhasesForPackage(wpi.packageName, 1);
}
// Test that switching provider using changeProviderAndSetting works.
@@ -463,7 +476,7 @@
mTestSystemImpl.setPackageInfo(
createPackageInfo(primaryPackage, true /* enabled */ , true /* valid */));
mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
- WebViewUpdateService.PACKAGE_ADDED);
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
// Verify fallback disabled, primary package used as provider, and fallback package killed
Mockito.verify(mTestSystemImpl).uninstallAndDisablePackageForAllUsers(
@@ -490,16 +503,22 @@
Mockito.eq(fallbackPackage), Mockito.eq(false) /* enable */,
Matchers.anyInt());
+ checkPreparationPhasesForPackage(primaryPackage, 1);
+
+ // Disable primary package and ensure fallback becomes enabled and used
mTestSystemImpl.setPackageInfo(
createPackageInfo(primaryPackage, false /* enabled */, true /* valid */));
mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
WebViewUpdateService.PACKAGE_CHANGED);
- // Verify fallback becomes enabled when primary package becomes disabled
Mockito.verify(mTestSystemImpl).enablePackageForUser(
Mockito.eq(fallbackPackage), Mockito.eq(true) /* enable */,
Matchers.anyInt());
+ checkPreparationPhasesForPackage(fallbackPackage, 1);
+
+
+ // Again enable primary package and verify primary is used and fallback becomes disabled
mTestSystemImpl.setPackageInfo(
createPackageInfo(primaryPackage, true /* enabled */, true /* valid */));
mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
@@ -509,6 +528,8 @@
Mockito.verify(mTestSystemImpl, Mockito.times(2)).enablePackageForUser(
Mockito.eq(fallbackPackage), Mockito.eq(false) /* enable */,
Matchers.anyInt());
+
+ checkPreparationPhasesForPackage(primaryPackage, 2);
}
public void testAddUserWhenFallbackLogicEnabled() {
@@ -565,6 +586,9 @@
Mockito.verify(mTestSystemImpl).onWebViewProviderChanged(
Mockito.argThat(new IsPackageInfoWithName(firstPackage)));
+ // Change provider during relro creation to enter a state where we are
+ // waiting for relro creation to complete just to re-run relro creation.
+ // (so that in next notifyRelroCreationCompleted() call we have to list webview packages)
mWebViewUpdateServiceImpl.changeProviderAndSetting(secondPackage);
// Make packages invalid to cause exception to be thrown
@@ -584,7 +608,7 @@
true /* valid */));
mWebViewUpdateServiceImpl.packageStateChanged(firstPackage,
- WebViewUpdateService.PACKAGE_ADDED);
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
// Ensure we use firstPackage
checkPreparationPhasesForPackage(firstPackage, 2 /* second preparation for this package */);
@@ -654,5 +678,143 @@
checkPreparationPhasesForPackage(nonChosenPackage, 1);
}
- // TODO (gsennton) add more tests for ensuring killPackageDependents is called / not called
+ public void testRecoverFailedListingWebViewPackagesSettingsChange() {
+ checkRecoverAfterFailListingWebviewPackages(true);
+ }
+
+ public void testRecoverFailedListingWebViewPackagesAddedPackage() {
+ checkRecoverAfterFailListingWebviewPackages(false);
+ }
+
+ /**
+ * Test that we can recover correctly from failing to list WebView packages.
+ * settingsChange: whether to fail during changeProviderAndSetting or packageStateChanged
+ */
+ public void checkRecoverAfterFailListingWebviewPackages(boolean settingsChange) {
+ String firstPackage = "first";
+ String secondPackage = "second";
+ WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
+ new WebViewProviderInfo(firstPackage, "", true /* default available */,
+ false /* fallback */, null),
+ new WebViewProviderInfo(secondPackage, "", true /* default available */,
+ false /* fallback */, null)};
+ checkCertainPackageUsedAfterWebViewBootPreparation(firstPackage, packages);
+
+ // Make both packages invalid so that we fail listing WebView packages
+ mTestSystemImpl.setPackageInfo(createPackageInfo(firstPackage, true /* enabled */,
+ false /* valid */));
+ mTestSystemImpl.setPackageInfo(createPackageInfo(secondPackage, true /* enabled */,
+ false /* valid */));
+
+ // Change package to hit the webview packages listing problem.
+ if (settingsChange) {
+ mWebViewUpdateServiceImpl.changeProviderAndSetting(secondPackage);
+ } else {
+ mWebViewUpdateServiceImpl.packageStateChanged(secondPackage,
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
+ }
+
+ WebViewProviderResponse response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
+ assertEquals(WebViewFactory.LIBLOAD_FAILED_LISTING_WEBVIEW_PACKAGES, response.status);
+
+ // Make second package valid and verify that we can load it again
+ mTestSystemImpl.setPackageInfo(createPackageInfo(secondPackage, true /* enabled */,
+ true /* valid */));
+
+ mWebViewUpdateServiceImpl.packageStateChanged(secondPackage,
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
+
+
+ checkPreparationPhasesForPackage(secondPackage, 1);
+ }
+
+ public void testDontKillIfPackageReplaced() {
+ checkDontKillIfPackageRemoved(true);
+ }
+
+ public void testDontKillIfPackageRemoved() {
+ checkDontKillIfPackageRemoved(false);
+ }
+
+ public void checkDontKillIfPackageRemoved(boolean replaced) {
+ String firstPackage = "first";
+ String secondPackage = "second";
+ WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
+ new WebViewProviderInfo(firstPackage, "", true /* default available */,
+ false /* fallback */, null),
+ new WebViewProviderInfo(secondPackage, "", true /* default available */,
+ false /* fallback */, null)};
+ checkCertainPackageUsedAfterWebViewBootPreparation(firstPackage, packages);
+
+ // Replace or remove the current webview package
+ if (replaced) {
+ mTestSystemImpl.setPackageInfo(
+ createPackageInfo(firstPackage, true /* enabled */, false /* valid */));
+ mWebViewUpdateServiceImpl.packageStateChanged(firstPackage,
+ WebViewUpdateService.PACKAGE_ADDED_REPLACED);
+ } else {
+ mTestSystemImpl.removePackageInfo(firstPackage);
+ mWebViewUpdateServiceImpl.packageStateChanged(firstPackage,
+ WebViewUpdateService.PACKAGE_REMOVED);
+ }
+
+ checkPreparationPhasesForPackage(secondPackage, 1);
+
+ Mockito.verify(mTestSystemImpl, Mockito.never()).killPackageDependents(
+ Mockito.anyObject());
+ }
+
+ public void testKillIfSettingChanged() {
+ String firstPackage = "first";
+ String secondPackage = "second";
+ WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
+ new WebViewProviderInfo(firstPackage, "", true /* default available */,
+ false /* fallback */, null),
+ new WebViewProviderInfo(secondPackage, "", true /* default available */,
+ false /* fallback */, null)};
+ checkCertainPackageUsedAfterWebViewBootPreparation(firstPackage, packages);
+
+ mWebViewUpdateServiceImpl.changeProviderAndSetting(secondPackage);
+
+ checkPreparationPhasesForPackage(secondPackage, 1);
+
+ Mockito.verify(mTestSystemImpl).killPackageDependents(Mockito.eq(firstPackage));
+ }
+
+ /**
+ * Test that we kill apps using an old provider when we change the provider setting, even if the
+ * new provider is not the one we intended to change to.
+ */
+ public void testKillIfChangeProviderIncorrectly() {
+ String firstPackage = "first";
+ String secondPackage = "second";
+ String thirdPackage = "third";
+ WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
+ new WebViewProviderInfo(firstPackage, "", true /* default available */,
+ false /* fallback */, null),
+ new WebViewProviderInfo(secondPackage, "", true /* default available */,
+ false /* fallback */, null),
+ new WebViewProviderInfo(thirdPackage, "", true /* default available */,
+ false /* fallback */, null)};
+ setupWithPackages(packages);
+ setEnabledAndValidPackageInfos(packages);
+
+ // Start with the setting pointing to the third package
+ mTestSystemImpl.updateUserSetting(null, thirdPackage);
+
+ mWebViewUpdateServiceImpl.prepareWebViewInSystemServer();
+ checkPreparationPhasesForPackage(thirdPackage, 1);
+
+ mTestSystemImpl.setPackageInfo(
+ createPackageInfo(secondPackage, true /* enabled */, false /* valid */));
+
+ // Try to switch to the invalid second package, this should result in switching to the first
+ // package, since that is more preferred than the third one.
+ assertEquals(firstPackage,
+ mWebViewUpdateServiceImpl.changeProviderAndSetting(secondPackage));
+
+ checkPreparationPhasesForPackage(firstPackage, 1);
+
+ Mockito.verify(mTestSystemImpl).killPackageDependents(Mockito.eq(thirdPackage));
+ }
}