Commit 95a23eca authored by Peter Kotwicz's avatar Peter Kotwicz Committed by Commit Bot

[Android WebAPK] Fix First Run showing twice for Maps Go

This CL fixes a bug introduced by
http://crrev.com/95ad83cedf394b9b37a8b3ab151116918b8f64ec where the
first run experience would show twice for Maps Go -> the lightweight
FRE launched by WebappLauncherActivity and the full FRE launched by
AsyncInitializationActivity. This CL is a partial revert of the CL.

This CL is simple so that it can be potentially merged back. The
"proper" fix is at
https://chromium-review.googlesource.com/c/chromium/src/+/1478460

BUG=932505
TEST=FirstRunIntegrationUnitTest.testUserAcceptedLightweightFreLaunch


Change-Id: Ic42adfe40eeaf03349ff838d52ee1e99c59f866e
Reviewed-on: https://chromium-review.googlesource.com/c/1476321Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634895}
parent d0c9f5d6
...@@ -302,7 +302,7 @@ public abstract class AsyncInitializationActivity ...@@ -302,7 +302,7 @@ public abstract class AsyncInitializationActivity
if (requiresFirstRunToBeCompleted(intent) if (requiresFirstRunToBeCompleted(intent)
&& FirstRunFlowSequencer.launch(this, intent, false /* requiresBroadcast */, && FirstRunFlowSequencer.launch(this, intent, false /* requiresBroadcast */,
false /* preferLightweightFre */)) { shouldPreferLightweightFre(intent))) {
abortLaunch(LaunchIntentDispatcher.Action.FINISH_ACTIVITY_REMOVE_TASK); abortLaunch(LaunchIntentDispatcher.Action.FINISH_ACTIVITY_REMOVE_TASK);
return; return;
} }
...@@ -400,6 +400,14 @@ public abstract class AsyncInitializationActivity ...@@ -400,6 +400,14 @@ public abstract class AsyncInitializationActivity
return true; return true;
} }
/**
* Whether to use the Lightweight First Run Experience instead of the
* non-Lightweight First Run Experience.
*/
protected boolean shouldPreferLightweightFre(Intent intent) {
return false;
}
/** /**
* Whether or not the Activity was started up via a valid Intent. * Whether or not the Activity was started up via a valid Intent.
*/ */
......
...@@ -14,6 +14,8 @@ import org.chromium.base.metrics.RecordHistogram; ...@@ -14,6 +14,8 @@ import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.metrics.WebApkSplashscreenMetrics; import org.chromium.chrome.browser.metrics.WebApkSplashscreenMetrics;
import org.chromium.chrome.browser.metrics.WebApkUma; import org.chromium.chrome.browser.metrics.WebApkUma;
import org.chromium.chrome.browser.util.IntentUtils;
import org.chromium.webapk.lib.common.WebApkConstants;
/** /**
* An Activity is designed for WebAPKs (native Android apps) and displays a webapp in a nearly * An Activity is designed for WebAPKs (native Android apps) and displays a webapp in a nearly
...@@ -49,6 +51,18 @@ public class WebApkActivity extends WebappActivity { ...@@ -49,6 +51,18 @@ public class WebApkActivity extends WebappActivity {
getActivityTab().setWebappManifestScope(getWebappInfo().scopeUri().toString()); getActivityTab().setWebappManifestScope(getWebappInfo().scopeUri().toString());
} }
@Override
public boolean shouldPreferLightweightFre(Intent intent) {
// We cannot use getWebApkPackageName() because {@link WebappActivity#preInflationStartup()}
// may not have been called yet.
String webApkPackageName =
IntentUtils.safeGetStringExtra(intent, WebApkConstants.EXTRA_WEBAPK_PACKAGE_NAME);
// Use the lightweight FRE for unbound WebAPKs.
return webApkPackageName != null
&& !webApkPackageName.startsWith(WebApkConstants.WEBAPK_PACKAGE_PREFIX);
}
@Override @Override
public String getWebApkPackageName() { public String getWebApkPackageName() {
return getWebappInfo().webApkPackageName(); return getWebappInfo().webApkPackageName();
......
...@@ -25,13 +25,18 @@ import org.robolectric.Robolectric; ...@@ -25,13 +25,18 @@ import org.robolectric.Robolectric;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows; import org.robolectric.Shadows;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.shadows.ShadowApplication; import org.robolectric.shadows.ShadowApplication;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.ShortcutHelper; import org.chromium.chrome.browser.ShortcutHelper;
import org.chromium.chrome.browser.document.ChromeLauncherActivity; import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.init.BrowserParts;
import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
import org.chromium.chrome.browser.searchwidget.SearchActivity; import org.chromium.chrome.browser.searchwidget.SearchActivity;
import org.chromium.chrome.browser.webapps.WebApkActivity;
import org.chromium.chrome.browser.webapps.WebappLauncherActivity; import org.chromium.chrome.browser.webapps.WebappLauncherActivity;
import org.chromium.webapk.lib.client.WebApkValidator; import org.chromium.webapk.lib.client.WebApkValidator;
import org.chromium.webapk.lib.common.WebApkConstants; import org.chromium.webapk.lib.common.WebApkConstants;
...@@ -40,8 +45,19 @@ import org.chromium.webapk.test.WebApkTestHelper; ...@@ -40,8 +45,19 @@ import org.chromium.webapk.test.WebApkTestHelper;
/** JUnit tests for first run triggering code. */ /** JUnit tests for first run triggering code. */
@RunWith(BaseRobolectricTestRunner.class) @RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE) @Config(manifest = Config.NONE,
shadows = {FirstRunIntegrationUnitTest.MockChromeBrowserInitializer.class})
public final class FirstRunIntegrationUnitTest { public final class FirstRunIntegrationUnitTest {
/** Do nothing version of {@link ChromeBrowserInitializer}. */
@Implements(ChromeBrowserInitializer.class)
public static class MockChromeBrowserInitializer {
@Implementation
public void __constructor__() {}
@Implementation
public void handlePreNativeStartup(final BrowserParts parts) {}
}
@Rule @Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule(); public MockitoRule mMockitoRule = MockitoJUnit.rule();
...@@ -61,6 +77,11 @@ public final class FirstRunIntegrationUnitTest { ...@@ -61,6 +77,11 @@ public final class FirstRunIntegrationUnitTest {
WebApkValidator.disableValidationForTesting(); WebApkValidator.disableValidationForTesting();
} }
/** Checks that the intent component targets the passed-in class. */
private boolean checkIntentComponentClass(Intent intent, Class componentClass) {
return checkIntentComponentClassOneOf(intent, new Class[] {componentClass});
}
/** Checks that the intent component is one of the provided classes. */ /** Checks that the intent component is one of the provided classes. */
private boolean checkIntentComponentClassOneOf(Intent intent, Class[] componentClassOptions) { private boolean checkIntentComponentClassOneOf(Intent intent, Class[] componentClassOptions) {
if (intent == null || intent.getComponent() == null) return false; if (intent == null || intent.getComponent() == null) return false;
...@@ -178,8 +199,7 @@ public final class FirstRunIntegrationUnitTest { ...@@ -178,8 +199,7 @@ public final class FirstRunIntegrationUnitTest {
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create(); Robolectric.buildActivity(WebappLauncherActivity.class, intent).create();
Intent launchedIntent = mShadowApplication.getNextStartedActivity(); Intent launchedIntent = mShadowApplication.getNextStartedActivity();
while (checkIntentComponentClassOneOf( while (checkIntentComponentClass(launchedIntent, WebappLauncherActivity.class)) {
launchedIntent, new Class[] {WebappLauncherActivity.class})) {
buildActivityWithClassNameFromIntent(launchedIntent); buildActivityWithClassNameFromIntent(launchedIntent);
launchedIntent = mShadowApplication.getNextStartedActivity(); launchedIntent = mShadowApplication.getNextStartedActivity();
} }
...@@ -191,4 +211,35 @@ public final class FirstRunIntegrationUnitTest { ...@@ -191,4 +211,35 @@ public final class FirstRunIntegrationUnitTest {
Assert.assertEquals(webApkPackageName, Assert.assertEquals(webApkPackageName,
Shadows.shadowOf(freCompleteLaunchIntent).getSavedIntent().getPackage()); Shadows.shadowOf(freCompleteLaunchIntent).getSavedIntent().getPackage());
} }
/**
* Test that if a WebAPK only requires the lightweight FRE and a user has gone through the
* lightweight FRE that the WebAPK launches and no FRE is shown to the user.
*/
@Test
public void testUserAcceptedLightweightFreLaunch() {
FirstRunStatus.setLightweightFirstRunFlowComplete(true);
String webApkPackageName = "unbound.webapk";
String startUrl = "https://pwa.rocks/";
Bundle bundle = new Bundle();
bundle.putString(WebApkMetaDataKeys.START_URL, startUrl);
WebApkTestHelper.registerWebApkWithMetaData(
webApkPackageName, bundle, null /* shareTargetMetaData */);
WebApkTestHelper.addIntentFilterForUrl(webApkPackageName, startUrl);
Intent intent = new Intent();
intent.putExtra(WebApkConstants.EXTRA_WEBAPK_PACKAGE_NAME, webApkPackageName);
intent.putExtra(ShortcutHelper.EXTRA_URL, startUrl);
Robolectric.buildActivity(WebappLauncherActivity.class, intent).create();
Intent launchedIntent = mShadowApplication.getNextStartedActivity();
Assert.assertTrue(checkIntentComponentClass(launchedIntent, WebApkActivity.class));
buildActivityWithClassNameFromIntent(launchedIntent);
// No FRE should have been launched.
Assert.assertNull(mShadowApplication.getNextStartedActivity());
}
} }
...@@ -73,6 +73,7 @@ ...@@ -73,6 +73,7 @@
<message key="name.invalidPattern" value="Static field names start with s."/> <message key="name.invalidPattern" value="Static field names start with s."/>
</module> </module>
<module name="MethodName"> <module name="MethodName">
<property name="id" value="MethodNameCheck"/>
<property name="severity" value="error"/> <property name="severity" value="error"/>
<property name="format" value="^[a-z][a-zA-Z0-9_]*$"/> <property name="format" value="^[a-z][a-zA-Z0-9_]*$"/>
<message key="name.invalidPattern" value="Method names should start with a lower case letter (e.g. getWidth())"/> <message key="name.invalidPattern" value="Method names should start with a lower case letter (e.g. getWidth())"/>
......
...@@ -8,4 +8,6 @@ ...@@ -8,4 +8,6 @@
<suppress id="AlertDialogCheck" <suppress id="AlertDialogCheck"
files="src/(android_webview|base|build|chromecast|components|content|device|media|mojo|net|printing|services|testing|third_party|tools|ui|url)/"/> files="src/(android_webview|base|build|chromecast|components|content|device|media|mojo|net|printing|services|testing|third_party|tools|ui|url)/"/>
<suppress id="StringBufferCheck" files="src/chrome/android/webapk/"/> <suppress id="StringBufferCheck" files="src/chrome/android/webapk/"/>
<!-- Robolectric shadows can overwrite constructor by implementing __constructor__() method. -->
<suppress id="MethodNameCheck" files="FirstRunIntegrationUnitTest.java"/>
</suppressions> </suppressions>
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