[WebAPKs] Display same text for menu & engagement banner

chrome/browser/android/banners/app_banner_infobar_delegate_android.cc

Index: chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
diff --git a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc b/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
index b9229eb88efa2a24ef9b62f51fa8386300560155..33b723ae7ac881f90dd541f1aad5d76de9b86ea3 100644
--- a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
+++ b/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
@@ -41,7 +41,6 @@ namespace banners {
 bool AppBannerInfoBarDelegateAndroid::Create(
     content::WebContents* web_contents,
     base::WeakPtr<AppBannerManager> weak_manager,
-    const base::string16& app_title,
     std::unique_ptr<ShortcutInfo> shortcut_info,
     const SkBitmap& primary_icon,
     const SkBitmap& badge_icon,
@@ -55,8 +54,8 @@ bool AppBannerInfoBarDelegateAndroid::Create(
 
   auto infobar_delegate =
       base::WrapUnique(new banners::AppBannerInfoBarDelegateAndroid(
-          weak_manager, app_title, std::move(shortcut_info), primary_icon,
-          badge_icon, event_request_id, is_webapk, webapk_install_source));
+          weak_manager, std::move(shortcut_info), primary_icon, badge_icon,
+          event_request_id, is_webapk, webapk_install_source));
   auto* raw_delegate = infobar_delegate.get();
   auto infobar = base::MakeUnique<AppBannerInfoBarAndroid>(
       std::move(infobar_delegate), url, is_webapk);
@@ -203,7 +202,6 @@ void AppBannerInfoBarDelegateAndroid::UpdateStateForInstalledWebAPK(
 
 AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid(
     base::WeakPtr<AppBannerManager> weak_manager,
-    const base::string16& app_title,
     std::unique_ptr<ShortcutInfo> shortcut_info,
     const SkBitmap& primary_icon,
     const SkBitmap& badge_icon,
@@ -211,7 +209,7 @@ AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid(
     bool is_webapk,
     webapk::InstallSource webapk_install_source)
     : weak_manager_(weak_manager),
-      app_title_(app_title),
+      app_title_(shortcut_info->name),
       shortcut_info_(std::move(shortcut_info)),
       primary_icon_(primary_icon),
       badge_icon_(badge_icon),
@@ -280,8 +278,8 @@ bool AppBannerInfoBarDelegateAndroid::AcceptWebApp(
   AppBannerSettingsHelper::RecordBannerInstallEvent(
       web_contents, shortcut_info_->url.spec(), AppBannerSettingsHelper::WEB);
 
-  ShortcutHelper::AddToLauncherWithSkBitmap(web_contents, *shortcut_info_,
-                                            primary_icon_);
+  ShortcutHelper::AddToLauncherWithSkBitmap(
+      web_contents, *shortcut_info_, shortcut_info_->short_name, primary_icon_);
 
   SendBannerAccepted();
   return true;
@@ -377,6 +375,7 @@ void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFailed(
         InfoBarService::WebContentsFromInfoBar(infobar());
     // Add webapp shortcut to the homescreen.
     ShortcutHelper::AddToLauncherWithSkBitmap(web_contents, *shortcut_info_,
+                                              shortcut_info_->short_name,

[dominickn, 2017/06/16 01:32:32]

This is a behaviour change for app banners. Previously, app banners would prefer
|name| over |short_name| if both were present (see the title setting logic in
app_banner_manager.cc). Now it will prefer |short_name| if both are present. In
fact, our documentation explicitly calls out the |name| field as being used in
the banner itself, while |short_name| is used on the homescreen, so we need to
preserve this default behaviour
(https://developers.google.com/web/fundamentals/engage-and-retain/app-install-...)

I'll reiterate the first comment I made: 
I think it makes more sense to just reverse the logic in UpdateFromManifest() to
match that used in app banners. The other thing that UpdateFromManifest()
doesn't do it check if the string() is empty as opposed to non-null. This means
you could set short_name: "" and end up with a WebAPK banner with an empty title
(we had this issue recently with app banners for m.forbes.com I think).

[pkotwicz, 2017/06/17 00:17:23]

Thank you for your question.

This is actually incorrect. The banner title and the name of the installed web
app used to come from two different places:
The banner title used to come from AppBannerManager::app_title_
The name of the install web app used to come from CreateShortcutInfo() in
app_banner_manager_android.cc

If I changed the logic in ShortcutInfo::UpdateFromManifest() to match the banner
code, this would change home screen shortcuts to use the "name" instead of the
"short_name". This would be a change in behavior for both home screen shortcuts
added from the app menu and home screen shortcuts added from the banner. As far
as I can tell, the "short_name" has been used for home screen shortcuts for at
least a year.

[dominickn, 2017/06/17 01:05:44]

Yes, we should still use the |short_name| for the home screen shortcut, so my
proposed solution there would break that.

The behaviour we want is to use |name| for the banner title if it's present,
rather than |short_name|. This means devs can use a longer, full name in the
banner and have the truncated one be in the actual shortcut if the real name is
too long. In short:

A2HS Dialog:
  - if not WebAPK: prefer short_name (actual text that will go in the shortcut)
  - if WebAPK: prefer name in the banner, but use short_name for shortcut

Banner:
 - prefer name in the banner, but use short_name for shortcut

                                               primary_icon_);
   }