Use 'display' when creating a shortcut on Android.

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_string.h"
@@ skipping 29 matching lines @@
 
   return reinterpret_cast<intptr_t>(shortcut_helper);
 }
 
 ShortcutHelper::ShortcutHelper(JNIEnv* env,
                                jobject obj,
                                content::WebContents* web_contents)
     : WebContentsObserver(web_contents),
       java_ref_(env, obj),
       url_(web_contents->GetURL()),
-      web_app_capable_(WebApplicationInfo::MOBILE_CAPABLE_UNSPECIFIED),
+      display_(content::Manifest::DISPLAY_MODE_BROWSER),
       weak_ptr_factory_(this) {
 }
 
 void ShortcutHelper::Initialize() {
   // Send a message to the renderer to retrieve information about the page.
   Send(new ChromeViewMsg_GetWebApplicationInfo(routing_id()));
 }
 
 ShortcutHelper::~ShortcutHelper() {
 }
 
 void ShortcutHelper::OnDidGetWebApplicationInfo(
     const WebApplicationInfo& received_web_app_info) {
   // Sanitize received_web_app_info.
   WebApplicationInfo web_app_info = received_web_app_info;
   web_app_info.title =
       web_app_info.title.substr(0, chrome::kMaxMetaTagAttributeLength);
   web_app_info.description =
       web_app_info.description.substr(0, chrome::kMaxMetaTagAttributeLength);
 
-  web_app_capable_ = web_app_info.mobile_capable;
-
   title_ = web_app_info.title.empty() ? web_contents()->GetTitle()
                                       : web_app_info.title;
 
+  if (web_app_info.mobile_capable == WebApplicationInfo::MOBILE_CAPABLE ||

[gone, 2014/09/17 17:11:24]

didn't this field get renamed to web_app_capable?  if not, still don't think
this field name makes sense.

[mlamouri (slow - plz ping), 2014/09/17 17:16:40]

When you asked me to rename, I assumed that you meant for the variable (that was
web_app_capable_ but which is removed by that CL).

It make sense to insist on the web part and less the mobile part in the context
of ShortcutHelper but WebApplicationInfo isn't mobile specific (it's being used
by Chrome Apps for example) so I think mobile_capable is a better name given
that the "web" part is kind of assumed (it's part of WebApplicationInfo...).

+      web_app_info.mobile_capable == WebApplicationInfo::MOBILE_CAPABLE_APPLE) {
+    display_ = content::Manifest::DISPLAY_MODE_STANDALONE;
+  }
+
+  // Record what type of shortcut was added by the user.
+  switch (web_app_info.mobile_capable) {
+    case WebApplicationInfo::MOBILE_CAPABLE:
+      content::RecordAction(
+          base::UserMetricsAction("webapps.AddShortcut.AppShortcut"));
+      break;
+    case WebApplicationInfo::MOBILE_CAPABLE_APPLE:
+      content::RecordAction(
+          base::UserMetricsAction("webapps.AddShortcut.AppShortcutApple"));
+      break;
+    case WebApplicationInfo::MOBILE_CAPABLE_UNSPECIFIED:
+      content::RecordAction(
+          base::UserMetricsAction("webapps.AddShortcut.Bookmark"));
+      break;
+    default:

[Bernhard Bauer, 2014/09/17 17:51:09]

You can leave out the default statement; the compiler will complain if you're
missing a value (and that way it will complain if a new value is added, but you
forget to update this place!).

+      NOTREACHED();
+  }
+
   web_contents()->GetManifest(base::Bind(&ShortcutHelper::OnDidGetManifest,
                                          weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ShortcutHelper::OnDidGetManifest(const content::Manifest& manifest) {
   // Set the title based on the manifest value, if any.
   if (!manifest.short_name.is_null())
     title_ = manifest.short_name.string();
   else if (!manifest.name.is_null())
     title_ = manifest.name.string();
 
   // Set the url based on the manifest value, if any.
   if (manifest.start_url.is_valid())
     url_ = manifest.start_url;
 
+  // Set the display based on the manifest value, if any.
+  if (manifest.display != content::Manifest::DISPLAY_MODE_UNSPECIFIED)
+    display_ = manifest.display;
+
+  // 'fullscreen' and 'minimal-ui' are not yet supported, fallback to the right
+  // mode in those cases.
+  if (manifest.display == content::Manifest::DISPLAY_MODE_FULLSCREEN)
+    display_ = content::Manifest::DISPLAY_MODE_STANDALONE;
+  if (manifest.display == content::Manifest::DISPLAY_MODE_MINIMAL_UI)
+    display_ = content::Manifest::DISPLAY_MODE_BROWSER;
+
   // The ShortcutHelper is now able to notify its Java counterpart that it is
   // initialized. OnInitialized method is not conceptually part of getting the
   // manifest data but it happens that the initialization is finalized when
   // these data are available.
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
   ScopedJavaLocalRef<jstring> j_title =
       base::android::ConvertUTF16ToJavaString(env, title_);
 
   Java_ShortcutHelper_onInitialized(env, j_obj.obj(), j_title.obj());
@@ skipping 44 matching lines @@
   icon_ = bitmap_result;
 
   // Stop observing so we don't get destroyed while doing the last steps.
   Observe(NULL);
 
   base::WorkerPool::PostTask(
       FROM_HERE,
       base::Bind(&ShortcutHelper::AddShortcutInBackground,
                  url_,
                  title_,
-                 web_app_capable_,
+                 display_,
                  icon_),
       true);
 
   Destroy();
 }
 
 bool ShortcutHelper::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
 
   IPC_BEGIN_MESSAGE_MAP(ShortcutHelper, message)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_DidGetWebApplicationInfo,
                         OnDidGetWebApplicationInfo)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
   return handled;
 }
 
 void ShortcutHelper::WebContentsDestroyed() {
   Destroy();
 }
 
 bool ShortcutHelper::RegisterShortcutHelper(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 void ShortcutHelper::AddShortcutInBackground(
     const GURL& url,
     const base::string16& title,
-    WebApplicationInfo::MobileCapable web_app_capable,
+    content::Manifest::DisplayMode display,
     const favicon_base::FaviconRawBitmapResult& bitmap_result) {
   DCHECK(base::WorkerPool::RunsTasksOnCurrentThread());
 
   // Grab the average color from the bitmap.
   SkColor color = SK_ColorWHITE;
   SkBitmap favicon_bitmap;
   if (bitmap_result.is_valid()) {
     if (gfx::PNGCodec::Decode(bitmap_result.bitmap_data->front(),
                               bitmap_result.bitmap_data->size(),
                               &favicon_bitmap))
@@ skipping 16 matching lines @@
 
   Java_ShortcutHelper_addShortcut(
       env,
       base::android::GetApplicationContext(),
       java_url.obj(),
       java_title.obj(),
       java_bitmap.obj(),
       r_value,
       g_value,
       b_value,
-      web_app_capable != WebApplicationInfo::MOBILE_CAPABLE_UNSPECIFIED);
-
-  // Record what type of shortcut was added by the user.
-  switch (web_app_capable) {
-    case WebApplicationInfo::MOBILE_CAPABLE:
-      content::RecordAction(
-          base::UserMetricsAction("webapps.AddShortcut.AppShortcut"));
-      break;
-    case WebApplicationInfo::MOBILE_CAPABLE_APPLE:
-      content::RecordAction(
-          base::UserMetricsAction("webapps.AddShortcut.AppShortcutApple"));
-      break;
-    case WebApplicationInfo::MOBILE_CAPABLE_UNSPECIFIED:
-      content::RecordAction(
-          base::UserMetricsAction("webapps.AddShortcut.Bookmark"));
-      break;
-    default:
-      NOTREACHED();
-  }
+      display == content::Manifest::DISPLAY_MODE_STANDALONE);
 }