Refactor the INSTALL_SHORTCUT broadcast code into ChromeShortcutManager

chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
index c8356b73ffd55880f02a18bf3581ac97bab3d0c8..e0bab6650f79d84e99d065bb86653203f31128bc 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser;
 
-import android.annotation.SuppressLint;
 import android.app.ActivityManager;
 import android.content.Context;
 import android.content.Intent;
@@ -12,7 +11,6 @@ import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
-import android.content.pm.ResolveInfo;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.graphics.Canvas;
@@ -35,6 +33,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.blink_public.platform.WebDisplayMode;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.webapps.ChromeShortcutManager;
 import org.chromium.chrome.browser.webapps.ChromeWebApkHost;
 import org.chromium.chrome.browser.webapps.WebApkInfo;
 import org.chromium.chrome.browser.webapps.WebappActivity;
@@ -89,11 +88,11 @@ public class ShortcutHelper {
     // This value is equal to kInvalidOrMissingColor in the C++ content::Manifest struct.
     public static final long MANIFEST_COLOR_INVALID_OR_MISSING = ((long) Integer.MAX_VALUE) + 1;
 
-    private static final String TAG = "ShortcutHelper";
+    // This intent category is for the callback intent when using the new shortcut API

[gone, 2017/02/15 19:40:18]

1) end comment with a period.

2) fix comment up on line 62 so it also ends with a period.

3) Should really be a one liner javadoc so it shows up in the IDE:
   /** Used for the callback Intent when using the new shortcut API. */

4) Stick this with the other Intent-related strings up top (under
EXTRA_WEBAPK_PACKAGE_NAME)

[Marti Wong, 2017/02/17 05:10:49]

Done.

+    public static final String SHORTCUT_TOAST_CATEGORY =
+            "com.google.intent.category.SHORTCUT_TOAST";
 
-    // There is no public string defining this intent so if Home changes the value, we
-    // have to update this string.
-    private static final String INSTALL_SHORTCUT = "com.android.launcher.action.INSTALL_SHORTCUT";
+    private static final String TAG = "ShortcutHelper";
 
     // The activity class used for launching a WebApk.
     private static final String WEBAPK_MAIN_ACTIVITY = "org.chromium.webapk.shell_apk.MainActivity";
@@ -106,15 +105,17 @@ public class ShortcutHelper {
     private static final float GENERATED_ICON_PADDING_RATIO = 1.0f / 12.0f;
     private static final float GENERATED_ICON_FONT_SIZE_RATIO = 1.0f / 3.0f;
 
-    /** Broadcasts Intents out Android for adding the shortcut. */
+    /** Request Android to add a shortcut to the home screen. */

[gone, 2017/02/15 19:40:19]

This javadoc sounds more like a function comment and applies only to the top
function.  Maybe something totally generic like:

/** Helper for generating home screen shortcuts. */

[Marti Wong, 2017/02/17 05:10:48]

Done.

     public static class Delegate {
         /**
-         * Broadcasts an intent to all interested BroadcastReceivers.
-         * @param context The Context to use.
-         * @param intent The intent to broadcast.
+         * Request Android to add a shortcut to the home screen
+         * @param title Title of the shortcut.
+         * @param icon Image that represents the shortcut.
+         * @param intent Intent to fire when the shortcut is activated.

[gone, 2017/02/15 19:40:18]

nit: Line up the @params to match rest of file?

@param title  Title of the shortcut.
@param icon   Image that represents the shortcut.
@param intent Intent to fire when the shortcut is activated.

[Marti Wong, 2017/02/17 05:10:49]

Done.

          */
-        public void sendBroadcast(Context context, Intent intent) {
-            context.sendBroadcast(intent);
+        public void addShortcutToHomescreen(String title, Bitmap icon, Intent shortcutIntent) {
+            ChromeShortcutManager.getInstance().addShortcutToHomeScreen(
+                    title, icon, shortcutIntent);
         }
 
         /**
@@ -165,9 +166,7 @@ public class ShortcutHelper {
             }
             @Override
             protected void onPostExecute(final Intent resultIntent) {
-                Context context = ContextUtils.getApplicationContext();
-                sDelegate.sendBroadcast(
-                        context, createAddToHomeIntent(userTitle, icon, resultIntent));
+                sDelegate.addShortcutToHomescreen(userTitle, icon, resultIntent);
 
                 // Store the webapp data so that it is accessible without the intent. Once this
                 // process is complete, call back to native code to start the splash image
@@ -180,8 +179,9 @@ public class ShortcutHelper {
                                 nativeOnWebappDataStored(callbackPointer);
                             }
                         });
-
-                showAddedToHomescreenToast(userTitle);
+                if (ChromeShortcutManager.getInstance().shouldShowToastWhenAddingShortcut()) {
+                    showAddedToHomescreenToast(userTitle);
+                }
             }
         }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }
@@ -198,7 +198,8 @@ public class ShortcutHelper {
             Intent i = new Intent();
             i.setClassName(packageName, WEBAPK_MAIN_ACTIVITY);
             i.addCategory(Intent.CATEGORY_LAUNCHER);
-            context.sendBroadcast(createAddToHomeIntent(shortcutTitle, bitmap, i));
+            context.sendBroadcast(
+                    ChromeShortcutManager.createAddToHomeIntent(shortcutTitle, bitmap, i));
         } catch (NameNotFoundException e) {
             e.printStackTrace();
         }
@@ -209,14 +210,17 @@ public class ShortcutHelper {
      */
     @SuppressWarnings("unused")
     @CalledByNative
-    private static void addShortcut(String url, String userTitle, Bitmap icon, int source) {
+    private static void addShortcut(
+            String id, String url, String userTitle, Bitmap icon, int source) {
         Context context = ContextUtils.getApplicationContext();
         final Intent shortcutIntent = createShortcutIntent(url);
+        shortcutIntent.putExtra(EXTRA_ID, id);
         shortcutIntent.putExtra(EXTRA_SOURCE, source);
         shortcutIntent.setPackage(context.getPackageName());
-        sDelegate.sendBroadcast(
-                context, createAddToHomeIntent(userTitle, icon, shortcutIntent));
-        showAddedToHomescreenToast(userTitle);
+        sDelegate.addShortcutToHomescreen(userTitle, icon, shortcutIntent);
+        if (ChromeShortcutManager.getInstance().shouldShowToastWhenAddingShortcut()) {
+            showAddedToHomescreenToast(userTitle);
+        }
     }
 
     /**
@@ -229,6 +233,24 @@ public class ShortcutHelper {
     }
 
     /**
+     * Show toast when getting the callback intent by the launcher after adding shortcut by using
+     * the new shortcut API

[gone, 2017/02/15 19:40:18]

End all your comments with periods

[Marti Wong, 2017/02/17 05:10:48]

Done.

+     */
+    public static void showAddedToHomescreenToastFromIntent(Intent intent) {
+        String title = intent.getStringExtra(Intent.EXTRA_SHORTCUT_NAME);

[gone, 2017/02/15 19:40:19]

1) Never trust an Intent; use IntentUtils#safeGetStringExtra
2) Should this function be doing the if (shouldShowToast) check instead of the
callers?  You can append "IfNeeded" or something to the function name.

[Marti Wong, 2017/02/17 05:10:48]

1) Done, thanks~!
2) this function should just show toast without checking.  It is called when a
callback intent is requesting a show toast so there is no question of checking.

+        showAddedToHomescreenToast(title);
+    }
+
+    /**
+     * Determine if it is a callback intent (which requests for a show-toast), used in the new
+     * shortcut API
+     */
+    public static boolean isShowtoastIntent(Intent intent) {

[gone, 2017/02/15 19:40:18]

nit: capitalize Toast

[Marti Wong, 2017/02/17 05:10:48]

Done.

+        if (intent == null || intent.getCategories() == null) return false;
+        return intent.getCategories().contains(SHORTCUT_TOAST_CATEGORY);
+    }
+
+    /**
      * Shows toast notifying user that a WebAPK install is already in progress when user tries to
      * queue a new install for the same WebAPK.
      */
@@ -273,22 +295,6 @@ public class ShortcutHelper {
     }
 
     /**
-     * Creates an intent that will add a shortcut to the home screen.
-     * @param title Title of the shortcut.
-     * @param icon Image that represents the shortcut.
-     * @param shortcutIntent Intent to fire when the shortcut is activated.
-     * @return Intent for the shortcut.
-     */
-    public static Intent createAddToHomeIntent(String title, Bitmap icon,
-            Intent shortcutIntent) {
-        Intent i = new Intent(INSTALL_SHORTCUT);
-        i.putExtra(Intent.EXTRA_SHORTCUT_INTENT, shortcutIntent);
-        i.putExtra(Intent.EXTRA_SHORTCUT_NAME, title);
-        i.putExtra(Intent.EXTRA_SHORTCUT_ICON, icon);
-        return i;
-    }
-
-    /**
      * Creates a shortcut to launch a web app on the home screen.
      * @param id              Id of the web app.
      * @param action          Intent action to open a full screen activity.
@@ -361,14 +367,8 @@ public class ShortcutHelper {
      * @param context Context used to get the package manager.
      * @return if a shortcut can be added to the home screen under the current profile.
      */
-    // TODO(crbug.com/635567): Fix this properly.
-    @SuppressLint("WrongConstant")
     public static boolean isAddToHomeIntentSupported(Context context) {

[gone, 2017/02/15 19:40:18]

This doesn't use the context anymore.  Might as well remove it.

[Marti Wong, 2017/02/17 05:10:48]

Done.  thanks.  Sorry for this careless mistake.

[gone, 2017/02/17 19:41:43]

Pfft, nothing to apologize for.

-        PackageManager pm = context.getPackageManager();
-        Intent i = new Intent(INSTALL_SHORTCUT);
-        List<ResolveInfo> receivers = pm.queryBroadcastReceivers(
-                i, PackageManager.GET_INTENT_FILTERS);
-        return !receivers.isEmpty();
+        return ChromeShortcutManager.getInstance().canAddShortcutToHomescreen();
     }
 
     /**