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

chrome/browser/android/webapk/webapk_installer.cc

Index: chrome/browser/android/webapk/webapk_installer.cc
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc
index 840e1147b10384bafe8f733ff94de58fd64767f1..fcf12771c04c248864bbc9e20a8a0f05caf080a5 100644
--- a/chrome/browser/android/webapk/webapk_installer.cc
+++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -111,7 +111,8 @@ std::string getCurrentAbi() {
 std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(
     const ShortcutInfo& shortcut_info,
     const SkBitmap& shortcut_icon,
-    const std::string& shortcut_icon_murmur2_hash) {
+    bool stale_manifest,

[dominickn, 2016/12/20 05:05:49]

is_manifest_stale

[Xi Han, 2016/12/20 20:25:09]

Done.

+    std::map<std::string, std::string> icon_url_to_murmur2_hash_map) {

[dominickn, 2016/12/20 05:05:48]

const std::map<std::string, std::string>&

Also call it icon_url_to_murmur2_map (std::map is not a hash map)

[Xi Han, 2016/12/20 20:25:09]

Done.
Yes, it is a little bit confusing. The "murmur2_hash" is together, so it is a
"icon_url" to "murmur2_hash" map.

[dominickn, 2016/12/21 02:51:49]

We should try and eliminate this confusion. An alternative suggestion is to just
call the variable icon_url_to_murmur2_hash, since the type already has map in
it.

[Xi Han, 2016/12/21 22:45:44]

Done.

   DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
 
   std::unique_ptr<webapk::WebApk> webapk(new webapk::WebApk);
@@ -120,11 +121,7 @@ std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(
       base::android::BuildInfo::GetInstance()->package_name());
   webapk->set_requester_application_version(version_info::GetVersionNumber());
   webapk->set_android_abi(getCurrentAbi());
-
-  // TODO(hanxi): crbug.com/665078. Add a flag in WebAPK's proto to indicate
-  // that the Web Manifest data in the proto might be stale.
-  if (shortcut_icon_murmur2_hash.empty())
-    return webapk;
+  webapk->set_stale_manifest(stale_manifest);
 
   webapk::WebAppManifest* web_app_manifest = webapk->mutable_manifest();
   web_app_manifest->set_name(base::UTF16ToUTF8(shortcut_info.name));
@@ -144,22 +141,35 @@ std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(
   scope->assign(GetScope(shortcut_info).spec());
 
   webapk::Image* best_image = web_app_manifest->add_icons();
-  best_image->set_src(shortcut_info.best_icon_url.spec());
-  best_image->set_hash(shortcut_icon_murmur2_hash);
+  std::string best_icon_url = shortcut_info.best_icon_url.spec();
+  best_image->set_src(best_icon_url);
+  std::map<std::string, std::string>::iterator it =

[dominickn, 2016/12/20 05:05:48]

Does auto it work here?

[Xi Han, 2016/12/20 20:25:08]

Done.

+      icon_url_to_murmur2_hash_map.find(best_icon_url);
+  if (it != icon_url_to_murmur2_hash_map.end())
+    best_image->set_hash(it->second);
   std::vector<unsigned char> png_bytes;
   gfx::PNGCodec::EncodeBGRASkBitmap(shortcut_icon, false, &png_bytes);
   best_image->set_image_data(&png_bytes.front(), png_bytes.size());
 
-  for (const std::string& icon_url : shortcut_info.icon_urls) {
-    if (icon_url == shortcut_info.best_icon_url.spec())
+  for (const std::pair<std::string, std::string>& entry :

[dominickn, 2016/12/20 05:05:48]

const auto& entry?

[Xi Han, 2016/12/20 20:25:09]

Done.

+       icon_url_to_murmur2_hash_map) {
+    if (entry.first == shortcut_info.best_icon_url.spec())
       continue;
     webapk::Image* image = web_app_manifest->add_icons();
-    image->set_src(icon_url);
+    image->set_src(entry.first);
+    image->set_hash(entry.second);
   }
 
   return webapk;
 }
 
+// Calls the callback when the |webapk| request is created.
+void OnWebApkProtoBuilt(
+    const base::Callback<void(std::unique_ptr<webapk::WebApk>)>& callback,
+    std::unique_ptr<webapk::WebApk> webapk) {
+  callback.Run(std::move(webapk));
+}
+
 // Returns task runner for running background tasks.
 scoped_refptr<base::TaskRunner> GetBackgroundTaskRunner() {
   return content::BrowserThread::GetBlockingPool()
@@ -250,33 +260,36 @@ void WebApkInstaller::InstallAsyncWithURLRequestContextGetter(
   DownloadAppIconAndComputeMurmur2Hash();
 }
 
-void WebApkInstaller::UpdateAsync(content::BrowserContext* browser_context,
-                                  const FinishCallback& finish_callback,
-                                  const std::string& icon_murmur2_hash,
-                                  const std::string& webapk_package,
-                                  int webapk_version) {
+void WebApkInstaller::UpdateAsync(
+    content::BrowserContext* browser_context,
+    const FinishCallback& finish_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) {

[dominickn, 2016/12/20 05:05:48]

icon_url_to_murmur2_map (std::map is not a hash map)

[Xi Han, 2016/12/20 20:25:09]

same.

   UpdateAsyncWithURLRequestContextGetter(
       Profile::FromBrowserContext(browser_context)->GetRequestContext(),
-      finish_callback, icon_murmur2_hash, webapk_package, webapk_version);
+      finish_callback, webapk_package, webapk_version, stale_manifest,
+      icon_url_to_murmur2_hash_map);
 }
 
 void WebApkInstaller::UpdateAsyncWithURLRequestContextGetter(
     net::URLRequestContextGetter* request_context_getter,
     const FinishCallback& finish_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) {

[dominickn, 2016/12/20 05:05:49]

icon_url_to_murmur2_map (std::map is not a hash map)

[Xi Han, 2016/12/20 20:25:09]

same.

   request_context_getter_ = request_context_getter;
   finish_callback_ = finish_callback;
-  shortcut_icon_murmur2_hash_ = icon_murmur2_hash;
   webapk_package_ = webapk_package;
   webapk_version_ = webapk_version;
   task_type_ = UPDATE;
 
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_,
-                  shortcut_icon_, shortcut_icon_murmur2_hash_),
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
+                 stale_manifest, icon_url_to_murmur2_hash_map),
       base::Bind(&WebApkInstaller::SendUpdateWebApkRequest,
                   weak_ptr_factory_.GetWeakPtr()));
 }
@@ -296,6 +309,17 @@ void WebApkInstaller::OnInstallFinished(
     OnFailure();
 }
 
+void WebApkInstaller::BuildWebApkProtoInBackgroundForTesting(
+    const base::Callback<void(std::unique_ptr<webapk::WebApk>)>& callback,
+    bool stale_manifest,

[dominickn, 2016/12/20 05:05:48]

is_manifest_stale

[Xi Han, 2016/12/20 20:25:09]

Done.

+    const std::map<std::string, std::string>& icon_url_to_murmur2_hash_map) {

[dominickn, 2016/12/20 05:05:48]

icon_url_to_murmur2_map (std::map is not a hash map)

[Xi Han, 2016/12/20 20:25:09]

same.

+  base::PostTaskAndReplyWithResult(
+      GetBackgroundTaskRunner().get(), FROM_HERE,
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
+                 stale_manifest, icon_url_to_murmur2_hash_map),
+      base::Bind(&OnWebApkProtoBuilt, callback));
+}
+
 // static
 bool WebApkInstaller::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
@@ -410,18 +434,24 @@ void WebApkInstaller::OnGotIconMurmur2Hash(
   timer_.Stop();
   icon_hasher_.reset();
 
-  shortcut_icon_murmur2_hash_ = icon_murmur2_hash;
-
   // An empty hash indicates that |icon_hasher_| encountered an error.
   if (icon_murmur2_hash.empty()) {
     OnFailure();
     return;
   }
 
+  std::map<std::string, std::string> icon_url_to_murmur2_hash_map;

[dominickn, 2016/12/20 05:05:49]

Minor nit: call this icon_url_to_murmur2_map. std::map isn't a hash map, it's a
red-black tree.

[Xi Han, 2016/12/20 20:25:09]

same.

+  for (const std::string& icon_url : shortcut_info_.icon_urls) {
+    if (icon_url != shortcut_info_.best_icon_url.spec())
+      icon_url_to_murmur2_hash_map[icon_url] = "";
+    else
+      icon_url_to_murmur2_hash_map[icon_url] = icon_murmur2_hash;
+  }
+
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_,
-                  shortcut_icon_, shortcut_icon_murmur2_hash_),
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
+                 false, icon_url_to_murmur2_hash_map),
       base::Bind(&WebApkInstaller::SendCreateWebApkRequest,
                  weak_ptr_factory_.GetWeakPtr()));
 }