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

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"
@@ skipping 104 matching lines @@
     const std::string& shortcut_icon_murmur2_hash) {
   DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
 
   std::unique_ptr<webapk::WebApk> webapk(new webapk::WebApk);
   webapk->set_manifest_url(shortcut_info.manifest_url.spec());
   webapk->set_requester_application_package(
       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())
+  if (shortcut_icon_murmur2_hash.empty()) {
+    webapk->set_stale_manifest(true);
     return webapk;

[pkotwicz, 2016/11/24 22:39:26]

For the sake of simplicity, can we not do the return here? We will populate all
the fields as usual and the server will choose to ignore them

[For a different CL perhaps] I don't know whether the server request will
succeed if
- we are updating a OneOffWebApk over a year after the server created the
WebAPK.
- the web manifest is stale
Does the server retain data for OneOffWebAPKs indefinitely?

[Xi Han, 2016/11/28 16:34:33]

We don't have the best_icon_url, so we can't really populate all the fields. So
for one-off WebAPK, we only send incomplete data; for certified WebAPK, the
server won't and shouldn't trust the resources sent from the client. Neither
case can't benefit from sending more data, personally I would prefer to have a
return here.

 
Currently yes, but they have a plan to clean up caches to avoid the storage
grow. Talked with Gelnn offline, maybe the best solution is to stop supporting
such a WebAPK. Filed a bug crbug.com/669060 to track this case.

[pkotwicz, 2016/11/28 21:29:07]

I agree that sending more data is not useful. However, we are sending the extra
data in the 99% case. In my opinion it is clearer and simpler to send data in
this case too (Not sending it requires knowledge of how the server is
implemented)
I think that we should store the bestIconUrl in AndroidManifest.xml to handle
this case

[Xi Han, 2016/12/05 21:18:27]

Done.
 
As discussed offline, it is better to avoid storing bestIconUrl anyway.

+  }
 
   webapk::WebAppManifest* web_app_manifest = webapk->mutable_manifest();
   web_app_manifest->set_name(base::UTF16ToUTF8(shortcut_info.name));
   web_app_manifest->set_short_name(
       base::UTF16ToUTF8(shortcut_info.short_name));
   web_app_manifest->set_start_url(shortcut_info.url.spec());
   web_app_manifest->set_orientation(
       content::WebScreenOrientationLockTypeToString(
           shortcut_info.orientation));
   web_app_manifest->set_display_mode(
@@ skipping 109 matching lines @@
   //
   // We redownload the icon in order to take the Murmur2 hash. The redownload
   // should be fast because the icon should be in the HTTP cache.
   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) {

[pkotwicz, 2016/11/24 22:39:26]

Can we pass whether the manifest is stale from Java?

[Xi Han, 2016/11/28 16:34:33]

Done.

   UpdateAsyncWithURLRequestContextGetter(
       Profile::FromBrowserContext(browser_context)->GetRequestContext(),
       finish_callback, icon_murmur2_hash, webapk_package, webapk_version);
 }
 
 void WebApkInstaller::UpdateAsyncWithURLRequestContextGetter(
     net::URLRequestContextGetter* request_context_getter,
     const FinishCallback& finish_callback,
     const std::string& icon_murmur2_hash,
     const std::string& webapk_package,
@@ skipping 248 matching lines @@
 
 void WebApkInstaller::OnSuccess() {
   finish_callback_.Run(true, webapk_package_);
   delete this;
 }
 
 void WebApkInstaller::OnFailure() {
   finish_callback_.Run(false, webapk_package_);
   delete this;
 }