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

chrome/browser/android/webapk/webapk_installer.cc

 // Copyright 2016 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/webapk/webapk_installer.h"
 
 #include "base/android/build_info.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/path_utils.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/android/shortcut_helper.h"
 #include "chrome/browser/android/webapk/webapk.pb.h"
 #include "chrome/browser/android/webapk/webapk_icon_hasher.h"
+#include "chrome/browser/android/webapk/webapk_install_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/manifest_util.h"
 #include "jni/WebApkInstaller_jni.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_fetcher.h"
@@ skipping 231 matching lines @@
 }
 
 void WebApkInstaller::SetTimeoutMs(int timeout_ms) {
   webapk_download_url_timeout_ms_ = timeout_ms;
   download_timeout_ms_ = timeout_ms;
 }
 
 void WebApkInstaller::OnInstallFinished(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
-    jboolean success) {
-  if (success)
-    OnSuccess();
-  else
-    OnFailure();
+    jint result) {
+  OnResult(static_cast<WebApkInstallResult>(result));
 }
 
 void WebApkInstaller::BuildWebApkProtoInBackgroundForTesting(
     const base::Callback<void(std::unique_ptr<webapk::WebApk>)>& callback,
     const std::map<std::string, std::string>& icon_url_to_murmur2_hash,
     bool is_manifest_stale) {
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
       base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
                  icon_url_to_murmur2_hash, is_manifest_stale),
       base::Bind(&OnWebApkProtoBuilt, callback));
 }
 
 // static
 bool WebApkInstaller::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 bool WebApkInstaller::StartInstallingDownloadedWebApk(
     JNIEnv* env,
     const base::android::ScopedJavaLocalRef<jstring>& java_file_path,
     const base::android::ScopedJavaLocalRef<jstring>& java_package_name) {
   return Java_WebApkInstaller_installAsyncAndMonitorInstallationFromNative(
       env, java_ref_, java_file_path, java_package_name);
 }
 
 bool WebApkInstaller::StartUpdateUsingDownloadedWebApk(
     JNIEnv* env,
     const base::android::ScopedJavaLocalRef<jstring>& java_file_path) {
   return Java_WebApkInstaller_updateAsyncFromNative(env, java_ref_,

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

Can updates also use WebApkInstallResult instead of a boolean for consistency?
Then they can also just call OnResult(result)

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

I have addressed this concern in https://codereview.chromium.org/2746723002/
This CL now depends on that CL

                                                     java_file_path);
 }
 
 bool WebApkInstaller::CanUseGooglePlayInstallService() {
   JNIEnv* env = base::android::AttachCurrentThread();
   return Java_WebApkInstaller_canUseGooglePlayInstallService(env, java_ref_);
 }
 
 void WebApkInstaller::InstallOrUpdateWebApkFromGooglePlay(
     const std::string& package_name,
@@ skipping 15 matching lines @@
     Java_WebApkInstaller_installWebApkFromGooglePlayAsync(
         env, java_ref_, java_webapk_package, version, java_title, java_token,
         java_url);
   } else {
     Java_WebApkInstaller_updateAsyncFromGooglePlay(
         env, java_ref_, java_webapk_package, version, java_title, java_token,
         java_url);
   }
 }
 
+void WebApkInstaller::OnSuccess() {

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

Can you remove this method and just call OnResult directly?

+  OnResult(WebApkInstallResult::INSTALLED);
+}
+
 WebApkInstaller::WebApkInstaller(content::BrowserContext* browser_context,
                                  const ShortcutInfo& shortcut_info,
                                  const SkBitmap& shortcut_icon)
     : request_context_getter_(
           Profile::FromBrowserContext(browser_context)->GetRequestContext()),
       shortcut_info_(shortcut_info),
       shortcut_icon_(shortcut_icon),
       server_url_(GetServerUrl()),
       webapk_download_url_timeout_ms_(kWebApkDownloadUrlTimeoutMs),
       download_timeout_ms_(kDownloadTimeoutMs),
@@ skipping 263 matching lines @@
     }
   }
   if (!success)
     OnFailure();
 }
 
 void WebApkInstaller::OnTimeout() {
   OnFailure();
 }
 
-void WebApkInstaller::OnSuccess() {
-  finish_callback_.Run(true, webapk_package_);
+void WebApkInstaller::OnFailure() {

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

Can you remove this method and call OnResult directly?

+  OnResult(WebApkInstallResult::INSTALL_FAILED);
+}
+
+void WebApkInstaller::OnResult(WebApkInstallResult result) {
+  finish_callback_.Run(result, webapk_package_);
   delete this;
 }
-
-void WebApkInstaller::OnFailure() {
-  finish_callback_.Run(false, webapk_package_);
-  delete this;
-}