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

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/guid.h"
 #include "base/location.h"
@@ skipping 21 matching lines @@
 
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertJavaStringToUTF16;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertUTF16ToJavaString;
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
 
 namespace {
 
-bool IsInfoEmpty(const ShortcutInfo* info) {
-  return !info || info->url.is_empty();
+bool IsInfoEmpty(const std::unique_ptr<ShortcutInfo>& info) {
+  return !info.get() || info->url.is_empty();

[Peter Kasting, 2016/09/09 21:50:45]

Nit: .get() probably not necessary

[Xi Han, 2016/09/12 13:49:50]

Done.

 }
 
 }
 
 namespace banners {
 
 // static
 bool AppBannerInfoBarDelegateAndroid::Create(
     content::WebContents* web_contents,
     base::WeakPtr<AppBannerManager> weak_manager,
@@ skipping 37 matching lines @@
       ->AddInfoBar(base::MakeUnique<AppBannerInfoBarAndroid>(
            std::move(infobar_delegate), native_app_data));
 }
 
 AppBannerInfoBarDelegateAndroid::~AppBannerInfoBarDelegateAndroid() {
   weak_ptr_factory_.InvalidateWeakPtrs();
 
   if (!has_user_interaction_) {
     if (!native_app_data_.is_null())
       TrackUserResponse(USER_RESPONSE_NATIVE_APP_IGNORED);
-    else if (!IsInfoEmpty(shortcut_info_.get()))
+    else if (!IsInfoEmpty(shortcut_info_))
       TrackUserResponse(USER_RESPONSE_WEB_APP_IGNORED);
   }
 
   TrackDismissEvent(DISMISS_EVENT_DISMISSED);
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_AppBannerInfoBarDelegateAndroid_destroy(env, java_delegate_);
   java_delegate_.Reset();
 }
 
 void AppBannerInfoBarDelegateAndroid::UpdateInstallState(
@@ skipping 43 matching lines @@
     TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_COMPLETED);
     UpdateInstallState(env, obj);
   } else if (infobar()->owner()) {
     TrackDismissEvent(DISMISS_EVENT_INSTALL_TIMEOUT);
     infobar()->owner()->RemoveInfoBar(infobar());
   }
 }
 
 bool AppBannerInfoBarDelegateAndroid::AcceptWebApk(
     content::WebContents* web_contents) {
-  if (IsInfoEmpty(shortcut_info_.get()))
+  if (IsInfoEmpty(shortcut_info_))
     return true;
 
   JNIEnv* env = base::android::AttachCurrentThread();
   // |webapk_package_name_| is set when the WebAPK has finished installing.
   // If the |webapk_package_name_| is empty, it means the "Add to Homescreen"
   // button is pressed, so request WebAPK installation. Otherwise, it means
   // the "Open" button is pressed, so open the installed WebAPK.
   if (!webapk_package_name_.empty()) {
     // Open the WebAPK.
     ScopedJavaLocalRef<jstring> java_webapk_package_name =
@@ skipping 34 matching lines @@
     int event_request_id,
     bool is_webapk)
     : weak_manager_(weak_manager),
       app_title_(app_title),
       shortcut_info_(std::move(shortcut_info)),
       icon_(std::move(icon)),
       event_request_id_(event_request_id),
       has_user_interaction_(false),
       is_webapk_(is_webapk),
       weak_ptr_factory_(this) {
-  DCHECK(!IsInfoEmpty(shortcut_info_.get()));
+  DCHECK(!IsInfoEmpty(shortcut_info_));
   CreateJavaDelegate();
 }
 
 AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid(
     const base::string16& app_title,
     const base::android::ScopedJavaGlobalRef<jobject>& native_app_data,
     std::unique_ptr<SkBitmap> icon,
     const std::string& native_app_package,
     const std::string& referrer,
     int event_request_id)
@@ skipping 38 matching lines @@
   has_user_interaction_ = true;
 
   content::WebContents* web_contents =
       InfoBarService::WebContentsFromInfoBar(infobar());
 
   web_contents->GetMainFrame()->Send(
       new ChromeViewMsg_AppBannerDismissed(
           web_contents->GetMainFrame()->GetRoutingID(),
           event_request_id_));
 
-  if (!native_app_data_.is_null()) {
+  if (native_app_data_.is_null()) {
+    TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
+    AppBannerSettingsHelper::RecordBannerDismissEvent(
+        web_contents, shortcut_info_->url.spec(), AppBannerSettingsHelper::WEB);
+  } else {
     TrackUserResponse(USER_RESPONSE_NATIVE_APP_DISMISSED);
     AppBannerSettingsHelper::RecordBannerDismissEvent(
         web_contents, native_app_package_, AppBannerSettingsHelper::NATIVE);
-  } else {
-    TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
-    AppBannerSettingsHelper::RecordBannerDismissEvent(
-        web_contents, shortcut_info_->url.spec(), AppBannerSettingsHelper::WEB);
   }
 }
 
 base::string16 AppBannerInfoBarDelegateAndroid::GetMessageText() const {
   return app_title_;
 }
 
 int AppBannerInfoBarDelegateAndroid::GetButtons() const {
   return BUTTON_OK;
 }
@@ skipping 36 matching lines @@
     TrackDismissEvent(DISMISS_EVENT_APP_OPEN);
   else
     TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_TRIGGERED);
 
   SendBannerAccepted(web_contents, "play");
   return was_opened;
 }
 
 bool AppBannerInfoBarDelegateAndroid::AcceptWebApp(
     content::WebContents* web_contents) {
-  if (IsInfoEmpty(shortcut_info_.get()))
+  if (IsInfoEmpty(shortcut_info_))
     return true;
   TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
 
   AppBannerSettingsHelper::RecordBannerInstallEvent(
       web_contents, shortcut_info_->url.spec(), AppBannerSettingsHelper::WEB);
 
   if (weak_manager_) {
     const std::string& uid = base::GenerateGUID();
     ShortcutHelper::AddToLauncherWithSkBitmap(
         web_contents->GetBrowserContext(), *shortcut_info_, uid,
         *icon_.get(), weak_manager_->FetchWebappSplashScreenImageCallback(uid));
   }
 
   SendBannerAccepted(web_contents, "web");
   return true;
 }
 
 void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished(
     bool success,
     const std::string& webapk_package_name) {
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!success) {
     // The installation failed.
+    Java_AppBannerInfoBarDelegateAndroid_showWebApkInstallFailureToast(env);
     if (infobar())
       infobar()->RemoveSelf();
-    Java_AppBannerInfoBarDelegateAndroid_showWebApkInstallFailureToast(env);
     DVLOG(1) << "The WebAPK installation failed.";

[Peter Kasting, 2016/09/09 21:50:45]

Nit: Move this to the top of the block and replace the then-redundant comment?

[Xi Han, 2016/09/12 13:49:50]

Done.

     return;
   }
 
   webapk_package_name_ = webapk_package_name;
   ScopedJavaLocalRef<jstring> java_webapk_package_name =
       base::android::ConvertUTF8ToJavaString(env, webapk_package_name);
   Java_AppBannerInfoBarDelegateAndroid_setWebApkInstallingState(
       env, java_delegate_, false);
   Java_AppBannerInfoBarDelegateAndroid_setWebApkPackageName(
       env, java_delegate_, java_webapk_package_name);
@@ skipping 18 matching lines @@
 
   TrackDismissEvent(DISMISS_EVENT_BANNER_CLICK);
   return true;
 }
 
 bool RegisterAppBannerInfoBarDelegateAndroid(JNIEnv* env) {
  return RegisterNativesImpl(env);
 }
 
 }  // namespace banners