[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/chrome_webapk_host.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 228 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 webApkInstallResult) {
+  OnResult(static_cast<WebApkInstallResult>(webApkInstallResult));
 }
 
 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),
@@ skipping 32 matching lines @@
   base::android::ScopedJavaLocalRef<jstring> java_token =
       base::android::ConvertUTF8ToJavaString(env, token);
   base::android::ScopedJavaLocalRef<jstring> java_url =
       base::android::ConvertUTF8ToJavaString(env, shortcut_info_.url.spec());
 
   Java_WebApkInstaller_installOrUpdateWebApkFromGooglePlayAsync(
       env, java_ref_, task_type_ == UPDATE, java_webapk_package, version,
       java_title, java_token, java_url);
 }
 
+void WebApkInstaller::OnResult(WebApkInstallResult result) {
+  finish_callback_.Run(result, relax_updates_, webapk_package_);

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

Nit: this should call weak_ptr_factory_.InvalidateWeakPtrs() first to make sure
no callbacks will be invoked.

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

Doesn't the destructor call WeakPtrFactory::InvalidateWeakPtrs() implicitly?

[dominickn, 2017/03/14 23:25:19]

Yes, but finish_callback_ could potentially be bound to something that triggers
one of the callbacks, or it could do some blocking work on a different thread
that allows one of the weak pointer calls to squeeze in before the destructor is
run. Invalidating the weak pointers first here makes sure none of that will
happen.

[pkotwicz, 2017/03/14 23:43:29]

Fair enough

+  delete this;
+}
+
 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 44 matching lines @@
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebApkInstaller::OnURLFetchComplete(const net::URLFetcher* source) {
   timer_.Stop();
 
   if (!source->GetStatus().is_success() ||
       source->GetResponseCode() != net::HTTP_OK) {
     LOG(WARNING) << base::StringPrintf(
         "WebAPK server returned response code %d.", source->GetResponseCode());
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   std::string response_string;
   source->GetResponseAsString(&response_string);
 
   std::unique_ptr<webapk::WebApkResponse> response(new webapk::WebApkResponse);
   if (!response->ParseFromString(response_string)) {
     LOG(WARNING) << "WebAPK server did not return proto.";
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   GURL signed_download_url(response->signed_download_url());
   if (task_type_ == UPDATE && signed_download_url.is_empty()) {
     // https://crbug.com/680131. The server sends an empty URL if the server
     // does not have a newer WebAPK to update to.
     relax_updates_ = response->relax_updates();
-    OnSuccess();
+    OnResult(WebApkInstallResult::SUCCESS);
     return;
   }
 
   if (!signed_download_url.is_valid() || response->package_name().empty()) {
     LOG(WARNING) << "WebAPK server returned incomplete proto.";
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   if (CanUseGooglePlayInstallService()) {
     int version = 1;
     base::StringToInt(response->version(), &version);
     InstallOrUpdateWebApkFromGooglePlay(response->package_name(), version,
                                         response->token());
     return;
   }
 
   OnGotWebApkDownloadUrl(signed_download_url, response->package_name());
 }
 
 void WebApkInstaller::DownloadAppIconAndComputeMurmur2Hash() {
   // Safeguard. WebApkIconHasher crashes if asked to fetch an invalid URL.
   if (!shortcut_info_.best_primary_icon_url.is_valid()) {
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   timer_.Start(
       FROM_HERE, base::TimeDelta::FromMilliseconds(download_timeout_ms_),
-      base::Bind(&WebApkInstaller::OnTimeout, weak_ptr_factory_.GetWeakPtr()));
+      base::Bind(&WebApkInstaller::OnResult, weak_ptr_factory_.GetWeakPtr(),
+                 WebApkInstallResult::FAILURE));
 
   icon_hasher_.reset(new WebApkIconHasher());
   icon_hasher_->DownloadAndComputeMurmur2Hash(
       request_context_getter_, shortcut_info_.best_primary_icon_url,
       base::Bind(&WebApkInstaller::OnGotIconMurmur2Hash,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebApkInstaller::OnGotIconMurmur2Hash(
     const std::string& icon_murmur2_hash) {
   timer_.Stop();
   icon_hasher_.reset();
 
   // An empty hash indicates that |icon_hasher_| encountered an error.
   if (icon_murmur2_hash.empty()) {
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   std::map<std::string, std::string> icon_url_to_murmur2_hash;
   for (const std::string& icon_url : shortcut_info_.icon_urls) {
     if (icon_url != shortcut_info_.best_primary_icon_url.spec())
       icon_url_to_murmur2_hash[icon_url] = "";
     else
       icon_url_to_murmur2_hash[icon_url] = icon_murmur2_hash;
   }
@@ skipping 17 matching lines @@
   webapk->set_version(std::to_string(webapk_version_));
 
   SendRequest(std::move(webapk), server_url_);
 }
 
 void WebApkInstaller::SendRequest(std::unique_ptr<webapk::WebApk> request_proto,
                                   const GURL& server_url) {
   timer_.Start(
       FROM_HERE,
       base::TimeDelta::FromMilliseconds(webapk_download_url_timeout_ms_),
-      base::Bind(&WebApkInstaller::OnTimeout, weak_ptr_factory_.GetWeakPtr()));
+      base::Bind(&WebApkInstaller::OnResult, weak_ptr_factory_.GetWeakPtr(),
+                 WebApkInstallResult::FAILURE));
 
   url_fetcher_ =
       net::URLFetcher::Create(server_url, net::URLFetcher::POST, this);
   url_fetcher_->SetRequestContext(request_context_getter_);
   std::string serialized_request;
   request_proto->SerializeToString(&serialized_request);
   url_fetcher_->SetUploadData(kProtoMimeType, serialized_request);
   url_fetcher_->SetLoadFlags(
       net::LOAD_DISABLE_CACHE | net::LOAD_DO_NOT_SEND_COOKIES |
       net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_AUTH_DATA);
@@ skipping 10 matching lines @@
                  task_type_ == WebApkInstaller::INSTALL ? "install" : "update",
                  package_name),
       base::Bind(&WebApkInstaller::OnCreatedSubDirAndSetPermissions,
                  weak_ptr_factory_.GetWeakPtr(), download_url));
 }
 
 void WebApkInstaller::OnCreatedSubDirAndSetPermissions(
     const GURL& download_url,
     const base::FilePath& output_dir) {
   if (output_dir.empty()) {
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   DownloadWebApk(output_dir.AppendASCII(webapk_package_ + ".apk"), download_url,
                  true);
 }
 
 void WebApkInstaller::DownloadWebApk(const base::FilePath& output_path,
                                      const GURL& download_url,
                                      bool retry_if_fails) {
   timer_.Start(
       FROM_HERE, base::TimeDelta::FromMilliseconds(download_timeout_ms_),
-      base::Bind(&WebApkInstaller::OnTimeout, weak_ptr_factory_.GetWeakPtr()));
+      base::Bind(&WebApkInstaller::OnResult, weak_ptr_factory_.GetWeakPtr(),
+                 WebApkInstallResult::FAILURE));
 
   downloader_.reset(new FileDownloader(
       download_url, output_path, true, request_context_getter_,
       base::Bind(&WebApkInstaller::OnWebApkDownloaded,
                  weak_ptr_factory_.GetWeakPtr(), output_path, download_url,
                  retry_if_fails),
       NO_TRAFFIC_ANNOTATION_YET));
 }
 
 void WebApkInstaller::OnWebApkDownloaded(const base::FilePath& file_path,
                                          const GURL& download_url,
                                          bool retry_if_fails,
                                          FileDownloader::Result result) {
   timer_.Stop();
 
   if (result != FileDownloader::DOWNLOADED) {
     if (!retry_if_fails) {
-      OnFailure();
+      OnResult(WebApkInstallResult::FAILURE);
       return;
     }
 
     content::BrowserThread::PostDelayedTask(
         content::BrowserThread::UI, FROM_HERE,
         base::Bind(&WebApkInstaller::DownloadWebApk,
                    weak_ptr_factory_.GetWeakPtr(), file_path, download_url,
                    false),
         base::TimeDelta::FromSeconds(2));
     return;
   }
 
   int posix_permissions = kWorldReadableFilePermission |
                           base::FILE_PERMISSION_WRITE_BY_USER |
                           base::FILE_PERMISSION_EXECUTE_BY_USER;
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
       base::Bind(&base::SetPosixFilePermissions, file_path, posix_permissions),
       base::Bind(&WebApkInstaller::OnWebApkMadeWorldReadable,
                  weak_ptr_factory_.GetWeakPtr(), file_path));
 }
 
 void WebApkInstaller::OnWebApkMadeWorldReadable(
     const base::FilePath& file_path,
     bool change_permission_success) {
   if (!change_permission_success) {
-    OnFailure();
+    OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jstring> java_file_path =
       base::android::ConvertUTF8ToJavaString(env, file_path.value());
   base::android::ScopedJavaLocalRef<jstring> java_package_name =
       base::android::ConvertUTF8ToJavaString(env, webapk_package_);
   InstallOrUpdateDownloadedWebApk(env, java_file_path, java_package_name);
 }
-
-void WebApkInstaller::OnTimeout() {
-  OnFailure();
-}
-
-void WebApkInstaller::OnSuccess() {
-  finish_callback_.Run(true, relax_updates_, webapk_package_);
-  delete this;
-}
-
-void WebApkInstaller::OnFailure() {
-  finish_callback_.Run(false, relax_updates_, webapk_package_);
-  delete this;
-}