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

chrome/browser/android/webapk/webapk_installer_unittest.cc

Index: chrome/browser/android/webapk/webapk_installer_unittest.cc
diff --git a/chrome/browser/android/webapk/webapk_installer_unittest.cc b/chrome/browser/android/webapk/webapk_installer_unittest.cc
index 0734459e9e6c4e76a8bf1dd1f2427afed6999d6c..1e38ca1879aea8f1e4f4147b9742fe601b86742f 100644
--- a/chrome/browser/android/webapk/webapk_installer_unittest.cc
+++ b/chrome/browser/android/webapk/webapk_installer_unittest.cc
@@ -132,13 +132,16 @@ class WebApkInstallerRunner {
     const int kWebApkVersion = 1;
 
     WebApkInstaller* installer = CreateWebApkInstaller();

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

Minor nit: remove this line, and just do CreateWebApkInstaller()->UpdateAsync on
line 138 (unless you want to DCHECK it or something)

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

Done.

+    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:10]

same.

+    icon_url_to_murmur2_hash_map[best_icon_url_.spec()] = kIconMurmur2Hash;

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

can this be

std::map<std::string, std::string> icon_url_to_murmur2_map {
{best_icon_url_.spec(), "0"} };

And then remove kIconMurmur2Hash

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

Done.

 
     installer->UpdateAsyncWithURLRequestContextGetter(
         url_request_context_getter_.get(),
         base::Bind(&WebApkInstallerRunner::OnCompleted, base::Unretained(this)),
-        kIconMurmur2Hash,
         kDownloadedWebApkPackageName,
-        kWebApkVersion);
+        kWebApkVersion,
+        false /* stale_manifest */,

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

is_manifest_stale

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

Done.

+        icon_url_to_murmur2_hash_map);
 
     Run();
   }
@@ -204,6 +207,53 @@ std::unique_ptr<net::test_server::HttpResponse> BuildValidWebApkResponse(
   return std::move(response);
 }
 
+// Builds WebApk proto and blocks till done.
+class BuildProtoRunner {
+ public:
+  BuildProtoRunner() {}
+
+  ~BuildProtoRunner() {}
+
+  void BuildSync(
+      const GURL& best_icon_url,
+      bool stale_manifest,
+      const std::map<std::string, std::string>& icon_url_to_murmur2_hash_map) {
+    ShortcutInfo info(GURL::EmptyGURL());
+    info.best_icon_url = best_icon_url;
+
+    // WebApkInstaller owns itself.
+    WebApkInstaller* installer =
+        new TestWebApkInstaller(info, SkBitmap(), false);
+    installer->SetTimeoutMs(100);

[pkotwicz, 2016/12/19 23:04:31]

Nit: You don't need to set a timeout

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

Done.

+    installer->BuildWebApkProtoInBackgroundForTesting(
+        base::Bind(&BuildProtoRunner::OnBuiltWebApkProto,
+                   base::Unretained(this)),
+        stale_manifest,
+        icon_url_to_murmur2_hash_map);
+
+    base::RunLoop run_loop;
+    on_completed_callback_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+  webapk::WebApk* GetWebApkRequest() { return webapk_request_.get(); }
+
+ private:
+  // Called when the |webapk| request is populated.

[pkotwicz, 2016/12/19 23:04:31]

|webapk| request -> |webapk_request_|

We use '||' to indicate a variable name

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

Done.

+  void OnBuiltWebApkProto(std::unique_ptr<webapk::WebApk> webapk) {
+    webapk_request_ = std::move(webapk);
+    on_completed_callback_.Run();
+  }
+
+  // The populated webapk::WebApk.
+  std::unique_ptr<webapk::WebApk> webapk_request_;
+
+  // Called after the installation process has succeeded or failed.

[pkotwicz, 2016/12/19 23:04:31]

Please update this comment :)

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

Done.

+  base::Closure on_completed_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(BuildProtoRunner);
+};
+
 }  // anonymous namespace
 
 class WebApkInstallerTest : public ::testing::Test {
@@ -248,6 +298,10 @@ class WebApkInstallerTest : public ::testing::Test {
         new WebApkInstallerRunner(best_icon_url_));
   }
 
+  std::unique_ptr<BuildProtoRunner> CreateBuildProtoRunner() {
+    return std::unique_ptr<BuildProtoRunner>(new BuildProtoRunner());
+  }
+
   net::test_server::EmbeddedTestServer* test_server() { return &test_server_; }
 
  private:
@@ -368,3 +422,73 @@ TEST_F(WebApkInstallerTest, InstallFromGooglePlaySuccess) {
   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<BuildProtoRunner> runner = CreateBuildProtoRunner();
+  runner->BuildSync(GURL(""), true, icon_url_to_murmur2_hash_map);
+  webapk::WebApk* webapk_request = runner->GetWebApkRequest();
+  ASSERT_NE(nullptr, webapk_request);
+
+  webapk::WebAppManifest manifest = webapk_request->manifest();
+  ASSERT_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<BuildProtoRunner> runner = CreateBuildProtoRunner();
+  runner->BuildSync(best_icon_url, false, icon_url_to_murmur2_hash_map);
+  webapk::WebApk* webapk_request = runner->GetWebApkRequest();
+  ASSERT_NE(nullptr, webapk_request);
+
+  webapk::WebAppManifest manifest = webapk_request->manifest();
+  ASSERT_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());
+}