Split ShortcutHelper#addShortcut() into separate functions for each type of shortcut

chrome/browser/android/shortcut_helper.cc

Index: chrome/browser/android/shortcut_helper.cc
diff --git a/chrome/browser/android/shortcut_helper.cc b/chrome/browser/android/shortcut_helper.cc
index e4769d98c6e7eda455c325684699a18b4f7067ed..426bb81e18534c9e8af6dfbc3381adfcb9d27fa9 100644
--- a/chrome/browser/android/shortcut_helper.cc
+++ b/chrome/browser/android/shortcut_helper.cc
@@ -11,9 +11,11 @@
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/command_line.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/manifest/manifest_icon_downloader.h"
+#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "jni/ShortcutHelper_jni.h"
@@ -66,6 +68,66 @@ void ShortcutHelper::AddShortcutInBackgroundWithSkBitmap(
     const base::Closure& splash_image_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
+  if (info.display == blink::WebDisplayModeStandalone ||
+      info.display == blink::WebDisplayModeFullscreen) {
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableWebApk) &&
+        AddWebApkInBackgroundWithSkBitmap(info, webapp_id, icon_bitmap)) {

[Xi Han, 2016/07/08 19:10:38]

If AddWebApkInBackgroundWithSkBitmap returns false, do we really want to fall
back to create a Webapp shortcut? I remembered that we will show a toast when
the WAM server fails to mint a WebAPK in the UI design.  Since creating a webapp
shortcut isn't the fallback anyway, I don't think we need to know nor check the
return value of this function. Also the return value only mean whether the
WebApkBuilder is available or not, not whether the WebAPK is installed or not,
since the installation task is async. 

[pkotwicz, 2016/07/10 19:25:00]

I have removed the fallback for now and have filed crbug.com/626950

[Xi Han, 2016/07/11 19:07:01]

Thanks for firing the bug!

+      return;
+    }
+    AddWebappInBackgroundWithSkBitmap(info, webapp_id, icon_bitmap,
+                                      splash_image_callback);
+    return;
+  }
+  AddBookmarkShortcutInBackgroundWithSkBitmap(info, icon_bitmap);
+}
+
+// static
+bool ShortcutHelper::AddWebApkInBackgroundWithSkBitmap(
+    const ShortcutInfo& info,
+    const std::string& webapp_id,
+    const SkBitmap& icon_bitmap) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // TODO(pkotwicz): Send request to WebAPK server to generate WebAPK.
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> java_url =
+      base::android::ConvertUTF8ToJavaString(env, info.url.spec());
+  ScopedJavaLocalRef<jstring> java_name =
+      base::android::ConvertUTF16ToJavaString(env, info.name);
+  ScopedJavaLocalRef<jstring> java_short_name =
+      base::android::ConvertUTF16ToJavaString(env, info.short_name);
+  ScopedJavaLocalRef<jstring> java_icon_url =
+      base::android::ConvertUTF8ToJavaString(env, info.icon_url.spec());
+  ScopedJavaLocalRef<jobject> java_bitmap;
+  if (icon_bitmap.getSize())
+    java_bitmap = gfx::ConvertToJavaBitmap(&icon_bitmap);
+  ScopedJavaLocalRef<jstring> java_manifest_url =
+      base::android::ConvertUTF8ToJavaString(env, info.manifest_url.spec());
+
+  return Java_ShortcutHelper_installWebApk(
+      env,
+      java_url.obj(),
+      java_name.obj(),
+      java_short_name.obj(),
+      java_icon_url.obj(),
+      java_bitmap.obj(),
+      info.display,
+      info.orientation,
+      info.theme_color,
+      info.background_color,
+      java_manifest_url.obj());
+}
+
+// static
+void ShortcutHelper::AddWebappInBackgroundWithSkBitmap(
+    const ShortcutInfo& info,
+    const std::string& webapp_id,
+    const SkBitmap& icon_bitmap,
+    const base::Closure& splash_image_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
   // Send the data to the Java side to create the shortcut.
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> java_webapp_id =
@@ -83,22 +145,15 @@ void ShortcutHelper::AddShortcutInBackgroundWithSkBitmap(
   ScopedJavaLocalRef<jobject> java_bitmap;
   if (icon_bitmap.getSize())
     java_bitmap = gfx::ConvertToJavaBitmap(&icon_bitmap);
-  ScopedJavaLocalRef<jstring> java_manifest_url =
-      base::android::ConvertUTF8ToJavaString(env, info.manifest_url.spec());
 
-  uintptr_t callback_pointer = 0;
-  if (info.display == blink::WebDisplayModeStandalone ||
-      info.display == blink::WebDisplayModeFullscreen) {
-    // The callback will need to be run after shortcut creation completes in
-    // order to download the splash image and save it to the WebappDataStorage.
-    // Create a copy of the callback here and send the pointer to Java, which
-    // will send it back once the asynchronous shortcut creation process
-    // finishes.
-    callback_pointer =
-        reinterpret_cast<uintptr_t>(new base::Closure(splash_image_callback));
-  }
+  // The callback will need to be run after shortcut creation completes in order
+  // to download the splash image and save it to the WebappDataStorage. Create a
+  // copy of the callback here and send the pointer to Java, which will send it
+  // back once the asynchronous shortcut creation process finishes.
+  uintptr_t callback_pointer =
+      reinterpret_cast<uintptr_t>(new base::Closure(splash_image_callback));
 
-  Java_ShortcutHelper_addShortcut(
+  Java_ShortcutHelper_addWebappShortcut(
       env,
       java_webapp_id.obj(),
       java_url.obj(),
@@ -112,10 +167,26 @@ void ShortcutHelper::AddShortcutInBackgroundWithSkBitmap(
       info.source,
       info.theme_color,
       info.background_color,
-      java_manifest_url.obj(),
       callback_pointer);
 }
 
+void ShortcutHelper::AddBookmarkShortcutInBackgroundWithSkBitmap(
+    const ShortcutInfo& info,
+    const SkBitmap& icon_bitmap) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> java_url =
+      base::android::ConvertUTF8ToJavaString(env, info.url.spec());
+  ScopedJavaLocalRef<jstring> java_user_title =
+      base::android::ConvertUTF16ToJavaString(env, info.user_title);
+  ScopedJavaLocalRef<jobject> java_bitmap;
+  if (icon_bitmap.getSize())
+    java_bitmap = gfx::ConvertToJavaBitmap(&icon_bitmap);
+
+  Java_ShortcutHelper_addBookmarkShortcut(env, java_url.obj(),
+                                          java_user_title.obj(),
+                                          java_bitmap.obj(), info.source);
+}
+
 int ShortcutHelper::GetIdealHomescreenIconSizeInDp() {
   if (kIdealHomescreenIconSize == -1)
     GetHomescreenIconAndSplashImageSizes();