Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a9159040 authored by Gustav Sennton's avatar Gustav Sennton
Browse files

Update persistent WebView packages setting only when user changes it.

To ensure that we don't permanently change WebView implementation if the
current package is temporarily uninstalled (e.g. when being replaced) we
don't update our persistent setting unless the user explicitly changes
WebView implementation (and on boot!).

Unfortunately this will means that the Dev Setting for changing WebView
implementation will work in a slightly less intuitive way. The
persistent setting is now persistent across uninstalls and installs.
I.e. the Dev Setting shows the current WebView implementation though
that could differ to the value chosen by the user since the package
chosen by the user could be uninstalled or disabled. In this case
installing/enabling that package would again make the Dev Setting point
to it.
However, as a compromise, we do change the setting at boot so that if
the currently chosen package is not valid we will change the setting so
that it points to the package we currently use instead.

Also ensure we only use WebView packages that are available-by-default
if no WebView packages are enabled.

Add unit test to ensure that if a user-chosen provider is uninstalled we
switch back to using that provider when it is installed again.
Add unit test to ensure we switch user-chosen provider at boot if the
chosen one is uninstalled.

Bug: 27673076
Change-Id: Icd27ae302798ebf695b9ef4bd4d5fd47fe4be02c
parent 557c716a
Loading
Loading
Loading
Loading
+12 −3
Original line number Diff line number Diff line
@@ -283,6 +283,13 @@ public class WebViewUpdateServiceImpl {
            try {
                synchronized(mLock) {
                    mCurrentWebViewPackage = findPreferredWebViewPackage();
                    // Don't persist the user-chosen setting across boots if the package being
                    // chosen is not used (could be disabled or uninstalled) so that the user won't
                    // be surprised by the device switching to using a certain webview package,
                    // that was uninstalled/disabled a long time ago, if it is installed/enabled
                    // again.
                    mSystemInterface.updateUserSetting(mContext,
                            mCurrentWebViewPackage.packageName);
                    onWebViewProviderChanged(mCurrentWebViewPackage);
                }
            } catch (Throwable t) {
@@ -337,7 +344,6 @@ public class WebViewUpdateServiceImpl {
                mAnyWebViewInstalled = true;
                if (mNumRelroCreationsStarted == mNumRelroCreationsFinished) {
                    mCurrentWebViewPackage = newPackage;
                    mSystemInterface.updateUserSetting(mContext, newPackage.packageName);

                    // The relro creations might 'finish' (not start at all) before
                    // WebViewFactory.onWebViewProviderChanged which means we might not know the
@@ -424,10 +430,13 @@ public class WebViewUpdateServiceImpl {
                }
            }

            // Could not find any enabled package either, use the most stable provider.
            // Could not find any enabled package either, use the most stable and default-available
            // provider.
            for (ProviderAndPackageInfo providerAndPackage : providers) {
                if (providerAndPackage.provider.availableByDefault) {
                    return providerAndPackage.packageInfo;
                }
            }

            mAnyWebViewInstalled = false;
            throw new WebViewFactory.MissingWebViewPackageException(
+91 −46
Original line number Diff line number Diff line
@@ -90,12 +90,13 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        }
    }

    private void checkCertainPackageUsedAfterWebViewPreparation(String expectedProviderName,
    private void checkCertainPackageUsedAfterWebViewBootPreparation(String expectedProviderName,
            WebViewProviderInfo[] webviewPackages) {
        checkCertainPackageUsedAfterWebViewPreparation(expectedProviderName, webviewPackages, 1);
        checkCertainPackageUsedAfterWebViewBootPreparation(
                expectedProviderName, webviewPackages, 1);
    }

    private void checkCertainPackageUsedAfterWebViewPreparation(String expectedProviderName,
    private void checkCertainPackageUsedAfterWebViewBootPreparation(String expectedProviderName,
            WebViewProviderInfo[] webviewPackages, int numRelros) {
        setupWithPackages(webviewPackages, true, numRelros);
        // Add (enabled and valid) package infos for each provider
@@ -156,6 +157,19 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        return p;
    }

    private void checkPreparationPhasesForPackage(String expectedPackage, int numPreparation) {
        // Verify that onWebViewProviderChanged was called for the numPreparation'th time for the
        // expected package
        Mockito.verify(mTestSystemImpl, Mockito.times(numPreparation)).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(expectedPackage)));

        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();

        WebViewProviderResponse response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, response.status);
        assertEquals(expectedPackage, response.packageInfo.packageName);
    }


    // ****************
    // Tests
@@ -164,7 +178,7 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {

    public void testWithSinglePackage() {
        String testPackageName = "test.package.name";
        checkCertainPackageUsedAfterWebViewPreparation(testPackageName,
        checkCertainPackageUsedAfterWebViewBootPreparation(testPackageName,
                new WebViewProviderInfo[] {
                    new WebViewProviderInfo(testPackageName, "",
                            true /*default available*/, false /* fallback */, null)});
@@ -176,12 +190,12 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
            new WebViewProviderInfo(nonDefaultPackage, "", false, false, null),
            new WebViewProviderInfo(defaultPackage, "", true, false, null)};
        checkCertainPackageUsedAfterWebViewPreparation(defaultPackage, packages);
        checkCertainPackageUsedAfterWebViewBootPreparation(defaultPackage, packages);
    }

    public void testSeveralRelros() {
        String singlePackage = "singlePackage";
        checkCertainPackageUsedAfterWebViewPreparation(
        checkCertainPackageUsedAfterWebViewBootPreparation(
                singlePackage,
                new WebViewProviderInfo[] {
                    new WebViewProviderInfo(singlePackage, "", true /*def av*/, false, null)},
@@ -215,14 +229,8 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {

        mWebViewUpdateServiceImpl.prepareWebViewInSystemServer();

        Mockito.verify(mTestSystemImpl).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(validPackage)));

        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();

        WebViewProviderResponse response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, response.status);
        assertEquals(validPackage, response.packageInfo.packageName);
        checkPreparationPhasesForPackage(validPackage, 1 /* first preparation for this package */);

        WebViewProviderInfo[] validPackages = mWebViewUpdateServiceImpl.getValidWebViewPackages();
        assertEquals(1, validPackages.length);
@@ -292,18 +300,10 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {

    private void checkSwitchingProvider(WebViewProviderInfo[] packages, String initialPackage,
            String finalPackage) {
        checkCertainPackageUsedAfterWebViewPreparation(initialPackage, packages);
        checkCertainPackageUsedAfterWebViewBootPreparation(initialPackage, packages);

        mWebViewUpdateServiceImpl.changeProviderAndSetting(finalPackage);

        Mockito.verify(mTestSystemImpl).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(finalPackage)));

        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();

        WebViewProviderResponse secondResponse = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, secondResponse.status);
        assertEquals(finalPackage, secondResponse.packageInfo.packageName);
        checkPreparationPhasesForPackage(finalPackage, 1 /* first preparation for this package */);

        Mockito.verify(mTestSystemImpl).killPackageDependents(Mockito.eq(initialPackage));
    }
@@ -455,14 +455,9 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        mWebViewUpdateServiceImpl.prepareWebViewInSystemServer();
        Mockito.verify(mTestSystemImpl, Mockito.never()).uninstallAndDisablePackageForAllUsers(
                Matchers.anyObject(), Matchers.anyObject());
        Mockito.verify(mTestSystemImpl).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(fallbackPackage)));

        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();

        WebViewProviderResponse response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, response.status);
        assertEquals(fallbackPackage, response.packageInfo.packageName);
        checkPreparationPhasesForPackage(fallbackPackage,
                1 /* first preparation for this package*/);

        // Install primary package
        mTestSystemImpl.setPackageInfo(
@@ -470,17 +465,10 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        mWebViewUpdateServiceImpl.packageStateChanged(primaryPackage,
                WebViewUpdateService.PACKAGE_ADDED);

        // Verify fallback disabled and primary package used as provider
        // Verify fallback disabled, primary package used as provider, and fallback package killed
        Mockito.verify(mTestSystemImpl).uninstallAndDisablePackageForAllUsers(
                Matchers.anyObject(), Mockito.eq(fallbackPackage));
        Mockito.verify(mTestSystemImpl).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(primaryPackage)));

        // Finish the webview preparation and ensure primary package used and fallback killed
        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();
        response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, response.status);
        assertEquals(primaryPackage, response.packageInfo.packageName);
        checkPreparationPhasesForPackage(primaryPackage, 1 /* first preparation for this package*/);
        Mockito.verify(mTestSystemImpl).killPackageDependents(Mockito.eq(fallbackPackage));
    }

@@ -598,15 +586,72 @@ public class WebViewUpdateServiceTest extends AndroidTestCase {
        mWebViewUpdateServiceImpl.packageStateChanged(firstPackage,
                WebViewUpdateService.PACKAGE_ADDED);

        // Second time we call onWebViewProviderChanged for firstPackage
        Mockito.verify(mTestSystemImpl, Mockito.times(2)).onWebViewProviderChanged(
                Mockito.argThat(new IsPackageInfoWithName(firstPackage)));
        // Ensure we use firstPackage
        checkPreparationPhasesForPackage(firstPackage, 2 /* second preparation for this package */);
    }

        mWebViewUpdateServiceImpl.notifyRelroCreationCompleted();
    /**
     * Verify that even if a user-chosen package is removed temporarily we start using it again when
     * it is added back.
     */
    public void testTempRemovePackageDoesntSwitchProviderPermanently() {
        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);

        response = mWebViewUpdateServiceImpl.waitForAndGetProvider();
        assertEquals(WebViewFactory.LIBLOAD_SUCCESS, response.status);
        assertEquals(firstPackage, response.packageInfo.packageName);
        // Explicitly use the second package
        mWebViewUpdateServiceImpl.changeProviderAndSetting(secondPackage);
        checkPreparationPhasesForPackage(secondPackage, 1 /* first time for this package */);

        // Remove second package (invalidate it) and verify that first package is used
        mTestSystemImpl.setPackageInfo(createPackageInfo(secondPackage, true /* enabled */,
                    false /* valid */));
        mWebViewUpdateServiceImpl.packageStateChanged(secondPackage,
                WebViewUpdateService.PACKAGE_ADDED);
        checkPreparationPhasesForPackage(firstPackage, 2 /* second time for this package */);

        // Now make the second package valid again and verify that it is used again
        mTestSystemImpl.setPackageInfo(createPackageInfo(secondPackage, true /* enabled */,
                    true /* valid */));
        mWebViewUpdateServiceImpl.packageStateChanged(secondPackage,
                WebViewUpdateService.PACKAGE_ADDED);
        checkPreparationPhasesForPackage(secondPackage, 2 /* second time for this package */);
    }

    /**
     * Ensure that we update the user-chosen setting across boots if the chosen package is no
     * longer installed and valid.
     */
    public void testProviderSettingChangedDuringBootIfProviderNotAvailable() {
        String chosenPackage = "chosenPackage";
        String nonChosenPackage = "non-chosenPackage";
        WebViewProviderInfo[] packages = new WebViewProviderInfo[] {
            new WebViewProviderInfo(chosenPackage, "", true /* default available */,
                    false /* fallback */, null),
            new WebViewProviderInfo(nonChosenPackage, "", true /* default available */,
                    false /* fallback */, null)};

        setupWithPackages(packages);
        // Only 'install' nonChosenPackage
        mTestSystemImpl.setPackageInfo(
                createPackageInfo(nonChosenPackage, true /* enabled */, true /* valid */));

        // Set user-chosen package
        mTestSystemImpl.updateUserSetting(null, chosenPackage);

        mWebViewUpdateServiceImpl.prepareWebViewInSystemServer();

        // Verify that we switch the setting to point to the current package
        Mockito.verify(mTestSystemImpl).updateUserSetting(
                Mockito.anyObject(), Mockito.eq(nonChosenPackage));
        assertEquals(nonChosenPackage, mTestSystemImpl.getUserChosenWebViewProvider(null));

        checkPreparationPhasesForPackage(nonChosenPackage, 1);
    }

    // TODO (gsennton) add more tests for ensuring killPackageDependents is called / not called