Make AppBannerInfoBar install WebAPK.

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 aa03a7d8e1665feab06ef6b40cbbe2344bf98778..0ca19c198cb0abd83fb84edc79b8551c2401df9f 100644
--- a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
+++ b/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/android/shortcut_helper.h"
 #include "chrome/browser/android/shortcut_info.h"
 #include "chrome/browser/android/tab_android.h"
+#include "chrome/browser/android/webapk/webapk_installer.h"
 #include "chrome/browser/banners/app_banner_manager.h"
 #include "chrome/browser/banners/app_banner_metrics.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
@@ -23,6 +24,7 @@
 #include "chrome/common/render_messages.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/rappor/rappor_utils.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/manifest.h"
@@ -46,7 +48,8 @@ AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid(
     const content::Manifest& manifest,
     const GURL& icon_url,
     std::unique_ptr<SkBitmap> icon,
-    int event_request_id)
+    int event_request_id,
+    bool is_webapk)
     : weak_manager_(weak_manager),
       app_title_(app_title),
       manifest_url_(manifest_url),
@@ -54,7 +57,8 @@ AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid(
       icon_url_(icon_url),
       icon_(std::move(icon)),
       event_request_id_(event_request_id),
-      has_user_interaction_(false) {
+      has_user_interaction_(false),
+      is_webapk_(is_webapk) {
   DCHECK(!manifest.IsEmpty());
   CreateJavaDelegate();
 }
@@ -81,6 +85,8 @@ AppBannerInfoBarDelegateAndroid::~AppBannerInfoBarDelegateAndroid() {
   if (!has_user_interaction_) {
     if (!native_app_data_.is_null())
       TrackUserResponse(USER_RESPONSE_NATIVE_APP_IGNORED);
+    else if (is_webapk_)
+      TrackUserResponse(USER_RESPONSE_WEBAPK_IGNORED);
     else if (!manifest_.IsEmpty())
       TrackUserResponse(USER_RESPONSE_WEB_APP_IGNORED);
   }
@@ -94,7 +100,7 @@ AppBannerInfoBarDelegateAndroid::~AppBannerInfoBarDelegateAndroid() {
 void AppBannerInfoBarDelegateAndroid::UpdateInstallState(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj) {
-  if (native_app_data_.is_null())
+  if (native_app_data_.is_null() && !is_webapk_)
     return;
 
   int newState = Java_AppBannerInfoBarDelegateAndroid_determineInstallState(
@@ -140,7 +146,13 @@ void AppBannerInfoBarDelegateAndroid::OnInstallFinished(
     return;
 
   if (success) {
-    TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_COMPLETED);
+    if (!is_webapk_) {
+      TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_COMPLETED);
+    } else {
+      TrackInstallEvent(INSTALL_EVENT_WEBAPK_INSTALL_COMPLETED);
+      Java_AppBannerInfoBarDelegateAndroid_setWebApkInstallingState(
+          env, java_delegate_, false);
+    }
     UpdateInstallState(env, obj);
   } else if (infobar()->owner()) {
     TrackDismissEvent(DISMISS_EVENT_INSTALL_TIMEOUT);
@@ -152,7 +164,8 @@ void AppBannerInfoBarDelegateAndroid::CreateJavaDelegate() {
   JNIEnv* env = base::android::AttachCurrentThread();
   java_delegate_.Reset(Java_AppBannerInfoBarDelegateAndroid_create(
       env,
-      reinterpret_cast<intptr_t>(this)));
+      reinterpret_cast<intptr_t>(this),
+      is_webapk_));
 }
 
 void AppBannerInfoBarDelegateAndroid::SendBannerAccepted(
@@ -192,10 +205,17 @@ void AppBannerInfoBarDelegateAndroid::InfoBarDismissed() {
     AppBannerSettingsHelper::RecordBannerDismissEvent(
         web_contents, native_app_package_, AppBannerSettingsHelper::NATIVE);
   } else if (!manifest_.IsEmpty()) {
-    TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
-    AppBannerSettingsHelper::RecordBannerDismissEvent(
-        web_contents, manifest_.start_url.spec(),
-        AppBannerSettingsHelper::WEB);
+    if (is_webapk_) {
+        TrackUserResponse(USER_RESPONSE_WEBAPK_DISMISSED);
+        AppBannerSettingsHelper::RecordBannerDismissEvent(
+            web_contents, manifest_.start_url.spec(),
+            AppBannerSettingsHelper::WEBAPK);
+    } else {
+      TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED);
+      AppBannerSettingsHelper::RecordBannerDismissEvent(
+          web_contents, manifest_.start_url.spec(),
+          AppBannerSettingsHelper::WEB);
+    }
   }
 }
 
@@ -218,56 +238,142 @@ bool AppBannerInfoBarDelegateAndroid::Accept() {
   }
 
   if (!native_app_data_.is_null()) {
-    TrackUserResponse(USER_RESPONSE_NATIVE_APP_ACCEPTED);
-    JNIEnv* env = base::android::AttachCurrentThread();
+    return AcceptNativeApp(web_contents);
+  } else if (is_webapk_) {
+    return AcceptWebApk(web_contents);
+  }
+  return AcceptWebApp(web_contents);
+}
 
-    TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
-    if (tab == nullptr) {
-      TrackDismissEvent(DISMISS_EVENT_ERROR);
-      return true;
-    }
-    ScopedJavaLocalRef<jstring> jreferrer(
-        ConvertUTF8ToJavaString(env, referrer_));
+bool AppBannerInfoBarDelegateAndroid::AcceptNativeApp(
+    content::WebContents* web_contents) {
+  TrackUserResponse(USER_RESPONSE_NATIVE_APP_ACCEPTED);
+  JNIEnv* env = base::android::AttachCurrentThread();
 
-    bool was_opened =
-        Java_AppBannerInfoBarDelegateAndroid_installOrOpenNativeApp(
-            env, java_delegate_, tab->GetJavaObject(), native_app_data_,
-            jreferrer);
+  TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
+  if (tab == nullptr) {
+    TrackDismissEvent(DISMISS_EVENT_ERROR);
+    return true;
+  }
+  ScopedJavaLocalRef<jstring> jreferrer(
+      ConvertUTF8ToJavaString(env, referrer_));
+
+  bool was_opened =
+      Java_AppBannerInfoBarDelegateAndroid_installOrOpenNativeApp(
+          env, java_delegate_, tab->GetJavaObject(),
+          native_app_data_, jreferrer);
+
+  if (was_opened) {
+    TrackDismissEvent(DISMISS_EVENT_APP_OPEN);
+  } else {
+    TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_TRIGGERED);
+  }
+  SendBannerAccepted(web_contents, "play");
+  return was_opened;
+}
 
-    if (was_opened) {
-      TrackDismissEvent(DISMISS_EVENT_APP_OPEN);
-    } else {
-      TrackInstallEvent(INSTALL_EVENT_NATIVE_APP_INSTALL_TRIGGERED);
-    }
-    SendBannerAccepted(web_contents, "play");
-    return was_opened;
-  } else if (!manifest_.IsEmpty()) {
-    TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
+bool AppBannerInfoBarDelegateAndroid::AcceptWebApp(
+    content::WebContents* web_contents) {
+  if (manifest_.IsEmpty())
+    return true;
+  TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
+
+  AppBannerSettingsHelper::RecordBannerInstallEvent(
+      web_contents, manifest_.start_url.spec(),
+      AppBannerSettingsHelper::WEB);
+
+  if (weak_manager_) {
+    ShortcutInfo info(GURL::EmptyGURL());
+    info.UpdateFromManifest(manifest_);
+    info.manifest_url = manifest_url_;
+    info.icon_url = icon_url_;
+    info.UpdateSource(ShortcutInfo::SOURCE_APP_BANNER);
+
+    const std::string& uid = base::GenerateGUID();
+    ShortcutHelper::AddToLauncherWithSkBitmap(
+        web_contents->GetBrowserContext(), info, uid, *icon_.get(),
+        weak_manager_->FetchWebappSplashScreenImageCallback(uid));
+  }
+
+  SendBannerAccepted(web_contents, "web");
+  return true;
+}
+
+bool AppBannerInfoBarDelegateAndroid::AcceptWebApk(
+    content::WebContents* web_contents) {
+  if (manifest_.IsEmpty())
+    return true;
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  if (webapk_package_name_.empty()) {
+    // Request install the WebAPK.
+    TrackUserResponse(USER_RESPONSE_WEBAPK_ACCEPTED);
 
     AppBannerSettingsHelper::RecordBannerInstallEvent(
         web_contents, manifest_.start_url.spec(),
-        AppBannerSettingsHelper::WEB);
-
-    if (weak_manager_) {
-      ShortcutInfo info(GURL::EmptyGURL());
-      info.UpdateFromManifest(manifest_);
-      info.manifest_url = manifest_url_;
-      info.icon_url = icon_url_;
-      info.UpdateSource(ShortcutInfo::SOURCE_APP_BANNER);
-
-      const std::string& uid = base::GenerateGUID();
-      ShortcutHelper::AddToLauncherWithSkBitmap(
-          web_contents->GetBrowserContext(), info, uid, *icon_.get(),
-          weak_manager_->FetchWebappSplashScreenImageCallback(uid));
-    }
+        AppBannerSettingsHelper::WEBAPK);
+
+    ShortcutInfo info(GURL::EmptyGURL());
+    info.UpdateFromManifest(manifest_);
+    info.manifest_url = manifest_url_;
+    info.icon_url = icon_url_;
+    info.UpdateSource(ShortcutInfo::SOURCE_APP_BANNER);
+
+    Java_AppBannerInfoBarDelegateAndroid_setWebApkInstallingState(
+        env, java_delegate_, true);
+    UpdateInstallState(env, nullptr);

[pkotwicz, 2016/08/25 22:23:15]

I am not a fan of these calls. We know exactly which state we want to transition
to.

Can we call UpdateInstallState(INSTALL_STATE_INSTALLING) directly?

I know that the UpdateInstallState() call in OnWebApkInstallFinished() is more
complicated because we want to double check that the WebAPK was in fact
installed.

[Xi Han, 2016/08/26 17:04:17]

The state INSTALL_STATE_INSTALLING is defined in AppBannerInfoBarAndroid on Java
side, and the delegate always ask the infobar which is the new state, and let
the infobar update itself. The advantage is the states are only declared in one
place. Do we really want to change this design? 

[pkotwicz, 2016/08/26 22:31:24]

I didn't realize that INSTALL_STATE_INSTALLING was defined in Java. I am
uncomfortable about we have hacked
AppBannerInfoBarDelegateAndroid#determineInstallState() to work for WebAPKs but
I don't have a suggestion on how to improve things

[Xi Han, 2016/08/29 14:44:52]

I am not a fan of the way of how the state is updated, but if we want to avoid
duplicate the states on the C++ side, this is the best way that I can think of:(

+
+    WebApkInstaller::FinishCallback callback = base::Bind(
+        &AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished,
+        base::Unretained(this));

[dominickn, 2016/08/25 04:25:56]

I'm a bit nervous about this unretained call, because it's relying on the info
bar being kept alive without ref counting or weak pointers. I can't help but
wonder whether we could see some use after free issues because of this.

[Xi Han, 2016/08/26 17:04:17]

Use WeakPtrFactory instead.

+    ShortcutHelper::InstallWebApkWithSkBitmap(
+        web_contents->GetBrowserContext(), info, *icon_.get(), callback);
+    TrackInstallEvent(INSTALL_EVENT_WEBAPK_INSTALL_TRIGGERED);
 
     SendBannerAccepted(web_contents, "web");
-    return true;
+    return false;
   }
 
+  // Open the WebAPK.
+  ScopedJavaLocalRef<jstring> java_webapk_package_name =
+      base::android::ConvertUTF8ToJavaString(env, webapk_package_name_);
+  Java_AppBannerInfoBarDelegateAndroid_openWebApk(
+      env, java_delegate_, java_webapk_package_name);
+
+  SendBannerAccepted(web_contents, "webapk");
   return true;
 }
 
+void AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished(
+    bool success,
+    const std::string& webapk_package_name) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  if (!success) {
+    // The installation failed.
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&AppBannerInfoBarDelegateAndroid::RemoveInfoBarOnUIThread,
+                   base::Unretained(this)));

[dominickn, 2016/08/25 04:25:56]

See the other comment about base::Unretained.

[Xi Han, 2016/08/26 17:04:17]

Done.

+    Java_AppBannerInfoBarDelegateAndroid_showWebApkInstallFailureToast(env);
+    TrackInstallEvent(INSTALL_EVENT_WEBAPK_INSTALL_FAILED);
+    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);
+  UpdateInstallState(env, nullptr);
+}
+
+void AppBannerInfoBarDelegateAndroid::RemoveInfoBarOnUIThread() {
+    static_cast<AppBannerInfoBarAndroid*>(infobar())

[dominickn, 2016/08/25 04:25:56]

Does this need the static cast at all? It's a method on the base class.

[Xi Han, 2016/08/26 17:04:17]

Removed.

+        ->RemoveSelf();
+}
+
 bool AppBannerInfoBarDelegateAndroid::LinkClicked(
     WindowOpenDisposition disposition) {
   if (native_app_data_.is_null())