Commit 4ee98e55 authored by Peter Kotwicz's avatar Peter Kotwicz Committed by Commit Bot

[Android WebAPKs] Reland: Fix launching > 1 WebAPK on pre-Android-N

This CL:
- Makes WebappLauncherActivity set the correct URI on the launch intent
for WebAPKs
- Renames WebappIntentUtils#getId() to make it clear that it only
applies to homescreen shortcuts
- Moves WebappRegistry#webApkIdForPackage() to WebappIntentUtils
alongside WebappIntentUtils#getIdForHomescreenShortcut()
- Adds a boolean parameter to
WebApkValidator#disableValidationForTesting() for the sake of tests
which need to keep validation enabled.

BUG=1114560
TEST=WebappLauncherActivityTest.*
R=peconn
TBR=sky
    (For trivial change to MapsGoFirstRunTest.java )

Change-Id: I615b1aca48b4a0c626a1e02613c58e9bdebc44ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348050
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796974}
parent b2df9948
...@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.preferences.SharedPreferencesManager; ...@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.webapps.WebApkDistributor; import org.chromium.chrome.browser.webapps.WebApkDistributor;
import org.chromium.chrome.browser.webapps.WebApkUkmRecorder; import org.chromium.chrome.browser.webapps.WebApkUkmRecorder;
import org.chromium.chrome.browser.webapps.WebappDataStorage; import org.chromium.chrome.browser.webapps.WebappDataStorage;
import org.chromium.chrome.browser.webapps.WebappIntentUtils;
import org.chromium.chrome.browser.webapps.WebappRegistry; import org.chromium.chrome.browser.webapps.WebappRegistry;
import org.chromium.components.browser_ui.util.ConversionUtils; import org.chromium.components.browser_ui.util.ConversionUtils;
...@@ -94,8 +95,7 @@ public class WebApkUma { ...@@ -94,8 +95,7 @@ public class WebApkUma {
int NUM_ENTRIES = 16; int NUM_ENTRIES = 16;
} }
public static final String HISTOGRAM_UPDATE_REQUEST_SENT = public static final String HISTOGRAM_UPDATE_REQUEST_SENT = "WebApk.Update.RequestSent";
"WebApk.Update.RequestSent";
public static final String HISTOGRAM_UPDATE_REQUEST_QUEUED = "WebApk.Update.RequestQueued"; public static final String HISTOGRAM_UPDATE_REQUEST_QUEUED = "WebApk.Update.RequestQueued";
...@@ -126,7 +126,7 @@ public class WebApkUma { ...@@ -126,7 +126,7 @@ public class WebApkUma {
for (String uninstalledPackage : uninstalledPackages) { for (String uninstalledPackage : uninstalledPackages) {
RecordHistogram.recordBooleanHistogram("WebApk.Uninstall.Browser", true); RecordHistogram.recordBooleanHistogram("WebApk.Uninstall.Browser", true);
String webApkId = WebappRegistry.webApkIdForPackage(uninstalledPackage); String webApkId = WebappIntentUtils.getIdForWebApkPackage(uninstalledPackage);
WebappDataStorage webappDataStorage = WebappDataStorage webappDataStorage =
WebappRegistry.getInstance().getWebappDataStorage(webApkId); WebappRegistry.getInstance().getWebappDataStorage(webApkId);
if (webappDataStorage != null) { if (webappDataStorage != null) {
...@@ -150,7 +150,7 @@ public class WebApkUma { ...@@ -150,7 +150,7 @@ public class WebApkUma {
public static void deferRecordWebApkUninstalled(String packageName) { public static void deferRecordWebApkUninstalled(String packageName) {
SharedPreferencesManager.getInstance().addToStringSet( SharedPreferencesManager.getInstance().addToStringSet(
ChromePreferenceKeys.WEBAPK_UNINSTALLED_PACKAGES, packageName); ChromePreferenceKeys.WEBAPK_UNINSTALLED_PACKAGES, packageName);
String webApkId = WebappRegistry.webApkIdForPackage(packageName); String webApkId = WebappIntentUtils.getIdForWebApkPackage(packageName);
WebappRegistry.warmUpSharedPrefsForId(webApkId); WebappRegistry.warmUpSharedPrefsForId(webApkId);
WebappDataStorage webappDataStorage = WebappDataStorage webappDataStorage =
WebappRegistry.getInstance().getWebappDataStorage(webApkId); WebappRegistry.getInstance().getWebappDataStorage(webApkId);
...@@ -306,8 +306,7 @@ public class WebApkUma { ...@@ -306,8 +306,7 @@ public class WebApkUma {
protected void onPostExecute(Void result) { protected void onPostExecute(Void result) {
logSpaceUsageUMAOnDataAvailable(mAvailableSpaceInByte, mCacheSizeInByte); logSpaceUsageUMAOnDataAvailable(mAvailableSpaceInByte, mCacheSizeInByte);
} }
} }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
} }
private static void logSpaceUsageUMAOnDataAvailable(long spaceSize, long cacheSize) { private static void logSpaceUsageUMAOnDataAvailable(long spaceSize, long cacheSize) {
......
...@@ -33,7 +33,7 @@ public class ChromeWebApkHost { ...@@ -33,7 +33,7 @@ public class ChromeWebApkHost {
if (ChromeVersionInfo.isLocalBuild() if (ChromeVersionInfo.isLocalBuild()
&& CommandLine.getInstance().hasSwitch(SKIP_WEBAPK_VERIFICATION)) { && CommandLine.getInstance().hasSwitch(SKIP_WEBAPK_VERIFICATION)) {
// Tell the WebApkValidator to work for all WebAPKs. // Tell the WebApkValidator to work for all WebAPKs.
WebApkValidator.disableValidationForTesting(); WebApkValidator.setDisableValidationForTesting(true);
} }
} }
......
...@@ -84,7 +84,7 @@ public class WebApkInstaller { ...@@ -84,7 +84,7 @@ public class WebApkInstaller {
// Stores the source info of WebAPK in WebappDataStorage. // Stores the source info of WebAPK in WebappDataStorage.
WebappRegistry.getInstance().register( WebappRegistry.getInstance().register(
WebappRegistry.webApkIdForPackage(packageName), WebappIntentUtils.getIdForWebApkPackage(packageName),
new WebappRegistry.FetchWebappDataStorageCallback() { new WebappRegistry.FetchWebappDataStorageCallback() {
@Override @Override
public void onWebappDataStorageRetrieved(WebappDataStorage storage) { public void onWebappDataStorageRetrieved(WebappDataStorage storage) {
......
...@@ -408,8 +408,8 @@ public class WebApkIntentDataProviderFactory { ...@@ -408,8 +408,8 @@ public class WebApkIntentDataProviderFactory {
} }
WebappExtras webappExtras = new WebappExtras( WebappExtras webappExtras = new WebappExtras(
WebappRegistry.webApkIdForPackage(webApkPackageName), url, scope, primaryIcon, name, WebappIntentUtils.getIdForWebApkPackage(webApkPackageName), url, scope, primaryIcon,
shortName, displayMode, orientation, source, name, shortName, displayMode, orientation, source,
WebappIntentUtils.colorFromLongColor(backgroundColor), defaultBackgroundColor, WebappIntentUtils.colorFromLongColor(backgroundColor), defaultBackgroundColor,
false /* isIconGenerated */, isPrimaryIconMaskable, forceNavigation); false /* isIconGenerated */, isPrimaryIconMaskable, forceNavigation);
WebApkExtras webApkExtras = new WebApkExtras(webApkPackageName, splashIcon, WebApkExtras webApkExtras = new WebApkExtras(webApkPackageName, splashIcon,
......
...@@ -54,7 +54,7 @@ public class WebappIntentDataProviderFactory { ...@@ -54,7 +54,7 @@ public class WebappIntentDataProviderFactory {
* @param intent Intent containing info about the app. * @param intent Intent containing info about the app.
*/ */
public static BrowserServicesIntentDataProvider create(Intent intent) { public static BrowserServicesIntentDataProvider create(Intent intent) {
String id = WebappIntentUtils.getId(intent); String id = WebappIntentUtils.getIdForHomescreenShortcut(intent);
String url = IntentUtils.safeGetStringExtra(intent, ShortcutHelper.EXTRA_URL); String url = IntentUtils.safeGetStringExtra(intent, ShortcutHelper.EXTRA_URL);
if (id == null || url == null) { if (id == null || url == null) {
Log.e(TAG, "Incomplete data provided: " + id + ", " + url); Log.e(TAG, "Incomplete data provided: " + id + ", " + url);
......
...@@ -70,10 +70,20 @@ public class WebappIntentUtils { ...@@ -70,10 +70,20 @@ public class WebappIntentUtils {
return isLongColorValid(longColor) ? Integer.valueOf((int) longColor) : null; return isLongColorValid(longColor) ? Integer.valueOf((int) longColor) : null;
} }
public static String getId(Intent intent) { /**
* Extracts id from homescreen shortcut intent.
*/
public static String getIdForHomescreenShortcut(Intent intent) {
return IntentUtils.safeGetStringExtra(intent, ShortcutHelper.EXTRA_ID); return IntentUtils.safeGetStringExtra(intent, ShortcutHelper.EXTRA_ID);
} }
/**
* Generates id for the passed-in WebAPK package name.
*/
public static String getIdForWebApkPackage(String packageName) {
return WebApkConstants.WEBAPK_ID_PREFIX + packageName;
}
public static String getUrl(Intent intent) { public static String getUrl(Intent intent) {
return IntentUtils.safeGetStringExtra(intent, WebApkConstants.EXTRA_URL); return IntentUtils.safeGetStringExtra(intent, WebApkConstants.EXTRA_URL);
} }
......
...@@ -186,10 +186,13 @@ public class WebappLauncherActivity extends Activity { ...@@ -186,10 +186,13 @@ public class WebappLauncherActivity extends Activity {
*/ */
private static LaunchData extractLaunchData(Intent intent) { private static LaunchData extractLaunchData(Intent intent) {
String webApkPackageName = WebappIntentUtils.getWebApkPackageName(intent); String webApkPackageName = WebappIntentUtils.getWebApkPackageName(intent);
boolean isSplashProvidedByWebApk = !TextUtils.isEmpty(webApkPackageName) boolean isForWebApk = !TextUtils.isEmpty(webApkPackageName);
boolean isSplashProvidedByWebApk = isForWebApk
&& IntentUtils.safeGetBooleanExtra(intent, EXTRA_SPLASH_PROVIDED_BY_WEBAPK, false); && IntentUtils.safeGetBooleanExtra(intent, EXTRA_SPLASH_PROVIDED_BY_WEBAPK, false);
return new LaunchData(WebappIntentUtils.getId(intent), WebappIntentUtils.getUrl(intent), String id = isForWebApk ? WebappIntentUtils.getIdForWebApkPackage(webApkPackageName)
webApkPackageName, isSplashProvidedByWebApk); : WebappIntentUtils.getIdForHomescreenShortcut(intent);
return new LaunchData(
id, WebappIntentUtils.getUrl(intent), webApkPackageName, isSplashProvidedByWebApk);
} }
/** /**
......
...@@ -97,13 +97,6 @@ public class WebappRegistry { ...@@ -97,13 +97,6 @@ public class WebappRegistry {
return Holder.sInstance; return Holder.sInstance;
} }
/**
* Returns the {@link WebappDataStorage} id for the passed-in WebAPK package name.
*/
public static String webApkIdForPackage(String webApkPackageName) {
return WebApkConstants.WEBAPK_ID_PREFIX + webApkPackageName;
}
/** /**
* Warm up the WebappRegistry and a specific WebappDataStorage SharedPreferences. Can be called * Warm up the WebappRegistry and a specific WebappDataStorage SharedPreferences. Can be called
* from any thread. * from any thread.
......
...@@ -57,7 +57,7 @@ public class WebApkIntegrationTest { ...@@ -57,7 +57,7 @@ public class WebApkIntegrationTest {
Uri.parse(mActivityTestRule.getEmbeddedTestServerRule().getServer().getURL("/")); Uri.parse(mActivityTestRule.getEmbeddedTestServerRule().getServer().getURL("/"));
CommandLine.getInstance().appendSwitchWithValue( CommandLine.getInstance().appendSwitchWithValue(
ContentSwitches.HOST_RESOLVER_RULES, "MAP * " + mapToUri.getAuthority()); ContentSwitches.HOST_RESOLVER_RULES, "MAP * " + mapToUri.getAuthority());
WebApkValidator.disableValidationForTesting(); WebApkValidator.setDisableValidationForTesting(true);
} }
/** /**
......
...@@ -76,7 +76,7 @@ public final class FirstRunIntegrationUnitTest { ...@@ -76,7 +76,7 @@ public final class FirstRunIntegrationUnitTest {
mShadowApplication.setSystemService(Context.USER_SERVICE, userManager); mShadowApplication.setSystemService(Context.USER_SERVICE, userManager);
FirstRunStatus.setFirstRunFlowComplete(false); FirstRunStatus.setFirstRunFlowComplete(false);
WebApkValidator.disableValidationForTesting(); WebApkValidator.setDisableValidationForTesting(true);
} }
/** Checks that the intent component targets the passed-in class. */ /** Checks that the intent component targets the passed-in class. */
......
...@@ -261,7 +261,8 @@ public class WebApkUpdateManagerUnitTest { ...@@ -261,7 +261,8 @@ public class WebApkUpdateManagerUnitTest {
} }
private void registerStorageForWebApkPackage(String webApkPackageName) { private void registerStorageForWebApkPackage(String webApkPackageName) {
WebappRegistry.getInstance().register(WebappRegistry.webApkIdForPackage(webApkPackageName), WebappRegistry.getInstance().register(
WebappIntentUtils.getIdForWebApkPackage(webApkPackageName),
new WebappRegistry.FetchWebappDataStorageCallback() { new WebappRegistry.FetchWebappDataStorageCallback() {
@Override @Override
public void onWebappDataStorageRetrieved(WebappDataStorage storage) {} public void onWebappDataStorageRetrieved(WebappDataStorage storage) {}
...@@ -270,7 +271,7 @@ public class WebApkUpdateManagerUnitTest { ...@@ -270,7 +271,7 @@ public class WebApkUpdateManagerUnitTest {
private static WebappDataStorage getStorage(String packageName) { private static WebappDataStorage getStorage(String packageName) {
return WebappRegistry.getInstance().getWebappDataStorage( return WebappRegistry.getInstance().getWebappDataStorage(
WebappRegistry.webApkIdForPackage(packageName)); WebappIntentUtils.getIdForWebApkPackage(packageName));
} }
/** /**
......
...@@ -67,7 +67,7 @@ public class WebappDisclosureSnackbarControllerTest { ...@@ -67,7 +67,7 @@ public class WebappDisclosureSnackbarControllerTest {
} }
private WebappDataStorage registerStorageForWebApk(String packageName) { private WebappDataStorage registerStorageForWebApk(String packageName) {
String id = WebappRegistry.webApkIdForPackage(packageName); String id = WebappIntentUtils.getIdForWebApkPackage(packageName);
WebappRegistry.getInstance().register(id, (storage) -> {}); WebappRegistry.getInstance().register(id, (storage) -> {});
return WebappRegistry.getInstance().getWebappDataStorage(id); return WebappRegistry.getInstance().getWebappDataStorage(id);
} }
......
...@@ -4,17 +4,26 @@ ...@@ -4,17 +4,26 @@
package org.chromium.chrome.browser.webapps; package org.chromium.chrome.browser.webapps;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import android.content.Intent; import android.content.Intent;
import android.os.Bundle; import android.os.Bundle;
import org.junit.Assert; import org.junit.After;
import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.Robolectric; import org.robolectric.Robolectric;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowApplication;
import org.chromium.base.CommandLine;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.ShortcutHelper; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.components.webapk.lib.client.WebApkValidator;
import org.chromium.components.webapk.lib.common.WebApkMetaDataKeys; import org.chromium.components.webapk.lib.common.WebApkMetaDataKeys;
import org.chromium.webapk.lib.common.WebApkConstants; import org.chromium.webapk.lib.common.WebApkConstants;
import org.chromium.webapk.test.WebApkTestHelper; import org.chromium.webapk.test.WebApkTestHelper;
...@@ -26,24 +35,81 @@ public class WebappLauncherActivityTest { ...@@ -26,24 +35,81 @@ public class WebappLauncherActivityTest {
private static final String WEBAPK_PACKAGE_NAME = "org.chromium.webapk.test_package"; private static final String WEBAPK_PACKAGE_NAME = "org.chromium.webapk.test_package";
private static final String START_URL = "https://www.google.com/scope/a_is_for_apple"; private static final String START_URL = "https://www.google.com/scope/a_is_for_apple";
@Before
public void setUp() {
CommandLine.getInstance().appendSwitchWithValue(
ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, null);
WebApkValidator.setDisableValidationForTesting(true);
}
@After
public void tearDown() {
WebApkValidator.setDisableValidationForTesting(false);
}
/** /**
* Test that WebappLauncherActivity modifies the passed-in intent so that * Test that WebappLauncherActivity modifies the passed-in intent so that
* WebApkIntentDataProviderFactory#create() returns null if the intent does not refer to a valid * WebApkIntentDataProviderFactory#create() returns null if the intent does not refer to a valid
* WebAPK. * WebAPK.
*/ */
@Test @Test
public void tryCreateWebappInfoAltersIntentIfNotValidWebApk() { public void testTryCreateWebappInfoAltersIntentIfNotValidWebApk() {
WebApkValidator.setDisableValidationForTesting(false);
registerWebApk(WEBAPK_PACKAGE_NAME, START_URL);
Intent intent = WebApkTestHelper.createMinimalWebApkIntent(WEBAPK_PACKAGE_NAME, START_URL);
assertNotNull(WebApkIntentDataProviderFactory.create(intent));
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create();
assertNull(WebApkIntentDataProviderFactory.create(intent));
}
/**
* Test the launch intent created by {@link WebappLauncherActivity} for old-style WebAPKs.
*/
@Test
public void testOldStyleLaunchIntent() {
registerWebApk(WEBAPK_PACKAGE_NAME, START_URL);
Intent intent = WebApkTestHelper.createMinimalWebApkIntent(WEBAPK_PACKAGE_NAME, START_URL);
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create();
Intent launchIntent = getNextStartedActivity();
assertEquals(WebappActivity.class.getName(), launchIntent.getComponent().getClassName());
// WebAPK package name should be part of the intent URI to enable launching multiple
// WebAPKs.
assertEquals("webapp://webapk-" + WEBAPK_PACKAGE_NAME, launchIntent.getDataString());
assertTrue((launchIntent.getFlags() & Intent.FLAG_ACTIVITY_NEW_TASK) != 0);
assertNotNull(WebApkIntentDataProviderFactory.create(launchIntent));
}
/**
* Test the launch intent created by {@link WebappLauncherActivity} for new-style WebAPKs.
*/
@Test
public void testNewStyleLaunchIntent() {
registerWebApk(WEBAPK_PACKAGE_NAME, START_URL);
Intent intent = WebApkTestHelper.createMinimalWebApkIntent(WEBAPK_PACKAGE_NAME, START_URL);
intent.putExtra(WebApkConstants.EXTRA_SPLASH_PROVIDED_BY_WEBAPK, true);
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create();
Intent launchIntent = getNextStartedActivity();
assertEquals(
SameTaskWebApkActivity.class.getName(), launchIntent.getComponent().getClassName());
assertEquals(launchIntent.getFlags() & Intent.FLAG_ACTIVITY_NEW_TASK, 0);
assertNotNull(WebApkIntentDataProviderFactory.create(launchIntent));
}
private void registerWebApk(String webApkPackage, String startUrl) {
Bundle bundle = new Bundle(); Bundle bundle = new Bundle();
bundle.putString(WebApkMetaDataKeys.START_URL, START_URL); bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
WebApkTestHelper.registerWebApkWithMetaData( WebApkTestHelper.registerWebApkWithMetaData(
WEBAPK_PACKAGE_NAME, bundle, null /* shareTargetMetaData */); webApkPackage, bundle, null /* shareTargetMetaData */);
WebApkTestHelper.addIntentFilterForUrl(webApkPackage, startUrl);
Intent intent = new Intent(); }
intent.putExtra(WebApkConstants.EXTRA_WEBAPK_PACKAGE_NAME, WEBAPK_PACKAGE_NAME);
intent.putExtra(ShortcutHelper.EXTRA_URL, START_URL);
Assert.assertNotNull(WebApkIntentDataProviderFactory.create(intent)); private Intent getNextStartedActivity() {
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create(); return ShadowApplication.getInstance().getNextStartedActivity();
Assert.assertNull(WebApkIntentDataProviderFactory.create(intent));
} }
} }
...@@ -49,7 +49,7 @@ public class MapsGoFirstRunTest { ...@@ -49,7 +49,7 @@ public class MapsGoFirstRunTest {
@Before @Before
public void setUp() { public void setUp() {
WebApkValidator.disableValidationForTesting(); WebApkValidator.setDisableValidationForTesting(true);
TestThreadUtils.runOnUiThreadBlocking(WebappRegistry::refreshSharedPrefsForTesting); TestThreadUtils.runOnUiThreadBlocking(WebappRegistry::refreshSharedPrefsForTesting);
} }
......
...@@ -407,11 +407,11 @@ public class WebApkValidator { ...@@ -407,11 +407,11 @@ public class WebApkValidator {
} }
/** /**
* Disables all validation performed by this class. This is meant only for development with * Sets whether validation performed by this class should be disabled. This is meant only for
* unsigned WebApks and should never be enabled in a real build. * development with unsigned WebApks and should never be enabled in a real build.
*/ */
public static void disableValidationForTesting() { public static void setDisableValidationForTesting(boolean disable) {
sOverrideValidationForTesting = true; sOverrideValidationForTesting = disable;
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment