Make AppBannerInfoBar install WebAPK.

chrome/browser/android/webapk/webapk_installer.cc

Index: chrome/browser/android/webapk/webapk_installer.cc
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc
index e01f9822b29355ced06abc2bdbfecb0e2ed4287e..beef1f2df699cb2936f6d55894ef42ae1919a4f4 100644
--- a/chrome/browser/android/webapk/webapk_installer.cc
+++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -143,9 +143,20 @@ WebApkInstaller::WebApkInstaller(const ShortcutInfo& shortcut_info,
       GURL(command_line->HasSwitch(switches::kWebApkServerUrl)
                ? command_line->GetSwitchValueASCII(switches::kWebApkServerUrl)
                : kDefaultWebApkServerUrl);
+  CreateJavaRef();
 }
 
-WebApkInstaller::~WebApkInstaller() {}
+void WebApkInstaller::CreateJavaRef() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  java_ref_.Reset(Java_WebApkInstaller_create(
+      env, reinterpret_cast<intptr_t>(this)));
+}
+
+WebApkInstaller::~WebApkInstaller() {

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

Noob question: Is calling WebApkInstaller#destroy() necessary?

(I know that C++ objects owned by Java objects need to be destroyed explicitly
but I am wondering whether Java objects owned by C++ objects are destroyed
automatically via the ScopedJavaGlobalRef destructor)

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

That is a good point. The ScopedJavaGlobalRef destructor will reset the Java
pointer in c++, but it doesn't set the native pointer in Jave to 0. I am not
sure whether that matters. AppBannerInfoBarDelegateAndroid is an example that
Java object owned by c++, and it does have a destroy function in Java. I found
ResourceManager(https://cs.chromium.org/chromium/src/ui/android/java/src/org/chromium/ui/resources/ResourceManager.java?rcl=0&l=180)
is created and destroy in the same way as I did.  

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

Thank you for looking into this. I learned something today!

+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_WebApkInstaller_destroy(env, java_ref_);
+  java_ref_.Reset();
+}
 
 void WebApkInstaller::InstallAsync(content::BrowserContext* browser_context,
                                    const FinishCallback& finish_callback) {
@@ -212,16 +223,16 @@ bool WebApkInstaller::StartInstallingDownloadedWebApk(
     JNIEnv* env,
     const base::android::ScopedJavaLocalRef<jstring>& java_file_path,
     const base::android::ScopedJavaLocalRef<jstring>& java_package_name) {
-  return Java_WebApkInstaller_installAsyncFromNative(env, java_file_path,
-                                                     java_package_name);
+  return Java_WebApkInstaller_installAsyncFromNative(
+      env, java_ref_, java_file_path, java_package_name);
 }
 
 bool WebApkInstaller::StartUpdateUsingDownloadedWebApk(
     JNIEnv* env,
     const base::android::ScopedJavaLocalRef<jstring>& java_file_path,
     const base::android::ScopedJavaLocalRef<jstring>& java_package_name) {
-  return Java_WebApkInstaller_updateAsyncFromNative(env, java_file_path,
-                                                    java_package_name);
+  return Java_WebApkInstaller_updateAsyncFromNative(
+      env, java_ref_, java_file_path, java_package_name);
 }
 
 void WebApkInstaller::OnURLFetchComplete(const net::URLFetcher* source) {
@@ -287,7 +298,7 @@ void WebApkInstaller::OnGotIconMurmur2Hash(
       base::Bind(&BuildWebApkProtoInBackground, shortcut_info_,
                   shortcut_icon_, shortcut_icon_murmur2_hash_),
       base::Bind(&WebApkInstaller::SendCreateWebApkRequest,
-                  weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebApkInstaller::SendCreateWebApkRequest(
@@ -377,12 +388,27 @@ void WebApkInstaller::OnWebApkMadeWorldReadable(
       base::android::ConvertUTF8ToJavaString(env, package_name);
   bool success = false;
   if (task_type_ == INSTALL) {
+    webapk_package_ = package_name;
     success = StartInstallingDownloadedWebApk(env, java_file_path,
                                               java_package_name);
-  } else if (task_type_ == UPDATE) {
+    if (success)
+      return;
+  }
+  if (task_type_ == UPDATE) {
     success = StartUpdateUsingDownloadedWebApk(env, java_file_path,
                                                java_package_name);

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

For updates you should call onSuccess() only once the WebAPK has been updated.
You can detect whether a WebAPK has been updated by checking the Android version
code of the installed WebAPK. For now you can keep the stub in
WebApkInstaller#updateAsyncFromNative()

For the sake of clarity I am suggesting:

if (task_type_ == INSTALL) {
  success = StartInstallingDownloadedWebApk(env, java_file_path,
java_package_name);
} else if (task_type_ == UPDATE) {
  success = StartUpdateUsingDownloadedWebApk(env, java_file_path,
java_package_name);
}

if (!success)
  OnFailure();

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

Yes, the update will also get a callback for the real success. Updated.

   }
+  OnInstallFinished(success);
+}
+
+void WebApkInstaller::OnInstallFinished(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj,
+    jboolean success) {
+  OnInstallFinished(success);
+}
+
+void WebApkInstaller::OnInstallFinished(bool success) {
   if (success)
     OnSuccess();
   else
@@ -396,11 +422,16 @@ void WebApkInstaller::OnTimeout() {
 void WebApkInstaller::OnSuccess() {
   FinishCallback callback = finish_callback_;
   delete this;
-  callback.Run(true);
+  callback.Run(true, webapk_package_);
 }
 
 void WebApkInstaller::OnFailure() {
   FinishCallback callback = finish_callback_;
   delete this;
-  callback.Run(false);
+  callback.Run(false, "");
+}
+
+// static
+bool WebApkInstaller::Register(JNIEnv* env) {
+  return RegisterNativesImpl(env);
 }