[WebApK] Don't exclude unused icons from the WebAPK proto.

Bug: 1051209
Change-Id: Id6b15f35f6db717162beb601147bbcf3f5ff35b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2051104Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740820}

chrome/browser/android/webapk/webapk_installer.cc

@@ -267,13 +267,14 @@ std::unique_ptr<std::string> BuildProtoInBackground(
     }
   }
 
-  for (const auto& entry : icon_url_to_murmur2_hash) {
-    if (entry.first != shortcut_info.best_primary_icon_url.spec() &&
-        entry.first != shortcut_info.best_badge_icon_url.spec()) {
-      continue;
-    }
+  for (const std::string& icon_url : shortcut_info.icon_urls) {
     webapk::Image* image = web_app_manifest->add_icons();
-    if (entry.first == shortcut_info.best_primary_icon_url.spec()) {
+    auto it = icon_url_to_murmur2_hash.find(icon_url);
+    image->set_src(icon_url);
+    if (it != icon_url_to_murmur2_hash.end())
+      image->set_hash(it->second);
+
+    if (icon_url == shortcut_info.best_primary_icon_url.spec()) {
       SetImageData(image, primary_icon);
       image->add_usages(webapk::Image::PRIMARY_ICON);
       if (is_primary_icon_maskable) {
@@ -282,15 +283,13 @@ std::unique_ptr<std::string> BuildProtoInBackground(
         image->add_purposes(webapk::Image::ANY);
       }
     }
-    if (entry.first == shortcut_info.best_badge_icon_url.spec()) {
+    if (icon_url == shortcut_info.best_badge_icon_url.spec()) {
       if (shortcut_info.best_badge_icon_url !=
           shortcut_info.best_primary_icon_url) {
         SetImageData(image, badge_icon);
       }
       image->add_usages(webapk::Image::BADGE_ICON);
     }
-    image->set_src(entry.first);
-    image->set_hash(entry.second);
   }
 
   for (const auto& manifest_shortcut_item : shortcut_info.shortcut_items) {

chrome/browser/android/webapk/webapk_installer_unittest.cc

@@ -65,6 +65,8 @@ const char* kToken = "token";
 // The package name of the downloaded WebAPK.
 const char* kDownloadedWebApkPackageName = "party.unicode";
 
+const char* kUnusedIconPath = "https://example.com/unused_icon.png";
+
 // WebApkInstaller subclass where
 // WebApkInstaller::StartInstallingDownloadedWebApk() and
 // WebApkInstaller::StartUpdateUsingDownloadedWebApk() and
@@ -213,6 +215,8 @@ class BuildProtoRunner {
     ShortcutInfo info(GURL::EmptyGURL());
     info.best_primary_icon_url = best_primary_icon_url;
     info.best_badge_icon_url = best_badge_icon_url;
+    info.icon_urls = {best_primary_icon_url.spec(), best_badge_icon_url.spec(),
+                      kUnusedIconPath};
 
     for (const GURL& shortcut_url : best_shortcut_icon_urls) {
       info.best_shortcut_icon_urls.push_back(shortcut_url);
@@ -538,7 +542,7 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsObsolete) {
   ASSERT_NE(nullptr, webapk_request);
 
   webapk::WebAppManifest manifest = webapk_request->manifest();
-  ASSERT_EQ(2, manifest.icons_size());
+  ASSERT_EQ(5, manifest.icons_size());
 
   EXPECT_EQ("", manifest.icons(0).src());
   EXPECT_FALSE(manifest.icons(0).has_hash());
@@ -547,6 +551,18 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsObsolete) {
   EXPECT_EQ("", manifest.icons(1).src());
   EXPECT_FALSE(manifest.icons(1).has_hash());
   EXPECT_TRUE(manifest.icons(1).has_image_data());
+
+  EXPECT_EQ("", manifest.icons(2).src());
+  EXPECT_EQ("", manifest.icons(2).hash());
+  EXPECT_TRUE(manifest.icons(2).has_image_data());
+
+  EXPECT_EQ("", manifest.icons(3).src());
+  EXPECT_EQ("", manifest.icons(3).hash());
+  EXPECT_TRUE(manifest.icons(3).has_image_data());
+
+  EXPECT_EQ(kUnusedIconPath, manifest.icons(4).src());
+  EXPECT_FALSE(manifest.icons(4).has_hash());
+  EXPECT_FALSE(manifest.icons(4).has_image_data());
 }
 
 // Tests a WebApk install or update request is built properly when the Chrome
@@ -573,24 +589,29 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoWhenManifestIsAvailable) {
   ASSERT_NE(nullptr, webapk_request);
 
   webapk::WebAppManifest manifest = webapk_request->manifest();
-  ASSERT_EQ(2, manifest.icons_size());
+  ASSERT_EQ(3, manifest.icons_size());
 
-  // Check protobuf fields for kBestBadgeIconUrl.
-  EXPECT_EQ(best_badge_icon_url, manifest.icons(0).src());
-  EXPECT_EQ(icon_url_to_murmur2_hash[best_badge_icon_url],
+  // Check protobuf fields for kBestPrimaryIconUrl.
+  EXPECT_EQ(best_primary_icon_url, manifest.icons(0).src());
+  EXPECT_EQ(icon_url_to_murmur2_hash[best_primary_icon_url],
             manifest.icons(0).hash());
   EXPECT_THAT(manifest.icons(0).usages(),
-              testing::ElementsAre(webapk::Image::BADGE_ICON));
+              testing::ElementsAre(webapk::Image::PRIMARY_ICON));
   EXPECT_TRUE(manifest.icons(0).has_image_data());
 
-  // Check protobuf fields for kBestPrimaryIconUrl.
-  EXPECT_EQ(best_primary_icon_url, manifest.icons(1).src());
-  EXPECT_EQ(icon_url_to_murmur2_hash[best_primary_icon_url],
+  // Check protobuf fields for kBestBadgeIconUrl.
+  EXPECT_EQ(best_badge_icon_url, manifest.icons(1).src());
+  EXPECT_EQ(icon_url_to_murmur2_hash[best_badge_icon_url],
             manifest.icons(1).hash());
   EXPECT_THAT(manifest.icons(1).usages(),
-              testing::ElementsAre(webapk::Image::PRIMARY_ICON));
+              testing::ElementsAre(webapk::Image::BADGE_ICON));
   EXPECT_TRUE(manifest.icons(1).has_image_data());
 
+  // Check protobuf fields for unused icon.
+  EXPECT_EQ(kUnusedIconPath, manifest.icons(2).src());
+  EXPECT_FALSE(manifest.icons(2).has_hash());
+  EXPECT_FALSE(manifest.icons(2).has_image_data());
+
   // Check shortcut fields.
   ASSERT_EQ(manifest.shortcuts_size(), 1);
   ASSERT_EQ(manifest.shortcuts(0).icons_size(), 1);
@@ -618,9 +639,9 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoPrimaryIconAndBadgeIconSameUrl) {
   ASSERT_NE(nullptr, webapk_request);
 
   webapk::WebAppManifest manifest = webapk_request->manifest();
-  ASSERT_EQ(1, manifest.icons_size());
+  ASSERT_EQ(3, manifest.icons_size());
 
-  // Check protobuf fields for kBestPrimaryIconUrl.
+  // Check protobuf fields for icons.
   EXPECT_EQ(best_icon_url, manifest.icons(0).src());
   EXPECT_EQ(icon_url_to_murmur2_hash[best_icon_url], manifest.icons(0).hash());
   EXPECT_THAT(manifest.icons(0).usages(),
@@ -628,6 +649,18 @@ TEST_F(WebApkInstallerTest, BuildWebApkProtoPrimaryIconAndBadgeIconSameUrl) {
                                    webapk::Image::BADGE_ICON));
   EXPECT_TRUE(manifest.icons(0).has_image_data());
 
+  EXPECT_EQ(best_icon_url, manifest.icons(1).src());
+  EXPECT_EQ(icon_url_to_murmur2_hash[best_icon_url], manifest.icons(1).hash());
+  EXPECT_THAT(manifest.icons(1).usages(),
+              testing::ElementsAre(webapk::Image::PRIMARY_ICON,
+                                   webapk::Image::BADGE_ICON));
+  EXPECT_TRUE(manifest.icons(1).has_image_data());
+
+  // Check protobuf fields for unused icon.
+  EXPECT_EQ(kUnusedIconPath, manifest.icons(2).src());
+  EXPECT_FALSE(manifest.icons(2).has_hash());
+  EXPECT_FALSE(manifest.icons(2).has_image_data());
+
   // Check shortcut fields.
   ASSERT_EQ(manifest.shortcuts_size(), 1);
   ASSERT_EQ(manifest.shortcuts(0).icons_size(), 1);