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

chrome/browser/android/webapk/webapk_install_service.h

Index: chrome/browser/android/webapk/webapk_install_service.h
diff --git a/chrome/browser/android/webapk/webapk_install_service.h b/chrome/browser/android/webapk/webapk_install_service.h
index e22dc9aea4339ce035f0c2e10f3bda3a1b8aec69..3dfe7ba0b9b4ec004fc991397c67ae2e9b3be8fe 100644
--- a/chrome/browser/android/webapk/webapk_install_service.h
+++ b/chrome/browser/android/webapk/webapk_install_service.h
@@ -22,6 +22,21 @@ class BrowserContext;
 struct ShortcutInfo;
 class SkBitmap;
 
+// A Java counterpart will be generated for this enum.
+// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser.webapps
+enum class WebApkInstallResult {
+  // For installs: this indicates that the WebAPK has been installed.
+  // For updates: this indicates that the update request has been sent to Google
+  // Play (or the Android OS in the case of the "unknown sources" flow).
+  INSTALLED = 0,

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

SUCCESS ("Install" is redundant)

+  INSTALL_FAILED = 1,

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

FAILURE

+  // An install was initiated but it timed out. We did not get a response from
+  // either Google Play (or the Android OS in the case of the "unknown sources"
+  // flow) so it is possible that the install will complete some time in the
+  // future.
+  INSTALL_PROBABLY_FAILED = 2

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

TIMEOUT (you can add new result values in future for different "probable
failures")

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

I prefer keeping this as INSTALL_PROBABLY_FAILED. In the case of some of the
time outs during the install flow (e.g. WebApkInstaller::SendRequest()) we know
for sure that the install failed.

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

I don't quite understand your reasoning - WebApkInstaller::SendRequest uses the
FAILURE code?

The comment here explicitly says that the install timed out, so why not call it
TIMEOUT?

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

I prefer keeping the PROBABLY_FAILURE because we want to send the
PROBABLY_FAILURE status for some but not all timeouts. In particular, we want to
send FAILURE instead of PROBABLY_FAILURE when the |WebApkInstaller::timer_|
fires

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

I don't want to block on this. Please rename it to PROBABLE_FAILURE (not
PROBABLY) and we'll leave it there.

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

Thanks!

+};
+
 // Service which talks to Chrome WebAPK server and Google Play to generate a
 // WebAPK on the server, download it, and install it.
 class WebApkInstallService : public KeyedService {
@@ -30,7 +45,8 @@ class WebApkInstallService : public KeyedService {
   // Parameters:
   // - whether the process succeeds.
   // - the package name of the WebAPK.
-  using FinishCallback = base::Callback<void(bool, const std::string&)>;
+  using FinishCallback =
+      base::Callback<void(WebApkInstallResult, const std::string&)>;
 
   static WebApkInstallService* Get(content::BrowserContext* browser_context);
 
@@ -63,7 +79,7 @@ class WebApkInstallService : public KeyedService {
   // Called once the install/update completed or failed.
   void OnFinishedInstall(const GURL& web_manifest_url,
                          const FinishCallback& finish_callback,
-                         bool success,
+                         WebApkInstallResult result,
                          const std::string& webapk_package_name);
 
   content::BrowserContext* browser_context_;