Trigger app banner when add to homescreen is pressed and WebAPKs are enabled.

chrome/browser/android/banners/app_banner_manager_android.cc

 // Copyright 2015 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 <utility>
 
 #include "chrome/browser/android/banners/app_banner_manager_android.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 22 matching lines @@
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(banners::AppBannerManagerAndroid);
 
 namespace {
 
 const char kPlayPlatform[] = "play";
 const char kReferrerName[] = "referrer";
 const char kIdName[] = "id";
 const char kPlayInlineReferrer[] = "playinline=chrome_inline";
 
+std::unique_ptr<ShortcutInfo> CreateShortcutInfo(
+    const GURL& manifest_url,
+    const content::Manifest& manifest,
+    const GURL& icon_url) {
+  std::unique_ptr<ShortcutInfo> info_ptr(new ShortcutInfo(GURL::EmptyGURL()));

[Peter Kasting, 2016/09/06 19:31:23]

Don't use EmptyGURL() just because you want to pass an empty GURL.  Use GURL()
for that.  EmptyGURL() is for functions that need to return a const ref to an
empty GURL.

Nit: Prefer a name like "shortcut_info" to a less-descriptive, code-redundant
name like "info_ptr" (we already know it's some kind of pointer from the use of
the -> operator).

[Xi Han, 2016/09/07 15:53:17]

Done.

+  if (!manifest.IsEmpty()) {
+    info_ptr->UpdateFromManifest(manifest);
+    info_ptr->manifest_url = manifest_url;
+    info_ptr->icon_url = icon_url;
+    info_ptr->UpdateSource(ShortcutInfo::SOURCE_APP_BANNER);
+  }
+  return info_ptr;
+}
+
 }  // anonymous namespace
 
 namespace banners {
 
 AppBannerManagerAndroid::AppBannerManagerAndroid(
     content::WebContents* web_contents)
     : AppBannerManager(web_contents) {
   CreateJavaBannerManager();
 }
 
@@ skipping 130 matching lines @@
 
 void AppBannerManagerAndroid::ShowBanner() {
   content::WebContents* contents = web_contents();
   DCHECK(contents);
 
   infobars::InfoBar* infobar = nullptr;
   if (native_app_data_.is_null()) {
     bool is_webapk = ChromeWebApkHost::AreWebApkEnabled();
     std::unique_ptr<AppBannerInfoBarDelegateAndroid> delegate(
         new AppBannerInfoBarDelegateAndroid(
-            GetWeakPtr(), app_title_, manifest_url_, manifest_, icon_url_,
+            GetWeakPtr(), app_title_,
+            CreateShortcutInfo(manifest_url_, manifest_, icon_url_),
             std::move(icon_), event_request_id(), is_webapk));
 
     infobar = new AppBannerInfoBarAndroid(
         std::move(delegate), manifest_.start_url, is_webapk);
     if (infobar) {
       RecordDidShowBanner("AppBanner.WebApp.Shown");

[Peter Kasting, 2016/09/06 19:31:23]

The fact that these blocks are basically duplicated, and that this one largely
duplicates the desktop code, is more support for the idea that this sort of
thing should all be happening in the infobar delegate Create() function, not in
the caller here.

Furthermore, these blocks are all incorrect: they all log things like "banner
shown" without checking the return of AddInfoBar() to see if the infobars in
question were actually shown.  See comment elsewhere noting you have to check
the return value of that.

[Xi Han, 2016/09/07 15:53:17]

Updated. Make the Create return a bool, and only log things when AddInfoBar()
succeeds.

       TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_CREATED);
       ReportStatus(contents, SHOWING_WEB_APP_BANNER);
     } else {
       ReportStatus(contents, FAILED_TO_CREATE_BANNER);
     }
   } else {
     std::unique_ptr<AppBannerInfoBarDelegateAndroid> delegate(
         new AppBannerInfoBarDelegateAndroid(
             app_title_, native_app_data_, std::move(icon_), native_app_package_,
             referrer_, event_request_id()));
@@ skipping 121 matching lines @@
 }
 
 // static
 void SetTimeDeltaForTesting(JNIEnv* env,
                             const JavaParamRef<jclass>& clazz,
                             jint days) {
   AppBannerManager::SetTimeDeltaForTesting(days);
 }
 
 }  // namespace banners