Add a flag in WebAPK's proto when the Web App Manifest is no longer available.

chrome/browser/android/webapk/webapk_installer.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_INSTALLER_H_
 #define CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_INSTALLER_H_
 
 #include <jni.h>
+#include <map>
 #include <memory>
 
 #include "base/android/scoped_java_ref.h"
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/android/shortcut_info.h"
 #include "chrome/browser/net/file_downloader.h"
 #include "net/url_request/url_fetcher.h"
@@ skipping 37 matching lines @@
                     const FinishCallback& finish_callback);
 
   // Same as InstallAsync() but uses the passed in |request_context_getter|.
   void InstallAsyncWithURLRequestContextGetter(
       net::URLRequestContextGetter* request_context_getter,
       const FinishCallback& finish_callback);
 
   // Talks to the Chrome WebAPK server to update a WebAPK on the server and to
   // the Google Play server to install the downloaded WebAPK. Calls |callback|
   // after the request to install the WebAPK is sent to the Google Play server.
-  void UpdateAsync(content::BrowserContext* browser_context,
-                   const FinishCallback& callback,
-                   const std::string& icon_murmur2_hash,
-                   const std::string& webapk_package,
-                   int webapk_version);
+  void UpdateAsync(
+      content::BrowserContext* browser_context,
+      const FinishCallback& callback,
+      const std::string& webapk_package,
+      int webapk_version,
+      bool stale_manifest,
+      const std::map<std::string, std::string>& icon_url_to_murmur2_hash_map);
 
   // Same as UpdateAsync() but uses the passed in |request_context_getter|.
   void UpdateAsyncWithURLRequestContextGetter(
       net::URLRequestContextGetter* request_context_getter,
       const FinishCallback& callback,
-      const std::string& icon_murmur2_hash,
       const std::string& webapk_package,
-      int webapk_version);
+      int webapk_version,
+      bool stale_manifest,
+      const std::map<std::string, std::string>& icon_url_to_murmur2_hash_map);
 
   // Sets the timeout for the server requests.
   void SetTimeoutMs(int timeout_ms);
 
   // Called once the installation is complete or failed.
   void OnInstallFinished(JNIEnv* env,
                          const base::android::JavaParamRef<jobject>& obj,
                          jboolean success);
 
   // Registers JNI hooks.
@@ skipping 25 matching lines @@
   // or update request is handled by Google Play.
   virtual bool InstallOrUpdateWebApkFromGooglePlay(
       const std::string& package_name,
       int version,
       const std::string& token);
 
   // Called when the request to install the WebAPK is sent to Google Play.
   void OnSuccess();
 
  private:
+  friend class WebApkInstallerRunner;
+
   enum TaskType {
     UNDEFINED,
     INSTALL,
     UPDATE,
   };
 
   // Create the Java object.
   void CreateJavaRef();
 
   // net::URLFetcherDelegate:
@@ skipping 57 matching lines @@
   // Called when the request to the WebAPK server times out or when the WebAPK
   // download times out.
   void OnTimeout();
 
   // Called if a WebAPK could not be created. WebApkInstaller only tracks the
   // WebAPK creation and the WebAPK download. It does not track the
   // WebAPK installation. OnFailure() is not called if the WebAPK could not be
   // installed.
   void OnFailure();
 
+  // Populates webapk::WebApk and returns it.
+  // Must be called on a worker thread because it encodes an SkBitmap.
+  static std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(

[dominickn, 2016/12/15 03:33:40]

A private static method with a friended test is very ugly. Is there a way to
restructure this? Perhaps Peter's suggestion will let you avoid having to call
this method outside this class.

[pkotwicz, 2016/12/15 04:21:52]

Dominick, how do you suggest making this cleaner. One option would be to make
this method public static

I was the one to suggest a private static method with a friended test class.

I know that friending a test class is a common pattern for testing code (or at
least it used to be.) For instance HistoryBackendTest and HistoryBackend.
The other pattern I know of is having an inner TestApi class.
ScopedCaptureClient::TestApi for instance

[dominickn, 2016/12/15 04:30:35]

Friending a test class is fine - what's ugly here is that the only reason this
method is private static is because it's called explicitly by the test class. It
would be better if this method didn't have to be called by the tester, and
instead the tester called a member method which then called this (so it can be
in the anonymous namespace again).

Otherwise, if this functionality is actually static and could be called by
multiple places legitimately (other than the test), then you should put it in
the webapk namespace as a top level method.

+      const ShortcutInfo& shortcut_info,
+      const SkBitmap& shortcut_icon,
+      bool stale_manifest,
+      std::map<std::string, std::string> icon_url_to_murmur2_hash_map);

[dominickn, 2016/12/15 03:33:40]

Make this a const reference.

+
   net::URLRequestContextGetter* request_context_getter_;
 
   // Sends HTTP request to WebAPK server.
   std::unique_ptr<net::URLFetcher> url_fetcher_;
 
   // Downloads app icon and computes Murmur2 hash.
   std::unique_ptr<WebApkIconHasher> icon_hasher_;
 
   // Downloads WebAPK.
   std::unique_ptr<FileDownloader> downloader_;
 
   // Fails WebApkInstaller if WebAPK server takes too long to respond or if the
   // download takes too long.
   base::OneShotTimer timer_;
 
   // Callback to call once WebApkInstaller succeeds or fails.
   FinishCallback finish_callback_;
 
   // Web Manifest info.
   const ShortcutInfo shortcut_info_;
 
   // WebAPK app icon.
   const SkBitmap shortcut_icon_;
 
-  // Murmur2 hash of the bitmap at the app icon URL prior to any transformations
-  // being applied to the bitmap (such as encoding/decoding the icon bitmap).
-  std::string shortcut_icon_murmur2_hash_;
-
   // WebAPK server URL.
   GURL server_url_;
 
   // The number of milliseconds to wait for the WebAPK download URL from the
   // WebAPK server.
   int webapk_download_url_timeout_ms_;
 
   // The number of milliseconds to wait for the WebAPK download to complete.
   int download_timeout_ms_;
 
   // WebAPK package name.
   std::string webapk_package_;
 
   // WebAPK version code.
   int webapk_version_;
 
   // Indicates whether the installer is for installing or updating a WebAPK.
   TaskType task_type_;
 
   // Points to the Java Object.
   base::android::ScopedJavaGlobalRef<jobject> java_ref_;
 
   // Used to get |weak_ptr_|.
   base::WeakPtrFactory<WebApkInstaller> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WebApkInstaller);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_WEBAPK_WEBAPK_INSTALLER_H_