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

chrome/browser/android/shortcut_helper.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/android/shortcut_helper.h"
 
 #include <jni.h>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #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"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/color_analysis.h"
 #include "url/gurl.h"
 
 using content::Manifest;
 
 namespace {
@@ skipping 32 matching lines @@
 } // anonymous namespace
 
 // static
 void ShortcutHelper::AddShortcutInBackgroundWithSkBitmap(
     const ShortcutInfo& info,
     const std::string& webapp_id,
     const SkBitmap& icon_bitmap,
     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 =
       base::android::ConvertUTF8ToJavaString(env, webapp_id);
   ScopedJavaLocalRef<jstring> java_url =
       base::android::ConvertUTF8ToJavaString(env, info.url.spec());
   ScopedJavaLocalRef<jstring> java_user_title =
       base::android::ConvertUTF16ToJavaString(env, info.user_title);
   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());
 
-  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(),
       java_user_title.obj(),
       java_name.obj(),
       java_short_name.obj(),
       java_icon_url.obj(),
       java_bitmap.obj(),
       info.display,
       info.orientation,
       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();
   return kIdealHomescreenIconSize;
 }
 
 int ShortcutHelper::GetMinimumHomescreenIconSizeInDp() {
   if (kMinimumHomescreenIconSize == -1)
     GetHomescreenIconAndSplashImageSizes();
   return kMinimumHomescreenIconSize;
@@ skipping 96 matching lines @@
   DCHECK(jsplash_image_callback);
   base::Closure* splash_image_callback =
       reinterpret_cast<base::Closure*>(jsplash_image_callback);
   splash_image_callback->Run();
   delete splash_image_callback;
 }
 
 bool ShortcutHelper::RegisterShortcutHelper(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }