[Android WebAPKs] Don't navigate WebAPK when launching it from launcher

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
index 9b95ea93e5db259d8c021c9ecbd57b6cbbeb0312..11c365c4e830acdeb332933f3023ee5994da7173 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
@@ -31,6 +31,7 @@ import java.util.Map;
 public class WebApkInfo extends WebappInfo {
     private static final String TAG = "WebApkInfo";
 
+    private boolean mShouldLoadUrlIfAlreadyRunning;
     private String mWebApkPackageName;
     private int mShellApkVersion;
     private String mManifestUrl;
@@ -110,7 +111,9 @@ public class WebApkInfo extends WebappInfo {
      * Construct a {@link WebApkInfo} instance.
      *
      * @param id                      ID for the WebAPK.
-     * @param url                     URL that the WebAPK should navigate to when launched.
+     * @param url                     URL that the WebAPK should navigate to when launched. A null
+     *                                URL should be passed if the WebAPK should stay at the current
+     *                                URL if it is already open.
      * @param scope                   Scope for the WebAPK.
      * @param icon                    Icon to show for the WebAPK.
      * @param name                    Name of the WebAPK.
@@ -133,9 +136,9 @@ public class WebApkInfo extends WebappInfo {
             String shortName, int displayMode, int orientation, int source, long themeColor,
             long backgroundColor, String webApkPackageName, int shellApkVersion, String manifestUrl,
             String manifestStartUrl, Map<String, String> iconUrlToMurmur2HashMap) {
-        if (id == null || url == null || manifestStartUrl == null || webApkPackageName == null) {
+        if (id == null || manifestStartUrl == null || webApkPackageName == null) {
             Log.e(TAG,
-                    "Incomplete data provided: " + id + ", " + url + ", " + manifestStartUrl + ", "
+                    "Incomplete data provided: " + id + ", " + manifestStartUrl + ", "
                             + webApkPackageName);
             return null;
         }
@@ -147,17 +150,25 @@ public class WebApkInfo extends WebappInfo {
             scope = ShortcutHelper.getScopeFromUrl(manifestStartUrl);
         }
 
-        return new WebApkInfo(id, url, scope, icon, name, shortName, displayMode,
-                orientation, source, themeColor, backgroundColor, webApkPackageName,
+        boolean shouldLoadUrlIfAlreadyRunning = true;

[dominickn, 2017/03/20 04:00:09]

The null url parameter + this new boolean is really magic and feels very
fragile.

This may be a naive question, but can't we just store a flag in
WebApkActivity#onNewIntent() indicating we've already loaded a URL and are
already running (hence don't load anything)? Or can we just check the currently
loaded URL and the incoming URL and if they're the same we no-op? That's the
main use case here right? Or do we still want to allow EXTRA_URL to be set and
respected in some cases for WebAPKs?

Otherwise, I really don't like this implementation - there are new special cases
everywhere, and it feels like something that's easy to get wrong when adding new
intents into WebAPKs.

[pkotwicz, 2017/03/20 22:58:37]

There are two situations where we hit WebApkActivity#onNewIntent()

Situation #1:
  Launching WebAPK from launcher. In this case, we don't want to navigate the
WebAPK if it is already open
Situation #2:
  Launching WebAPK from a deep link. In this case, we want to navigate the
WebAPK even if it is already open

I have changed the CL to make MainActivity.java add an extra on the intent to
indicate whether the WebAPK should navigate if it is already open 

+        if (TextUtils.isEmpty(url)) {
+            url = manifestStartUrl;
+            shouldLoadUrlIfAlreadyRunning = false;
+        }
+
+        return new WebApkInfo(id, url, shouldLoadUrlIfAlreadyRunning, scope, icon, name, shortName,
+                displayMode, orientation, source, themeColor, backgroundColor, webApkPackageName,
                 shellApkVersion, manifestUrl, manifestStartUrl, iconUrlToMurmur2HashMap);
     }
 
-    protected WebApkInfo(String id, String url, String scope, Icon icon, String name,
-            String shortName, int displayMode, int orientation, int source, long themeColor,
-            long backgroundColor, String webApkPackageName, int shellApkVersion, String manifestUrl,
-            String manifestStartUrl, Map<String, String> iconUrlToMurmur2HashMap) {
+    protected WebApkInfo(String id, String url, boolean shouldLoadUrlIfAlreadyRunning, String scope,
+            Icon icon, String name, String shortName, int displayMode, int orientation, int source,
+            long themeColor, long backgroundColor, String webApkPackageName, int shellApkVersion,
+            String manifestUrl, String manifestStartUrl,
+            Map<String, String> iconUrlToMurmur2HashMap) {
         super(id, url, scope, icon, name, shortName, displayMode, orientation, source, themeColor,
                 backgroundColor, false);
+        mShouldLoadUrlIfAlreadyRunning = shouldLoadUrlIfAlreadyRunning;
         mWebApkPackageName = webApkPackageName;
         mShellApkVersion = shellApkVersion;
         mManifestUrl = manifestUrl;
@@ -168,6 +179,11 @@ public class WebApkInfo extends WebappInfo {
     protected WebApkInfo() {}
 
     @Override
+    public boolean shouldLoadUrlIfAlreadyRunning() {
+        return mShouldLoadUrlIfAlreadyRunning;
+    }
+
+    @Override
     public String webApkPackageName() {
         return mWebApkPackageName;
     }
@@ -191,7 +207,9 @@ public class WebApkInfo extends WebappInfo {
     @Override
     public void setWebappIntentExtras(Intent intent) {
         // For launching a {@link WebApkActivity}.
-        intent.putExtra(ShortcutHelper.EXTRA_URL, uri().toString());
+        if (shouldLoadUrlIfAlreadyRunning()) {
+            intent.putExtra(ShortcutHelper.EXTRA_URL, uri().toString());
+        }
         intent.putExtra(ShortcutHelper.EXTRA_SOURCE, source());
         intent.putExtra(ShortcutHelper.EXTRA_WEBAPK_PACKAGE_NAME, webApkPackageName());
     }