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

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.webapps;
 
 import android.content.Intent;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.res.Resources;
@@ skipping 13 matching lines @@
 
 import java.util.HashMap;
 import java.util.Map;
 
 /**
  * Stores info for WebAPK.
  */
 public class WebApkInfo extends WebappInfo {
     private static final String TAG = "WebApkInfo";
 
+    private boolean mShouldLoadUrlIfAlreadyRunning;
     private String mWebApkPackageName;
     private int mShellApkVersion;
     private String mManifestUrl;
     private String mManifestStartUrl;
     private Map<String, String> mIconUrlToMurmur2HashMap;
 
     public static WebApkInfo createEmpty() {
         return new WebApkInfo();
     }
 
@@ skipping 59 matching lines @@
         return create(WebApkConstants.WEBAPK_ID_PREFIX + webApkPackageName, url, scope,
                 new Icon(icon), name, shortName, displayMode, orientation, source, themeColor,
                 backgroundColor, webApkPackageName, shellApkVersion, manifestUrl, manifestStartUrl,
                 iconUrlToMurmur2HashMap);
     }
 
     /**
      * 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.
      * @param shortName               The short name of the WebAPK.
      * @param displayMode             Display mode of the WebAPK.
      * @param orientation             Orientation of the WebAPK.
      * @param source                  Source that the WebAPK was launched from.
      * @param themeColor              The theme color of the WebAPK.
      * @param backgroundColor         The background color of the WebAPK.
      * @param webApkPackageName       The package of the WebAPK.
      * @param shellApkVersion         Version of the code in //chrome/android/webapk/shell_apk.
      * @param manifestUrl             URL of the Web Manifest.
      * @param manifestStartUrl        URL that the WebAPK should navigate to when launched from the
      *                                homescreen. Different from the {@link url} parameter if the
      *                                WebAPK is launched from a deep link.
      * @param iconUrlToMurmur2HashMap Map of the WebAPK's icon URLs to Murmur2 hashes of the
      *                                icon untransformed bytes.
      */
     public static WebApkInfo create(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) {
-        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;
         }
 
         // The default scope should be computed from the Web Manifest start URL. If the WebAPK was
         // launched from a deep link {@link startUrl} may be different from the Web Manifest start
         // URL.
         if (TextUtils.isEmpty(scope)) {
             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;
         mManifestStartUrl = manifestStartUrl;
         mIconUrlToMurmur2HashMap = iconUrlToMurmur2HashMap;
     }
 
     protected WebApkInfo() {}
 
     @Override
+    public boolean shouldLoadUrlIfAlreadyRunning() {
+        return mShouldLoadUrlIfAlreadyRunning;
+    }
+
+    @Override
     public String webApkPackageName() {
         return mWebApkPackageName;
     }
 
     public int shellApkVersion() {
         return mShellApkVersion;
     }
 
     public String manifestUrl() {
         return mManifestUrl;
     }
 
     public String manifestStartUrl() {
         return mManifestStartUrl;
     }
 
     public Map<String, String> iconUrlToMurmur2HashMap() {
         return mIconUrlToMurmur2HashMap;
     }
 
     @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());
     }
 
     /**
      * Extracts meta data from a WebAPK's Android Manifest.
      * @param webApkPackageName WebAPK's package name.
      * @return Bundle with the extracted meta data.
      */
     private static Bundle extractWebApkMetaData(String webApkPackageName) {
@@ skipping 145 matching lines @@
             return ScreenOrientationValues.PORTRAIT;
         } else if (orientation.equals("portrait-primary")) {
             return ScreenOrientationValues.PORTRAIT_PRIMARY;
         } else if (orientation.equals("portrait-secondary")) {
             return ScreenOrientationValues.PORTRAIT_SECONDARY;
         } else {
             return ScreenOrientationValues.DEFAULT;
         }
     }
 }