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

chrome/browser/android/webapk/webapk_install_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_INSTALL_SERVICE_H_
 #define CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_INSTALL_SERVICE_H_
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "url/gurl.h"
 
 namespace content {
 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 {
  public:
   // Called when the creation/updating of a WebAPK is finished or failed.
   // 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);
 
   explicit WebApkInstallService(content::BrowserContext* browser_context);
   ~WebApkInstallService() override;
 
   // Returns whether an install for |web_manifest_url| is in progress.
   bool IsInstallInProgress(const GURL& web_manifest_url);
 
   // Talks to the Chrome WebAPK server to generate a WebAPK on the server and to
@@ skipping 12 matching lines @@
       const std::string& webapk_package,
       int webapk_version,
       const std::map<std::string, std::string>& icon_url_to_murmur2_hash,
       bool is_manifest_stale,
       const FinishCallback& finish_callback);
 
  private:
   // 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_;
 
   // In progress installs.
   std::set<GURL> installs_;
 
   // Used to get |weak_ptr_|.
   base::WeakPtrFactory<WebApkInstallService> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WebApkInstallService);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_INSTALL_SERVICE_H_