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

chrome/browser/android/webapk/webapk_installer_unittest.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 <jni.h>
 #include <memory>
 #include <string>
 
@@ skipping 84 matching lines @@
         base::Bind(&TestWebApkInstaller::OnSuccess, base::Unretained(this)));
   }
 
  private:
   // Whether the Google Play install delegate is available.
   bool has_google_play_webapk_install_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWebApkInstaller);
 };
 
+}  // anonymous namespace
+
 // Runs the WebApkInstaller installation process/update and blocks till done.
 class WebApkInstallerRunner {
  public:
   explicit WebApkInstallerRunner(const GURL& best_icon_url)
       : url_request_context_getter_(new net::TestURLRequestContextGetter(
             base::ThreadTaskRunnerHandle::Get())),
         best_icon_url_(best_icon_url),
         has_google_play_webapk_install_delegate_(false) {}
 
   ~WebApkInstallerRunner() {}
@@ skipping 10 matching lines @@
         base::Bind(&WebApkInstallerRunner::OnCompleted,
                    base::Unretained(this)));
     Run();
   }
 
   void RunUpdateWebApk() {
     const std::string kIconMurmur2Hash = "0";
     const int kWebApkVersion = 1;
 
     WebApkInstaller* installer = CreateWebApkInstaller();
+    std::map<std::string, std::string> icon_url_to_murmur2_hash_map;
+    icon_url_to_murmur2_hash_map[best_icon_url_.spec()] = kIconMurmur2Hash;
 
     installer->UpdateAsyncWithURLRequestContextGetter(
         url_request_context_getter_.get(),
         base::Bind(&WebApkInstallerRunner::OnCompleted, base::Unretained(this)),
-        kIconMurmur2Hash,
         kDownloadedWebApkPackageName,
-        kWebApkVersion);
+        kWebApkVersion,
+        false /* stale_manifest */,
+        icon_url_to_murmur2_hash_map);
 
     Run();
   }
 
   WebApkInstaller* CreateWebApkInstaller() {
     ShortcutInfo info(GURL::EmptyGURL());
     info.best_icon_url = best_icon_url_;
 
     // WebApkInstaller owns itself.
     WebApkInstaller* installer = new TestWebApkInstaller(
         info, SkBitmap(), has_google_play_webapk_install_delegate_);
     installer->SetTimeoutMs(100);
     return installer;
   }
 
+  // Populates a webapk::WebApk.
+  void BuildWebApkProto(
+      const GURL& best_icon_url,
+      bool stale_manifest,
+      const std::map<std::string, std::string> icon_url_to_murmur2_hash_map) {
+    ShortcutInfo shortcut_info(GURL::EmptyGURL());
+    shortcut_info.best_icon_url = best_icon_url;
+
+    base::PostTaskAndReplyWithResult(
+        GetBackgroundTaskRunner().get(), FROM_HERE,
+        base::Bind(&WebApkInstaller::BuildWebApkProtoInBackground,
+                   shortcut_info, SkBitmap(), stale_manifest,
+                   icon_url_to_murmur2_hash_map),
+        base::Bind(&WebApkInstallerRunner::OnBuiltWebApkProto,
+                   base::Unretained(this)));
+
+    Run();

[pkotwicz, 2016/12/14 04:05:41]

Can you do this:

base::RunLoop run_loop;
base::PostTaskAndReplyWithResult(
    GetBackgroundTaskRunner().get(), FROM_HERE,
    base::Bind(&WebApkInstaller::BuildWebApkProtoInBackground,
               shortcut_info, SkBitmap(), stale_manifest,
               icon_url_to_murmur2_hash_map), run_loop.QuitClosure());
run_loop.Run();

Can you put this code in a function which is not in WebApkInstallerRunner? Given
that this function doesn't try to do an install or an update I think that it
doesn't belong.

[Xi Han, 2016/12/14 18:02:23]

We can't use run_loop.QuitClosure() directly. This is because the
base::PostTaskAndReplyWithResult() requires the return value of the first
callback be the input parameter of the second callback. That is why we need
another function to bind. Personally I would prefer to move the
BuildWebApkProto() to WebApkInstallerRunner since it is neat to have a run_loop
in only one class, not both WebApkInstallerRunner and WebApkInstallerTest. 

[pkotwicz, 2016/12/15 02:38:53]

Can you use PostTaskAndReply() instead?
https://cs.chromium.org/chromium/src/base/task_runner.h?rcl=0&l=125

[Xi Han, 2016/12/16 22:39:20]

Since the WebApkInstaller::BuildWebApkProtoInBackground() has a return value, it
doesn't match the signature of base::Closure, which is void().

+  }
+
   void Run() {
     base::RunLoop run_loop;
     on_completed_callback_ = run_loop.QuitClosure();
     run_loop.Run();
   }
 
   bool success() { return success_; }
 
+  webapk::WebApk* GetWebApkRequest() { return webapk_request_.get(); }
+
  private:
   void OnCompleted(bool success, const std::string& webapk_package) {
     success_ = success;
     on_completed_callback_.Run();
   }
 
+  scoped_refptr<base::TaskRunner> GetBackgroundTaskRunner() {
+    return content::BrowserThread::GetBlockingPool()
+        ->GetTaskRunnerWithShutdownBehavior(
+            base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
+  }
+
+  // Called when the |webapk| request is populated.
+  void OnBuiltWebApkProto(std::unique_ptr<webapk::WebApk> webapk) {
+    webapk_request_ = std::move(webapk);
+    on_completed_callback_.Run();
+  }
+
   scoped_refptr<net::TestURLRequestContextGetter>
       url_request_context_getter_;
 
   // The Web Manifest's icon URL.
   const GURL best_icon_url_;
 
   // Called after the installation process has succeeded or failed.
   base::Closure on_completed_callback_;
 
   // Whether the installation process succeeded.
   bool success_;
 
   // Whether the Google Play install delegate is available.
   bool has_google_play_webapk_install_delegate_;
 
+  // The populated webapk::WebApk.
+  std::unique_ptr<webapk::WebApk> webapk_request_;
+
   DISALLOW_COPY_AND_ASSIGN(WebApkInstallerRunner);
 };
 
 // Builds a webapk::WebApkResponse with |download_url| as the WebAPK download
 // URL.
 std::unique_ptr<net::test_server::HttpResponse> BuildValidWebApkResponse(
     const GURL& download_url) {
   std::unique_ptr<webapk::WebApkResponse> response_proto(
       new webapk::WebApkResponse);
   response_proto->set_package_name(kDownloadedWebApkPackageName);
   response_proto->set_signed_download_url(download_url.spec());
   std::string response_content;
   response_proto->SerializeToString(&response_content);
 
   std::unique_ptr<net::test_server::BasicHttpResponse> response(
       new net::test_server::BasicHttpResponse());
   response->set_code(net::HTTP_OK);
   response->set_content(response_content);
   return std::move(response);
 }
 
-}  // anonymous namespace
 
 class WebApkInstallerTest : public ::testing::Test {
  public:
   typedef base::Callback<std::unique_ptr<net::test_server::HttpResponse>(void)>
       WebApkResponseBuilder;
 
   WebApkInstallerTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
   ~WebApkInstallerTest() override {}
 
@@ skipping 143 matching lines @@
   EXPECT_TRUE(runner->success());
 }
 
 // Test installation succeeds using Google Play.
 TEST_F(WebApkInstallerTest, InstallFromGooglePlaySuccess) {
   std::unique_ptr<WebApkInstallerRunner> runner = CreateWebApkInstallerRunner();
   runner->SetHasGooglePlayWebApkInstallDelegate(true);
   runner->RunInstallWebApk();
   EXPECT_TRUE(runner->success());
 }
+
+// When there is no Web Manifest available for a site, an empty |best_icon_url|
+// is used to build a WebApk update request. Tests the request can be built
+// properly.
+TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsObsolete) {
+  GURL icon_url_1 = test_server()->GetURL("/icon1.png");
+  GURL icon_url_2 = test_server()->GetURL("/icon2.png");
+  std::string icon_murmur2_hash_1 = "1";
+  std::string icon_murmur2_hash_2 = "2";
+  std::map<std::string, std::string> icon_url_to_murmur2_hash_map;
+  icon_url_to_murmur2_hash_map[icon_url_1.spec()] = icon_murmur2_hash_1;
+  icon_url_to_murmur2_hash_map[icon_url_2.spec()] = icon_murmur2_hash_2;
+
+  std::unique_ptr<WebApkInstallerRunner> runner = CreateWebApkInstallerRunner();
+  runner->BuildWebApkProto(GURL(""), true, icon_url_to_murmur2_hash_map);
+  webapk::WebApk* webapk_request = runner->GetWebApkRequest();
+  EXPECT_NE(nullptr, webapk_request);
+
+  webapk::WebAppManifest manifest = webapk_request->manifest();
+  EXPECT_EQ(3, manifest.icons_size());
+
+  webapk::Image icons[3];
+  for (int i = 0; i < 3; ++i)
+    icons[i] = manifest.icons(i);
+
+  EXPECT_EQ("", icons[0].src());
+  EXPECT_FALSE(icons[0].has_hash());
+  EXPECT_TRUE(icons[0].has_image_data());
+
+  EXPECT_EQ(icon_url_1.spec(), icons[1].src());
+  EXPECT_EQ(icon_murmur2_hash_1, icons[1].hash());
+  EXPECT_FALSE(icons[1].has_image_data());
+
+  EXPECT_EQ(icon_url_2.spec(), icons[2].src());
+  EXPECT_EQ(icon_murmur2_hash_2, icons[2].hash());
+  EXPECT_FALSE(icons[2].has_image_data());
+}
+
+// Tests a WebApk install or update request is built properly when the Chrome
+// knows the best icon URL of a site after fetching its Web Manifest.
+TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsAvailable) {
+  GURL icon_url_1 = test_server()->GetURL("/icon.png");
+  GURL best_icon_url = test_server()->GetURL(kBestIconUrl);
+  std::string icon_murmur2_hash_1 = "1";
+  std::string best_icon_murmur2_hash = "0";
+  std::map<std::string, std::string> icon_url_to_murmur2_hash_map;
+  icon_url_to_murmur2_hash_map[icon_url_1.spec()] = icon_murmur2_hash_1;
+  icon_url_to_murmur2_hash_map[best_icon_url.spec()] =
+      best_icon_murmur2_hash;
+
+  std::unique_ptr<WebApkInstallerRunner> runner = CreateWebApkInstallerRunner();
+  runner->BuildWebApkProto(best_icon_url, false, icon_url_to_murmur2_hash_map);
+  webapk::WebApk* webapk_request = runner->GetWebApkRequest();
+  EXPECT_NE(nullptr, webapk_request);
+
+  webapk::WebAppManifest manifest = webapk_request->manifest();
+  EXPECT_EQ(2, manifest.icons_size());
+
+  webapk::Image icons[2];
+  for (int i = 0; i < 2; ++i)
+    icons[i] = manifest.icons(i);
+
+  EXPECT_EQ(best_icon_url.spec(), icons[0].src());
+  EXPECT_EQ(best_icon_murmur2_hash, icons[0].hash());
+  EXPECT_TRUE(icons[0].has_image_data());
+
+  EXPECT_EQ(icon_url_1.spec(), icons[1].src());
+  EXPECT_EQ(icon_murmur2_hash_1, icons[1].hash());
+  EXPECT_FALSE(icons[1].has_image_data());
+}