Use 'display' when creating a shortcut on Android.

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 2a2f2a3fb7fac4514323465c91a5e16aa8706157..b2093f8c466221b3ae365fdb551490806fb541e8 100644
--- a/chrome/browser/android/shortcut_helper.cc
+++ b/chrome/browser/android/shortcut_helper.cc
@@ -47,7 +47,7 @@ ShortcutHelper::ShortcutHelper(JNIEnv* env,
     : 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) {
 }
 
@@ -68,11 +68,32 @@ void ShortcutHelper::OnDidGetWebApplicationInfo(
   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()));
 }
@@ -88,6 +109,17 @@ void ShortcutHelper::OnDidGetManifest(const content::Manifest& manifest) {
   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
@@ -152,7 +184,7 @@ void ShortcutHelper::FinishAddingShortcut(
       base::Bind(&ShortcutHelper::AddShortcutInBackground,
                  url_,
                  title_,
-                 web_app_capable_,
+                 display_,
                  icon_),
       true);
 
@@ -182,7 +214,7 @@ bool ShortcutHelper::RegisterShortcutHelper(JNIEnv* 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());
 
@@ -219,23 +251,5 @@ void ShortcutHelper::AddShortcutInBackground(
       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);
 }