[Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 1/3

chrome/browser/android/banners/app_banner_infobar_delegate_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 "chrome/browser/android/banners/app_banner_infobar_delegate_android.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
@@ skipping 355 matching lines @@
   if (weak_manager_ && TriggeredFromBanner()) {
     weak_manager_->SendBannerAccepted(event_request_id_);
     // TODO(mgiuca): Send the appinstalled event for WebAPKs (but just removing
     // this check won't be sufficient).
     if (!is_webapk_)
       weak_manager_->OnInstall();
   }
 }
 
 void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished(
-    bool success,
+    WebApkInstallResult result,
     const std::string& webapk_package_name) {
-  if (!success) {
-    OnWebApkInstallFailed();
+  if (result != WebApkInstallResult::INSTALLED) {
+    OnWebApkInstallFailed(result);
     return;
   }
   UpdateStateForInstalledWebAPK(webapk_package_name);
   webapk::TrackInstallDuration(timer_->Elapsed());
   timer_.reset();
   webapk::TrackInstallEvent(webapk::INSTALL_COMPLETED);
 }
 
-void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFailed() {
+void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFailed(
+    WebApkInstallResult result) {
   DVLOG(1) << "The WebAPK installation failed.";
   webapk::TrackInstallEvent(webapk::INSTALL_FAILED);
 
   if (!infobar())
     return;
 
-  content::WebContents* web_contents =
-      InfoBarService::WebContentsFromInfoBar(infobar());
-  // Add webapp shortcut to the homescreen.
-  // TODO(pkotwicz): Only add webapp shortcut to the homescreen if
-  // WebAPK install did not timeout. If the WebAPK install timed out
-  // it is possible that Google Play is taking a long time and will
-  // eventually installs the WebAPK.
-  ShortcutHelper::AddToLauncherWithSkBitmap(web_contents, *shortcut_info_,
-                                            *icon_.get());
+  if (result == WebApkInstallResult::INSTALL_FAILED) {
+    content::WebContents* web_contents =
+        InfoBarService::WebContentsFromInfoBar(infobar());
+    // Add webapp shortcut to the homescreen.
+    ShortcutHelper::AddToLauncherWithSkBitmap(web_contents, *shortcut_info_,
+                                              *icon_.get());
+  } else {
+    // The WebAPK install timed out. Don't add shortcut because the install
+    // might still succeed (e.g. the install timed out because Google Play was
+    // busy with another install). It is confusing for a user to have homescreen
+    // shortcuts for both a WebAPK and a webapp for the same page URL.
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_AppBannerInfoBarDelegateAndroid_showWebApkInstallFailureToast(env);

[dominickn, 2017/03/05 23:52:39]

If the WebAPK install still might succeed, why show the toast? Won't it be
confusing if the toast shows and then the install succeeds afterwards?

[pkotwicz, 2017/03/12 21:11:48]

It would also be confusing if the install fails and the user gets no feedback at
all.

Personally, I prefer if the user is happily surprised that the WebAPK did in
fact succeed rather than the user being confused about why the infobar
disappeared with no explanation

[dominickn, 2017/03/13 03:22:23]

We should probably check in with UI/PM on this. Given the timeout is 3 minutes
long, it's quite possible that the user has already navigated off the page
(killing the infobar), or scrolled around (hiding it) by the time we hit this.

I think the confusion of being told that the install failed and then having the
WebAPK end up mysteriously in your All Apps is worse than the WebAPK infobar
disappearing.

[pkotwicz, 2017/03/14 22:38:57]

We discussed this at the weekly meeting and decided on not showing a toast

+  }
 
   infobar()->RemoveSelf();
 }
 
 void AppBannerInfoBarDelegateAndroid::TrackWebApkInstallationDismissEvents(
     InstallState install_state) {
   if (install_state == INSTALL_NOT_STARTED) {
     webapk::TrackInstallEvent(webapk::INFOBAR_DISMISSED_BEFORE_INSTALLATION);
   } else if (install_state == INSTALLING) {
     webapk::TrackInstallEvent(webapk::INFOBAR_DISMISSED_DURING_INSTALLATION);
@@ skipping 67 matching lines @@
 
   TrackDismissEvent(DISMISS_EVENT_BANNER_CLICK);
   return true;
 }
 
 bool RegisterAppBannerInfoBarDelegateAndroid(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace banners