Run ShortcutHelper::AddToLauncherInBackgroundWithSkBitmap() on Worker thread instead of IO thread

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 936279dd595f5887c23ef20d07d17be46255304f..2e514228a10900afb18a907ad079c3eee086747e 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java
@@ -22,6 +22,7 @@ import android.graphics.RectF;
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
+import android.os.AsyncTask;
 import android.text.TextUtils;
 import android.util.Base64;
 
@@ -130,54 +131,59 @@ public class ShortcutHelper {
 
     /**
      * Adds home screen shortcut which opens in a {@link WebappActivity}.

[dominickn, 2016/08/10 05:18:10]

Nit: add a comment that this runs the procedure on a worker thread, and runs
OnWebappDataStored on the callbackPointer when it's finished.

-     * This method must not be called on the UI thread.
      */
     @SuppressWarnings("unused")
     @CalledByNative
-    private static void addWebapp(String id, String url, String scopeUrl, final String userTitle,
-            String name, String shortName, String iconUrl, Bitmap icon, int displayMode,
-            int orientation, int source, long themeColor, long backgroundColor,
-            final long callbackPointer) {
-        assert !ThreadUtils.runningOnUiThread();
-
-        Context context = ContextUtils.getApplicationContext();
-        if (TextUtils.isEmpty(scopeUrl)) {
-            scopeUrl = getScopeFromUrl(url);
-        }
+    private static void addWebapp(final String id, final String url, final String scopeUrl,
+            final String userTitle, final String name, final String shortName, final String iconUrl,
+            final Bitmap icon, final int displayMode, final int orientation, final int source,
+            final long themeColor, final long backgroundColor, final long callbackPointer) {
+        assert ThreadUtils.runningOnUiThread();
 
-        final Intent shortcutIntent =
-                createWebappShortcutIntent(id, sDelegate.getFullscreenAction(), url, scopeUrl, name,
-                        shortName, icon, WEBAPP_SHORTCUT_VERSION, displayMode, orientation,
-                        themeColor, backgroundColor, iconUrl.isEmpty());
-        shortcutIntent.putExtra(EXTRA_MAC, getEncodedMac(context, url));
-        shortcutIntent.putExtra(EXTRA_SOURCE, source);
-        shortcutIntent.setPackage(context.getPackageName());
-        sDelegate.sendBroadcast(
-                context, createAddToHomeIntent(url, userTitle, icon, shortcutIntent));
-
-        // 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 download.
-        WebappRegistry.registerWebapp(context, id,
-                new WebappRegistry.FetchWebappDataStorageCallback() {
-                    @Override
-                    public void onWebappDataStorageRetrieved(WebappDataStorage storage) {
-                        storage.updateFromShortcutIntent(shortcutIntent);
-                        nativeOnWebappDataStored(callbackPointer);
-                    }
-                });
-
-        showAddedToHomescreenToast(userTitle);
+        new AsyncTask<Void, Void, Intent>() {
+            @Override
+            protected Intent doInBackground(Void... args0) {
+                Context context = ContextUtils.getApplicationContext();
+                String nonEmptyScopeUrl =
+                        TextUtils.isEmpty(scopeUrl) ? getScopeFromUrl(url) : scopeUrl;
+                Intent shortcutIntent = createWebappShortcutIntent(id,
+                        sDelegate.getFullscreenAction(), url, nonEmptyScopeUrl, name, shortName,
+                        icon, WEBAPP_SHORTCUT_VERSION, displayMode, orientation, themeColor,
+                        backgroundColor, iconUrl.isEmpty());
+                shortcutIntent.putExtra(EXTRA_MAC, getEncodedMac(context, url));
+                shortcutIntent.putExtra(EXTRA_SOURCE, source);
+                shortcutIntent.setPackage(context.getPackageName());
+                return shortcutIntent;
+            }
+            @Override
+            protected void onPostExecute(final Intent resultIntent) {
+                Context context = ContextUtils.getApplicationContext();
+                sDelegate.sendBroadcast(
+                        context, createAddToHomeIntent(url, 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
+                // download.
+                WebappRegistry.registerWebapp(
+                        context, id, new WebappRegistry.FetchWebappDataStorageCallback() {
+                            @Override
+                            public void onWebappDataStorageRetrieved(WebappDataStorage storage) {
+                                storage.updateFromShortcutIntent(resultIntent);
+                                nativeOnWebappDataStored(callbackPointer);
+                            }
+                        });
+
+                showAddedToHomescreenToast(userTitle);
+            }
+        }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);

[dominickn, 2016/08/10 05:18:10]

Does this really need to be parallelisable? Just execute() is probably okay
here.

[pkotwicz, 2016/08/10 20:28:43]

Adding agrieve@. My understanding is that we should always use
AsyncTask.THREAD_POOL_EXECUTOR now except if there is a good reason not ot

[agrieve, 2016/08/11 00:42:13]

There are certainly trade-offs in either direction, but until we get something
better, I think THREAD_POOL_EXECUTOR is a good default.

.execute() should never be used since it maps to different executors on
different android versions.

     }
 
     /**
      * Adds home screen shortcut which opens in the browser Activity.
-     * This method must not be called on the UI thread.
      */
     @SuppressWarnings("unused")
     @CalledByNative
     private static void addShortcut(String url, String userTitle, Bitmap icon, int source) {
-        assert !ThreadUtils.runningOnUiThread();
-
         Context context = ContextUtils.getApplicationContext();
         final Intent shortcutIntent = createShortcutIntent(url);
         shortcutIntent.putExtra(EXTRA_SOURCE, source);
@@ -191,16 +197,10 @@ public class ShortcutHelper {
      * Show toast to alert user that the shortcut was added to the home screen.
      */
     private static void showAddedToHomescreenToast(final String title) {
-        ThreadUtils.runOnUiThread(new Runnable() {
-            @Override
-            public void run() {
-                Context applicationContext = ContextUtils.getApplicationContext();
-                String toastText =
-                        applicationContext.getString(R.string.added_to_homescreen, title);
-                Toast toast = Toast.makeText(applicationContext, toastText, Toast.LENGTH_SHORT);
-                toast.show();
-            }
-        });
+        Context applicationContext = ContextUtils.getApplicationContext();

[Yaron, 2016/08/10 14:57:01]

assert ui thread

+        String toastText = applicationContext.getString(R.string.added_to_homescreen, title);
+        Toast toast = Toast.makeText(applicationContext, toastText, Toast.LENGTH_SHORT);
+        toast.show();
     }
 
     /**
@@ -257,10 +257,13 @@ public class ShortcutHelper {
      * @param backgroundColor Background color of the web app.
      * @param isIconGenerated True if the icon is generated by Chromium.
      * @return Intent for onclick action of the shortcut.
+     * This method must not be called on the UI thread.
      */
     public static Intent createWebappShortcutIntent(String id, String action, String url,
             String scope, String name, String shortName, Bitmap icon, int version, int displayMode,
             int orientation, long themeColor, long backgroundColor, boolean isIconGenerated) {
+        assert !ThreadUtils.runningOnUiThread();
+
         // Encode the icon as a base64 string (Launcher drops Bitmaps in the Intent).
         String encodedIcon = encodeBitmapAsString(icon);