WebApps: Support PWA manifest icon URL updating

This CL enables PWA manifest updating to detect when sites update their
"icons" field (but not when the icon contents themselves change).

Screencast: https://bugs.chromium.org/p/chromium/issues/attachment?aid=423127&signed_aid=pWxFmMl1EjQvs_LiPg2zog==&inline=1

Bug: 926083
Change-Id: I6ff14e7b813c9526f8e423b4d3de50c7f5349abc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935297
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720855}

chrome/browser/web_applications/components/app_registrar.h

@@ -13,6 +13,7 @@
 #include "base/optional.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/common/web_application_info.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 class GURL;
@@ -70,6 +71,11 @@ class AppRegistrar {
   virtual DisplayMode GetAppDisplayMode(const AppId& app_id) const = 0;
   virtual DisplayMode GetAppUserDisplayMode(const AppId& app_id) const = 0;
 
+  // Returns the "icons" field from the app manifest, use |AppIconManager| to
+  // load icon bitmap data.
+  virtual std::vector<WebApplicationIconInfo> GetAppIconInfos(
+      const AppId& app_id) const = 0;
+
   virtual std::vector<AppId> GetAppIds() const = 0;
 
   // Searches for the first app id in the registry for which the |url| is in

chrome/browser/web_applications/components/manifest_update_manager_browsertest.cc

@@ -551,9 +551,8 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest, CheckKeepsSameName) {
   EXPECT_EQ(GetProvider().registrar().GetAppShortName(app_id), "App name 1");
 }
 
-// TODO(crbug.com/926083): Implement icon URL change detection.
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
-                       DISABLED_CheckFindsIconUrlChange) {
+                       CheckFindsIconUrlChange) {
   constexpr char kManifestTemplate[] = R"(
     {
       "name": "Test app name",
@@ -567,11 +566,10 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
   AppId app_id = InstallWebApp();
 
   OverrideManifest(kManifestTemplate, {kAnotherInstallableIconList});
-  // TODO(crbug.com/926083): Implement icon updating.
   EXPECT_EQ(GetResultAfterPageLoad(GetAppURL(), &app_id),
-            ManifestUpdateResult::kAppUpdateFailed);
-  histogram_tester_.ExpectBucketCount(
-      kUpdateHistogramName, ManifestUpdateResult::kAppUpdateFailed, 1);
+            ManifestUpdateResult::kAppUpdated);
+  histogram_tester_.ExpectBucketCount(kUpdateHistogramName,
+                                      ManifestUpdateResult::kAppUpdated, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,

chrome/browser/web_applications/components/manifest_update_task.cc

@@ -114,6 +114,9 @@ bool ManifestUpdateTask::IsUpdateNeeded(
   if (web_application_info.scope != registrar_.GetAppScope(app_id_))
     return true;
 
+  if (web_application_info.icon_infos != registrar_.GetAppIconInfos(app_id_))
+    return true;
+
   // TODO(crbug.com/926083): Check more manifest fields.
   return false;
 }

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -17,6 +17,7 @@
 #include "chrome/common/extensions/manifest_handlers/app_display_mode_info.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/extensions/manifest_handlers/app_theme_color_info.h"
+#include "chrome/common/extensions/manifest_handlers/linked_app_icons.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
@@ -162,6 +163,22 @@ DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
   }
 }
 
+std::vector<WebApplicationIconInfo> BookmarkAppRegistrar::GetAppIconInfos(
+    const web_app::AppId& app_id) const {
+  std::vector<WebApplicationIconInfo> result;
+  const Extension* extension = GetExtension(app_id);
+  if (!extension)
+    return result;
+  for (const LinkedAppIcons::IconInfo& icon_info :
+       LinkedAppIcons::GetLinkedAppIcons(extension).icons) {
+    WebApplicationIconInfo web_app_icon_info;
+    web_app_icon_info.url = icon_info.url;
+    web_app_icon_info.square_size_px = icon_info.size;
+    result.push_back(std::move(web_app_icon_info));
+  }
+  return result;
+}
+
 std::vector<web_app::AppId> BookmarkAppRegistrar::GetAppIds() const {
   std::vector<web_app::AppId> app_ids;
   for (scoped_refptr<const Extension> app :

chrome/browser/web_applications/extensions/bookmark_app_registrar.h

@@ -39,6 +39,8 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
       const web_app::AppId& app_id) const override;
   web_app::DisplayMode GetAppUserDisplayMode(
       const web_app::AppId& app_id) const override;
+  std::vector<WebApplicationIconInfo> GetAppIconInfos(
+      const web_app::AppId& app_id) const override;
   std::vector<web_app::AppId> GetAppIds() const override;
 
   // ExtensionRegistryObserver:

chrome/browser/web_applications/test/test_app_registrar.cc

@@ -120,6 +120,12 @@ DisplayMode TestAppRegistrar::GetAppUserDisplayMode(const AppId& app_id) const {
   return DisplayMode::kBrowser;
 }
 
+std::vector<WebApplicationIconInfo> TestAppRegistrar::GetAppIconInfos(
+    const AppId& app_id) const {
+  NOTIMPLEMENTED();
+  return {};
+}
+
 std::vector<AppId> TestAppRegistrar::GetAppIds() const {
   std::vector<AppId> result;
   for (const std::pair<AppId, AppInfo>& it : installed_apps_) {

chrome/browser/web_applications/test/test_app_registrar.h

@@ -52,6 +52,8 @@ class TestAppRegistrar : public AppRegistrar {
   base::Optional<GURL> GetAppScope(const AppId& app_id) const override;
   DisplayMode GetAppDisplayMode(const AppId& app_id) const override;
   DisplayMode GetAppUserDisplayMode(const AppId& app_id) const override;
+  std::vector<WebApplicationIconInfo> GetAppIconInfos(
+      const AppId& app_id) const override;
   std::vector<AppId> GetAppIds() const override;
 
  private:

chrome/browser/web_applications/web_app_registrar.cc

@@ -93,6 +93,13 @@ DisplayMode WebAppRegistrar::GetAppUserDisplayMode(const AppId& app_id) const {
   return web_app ? web_app->user_display_mode() : DisplayMode::kUndefined;
 }
 
+std::vector<WebApplicationIconInfo> WebAppRegistrar::GetAppIconInfos(
+    const AppId& app_id) const {
+  auto* web_app = GetAppById(app_id);
+  return web_app ? web_app->icon_infos()
+                 : std::vector<WebApplicationIconInfo>();
+}
+
 std::vector<AppId> WebAppRegistrar::GetAppIds() const {
   std::vector<AppId> app_ids;
   app_ids.reserve(registry_.size());

chrome/browser/web_applications/web_app_registrar.h

@@ -44,6 +44,8 @@ class WebAppRegistrar : public AppRegistrar {
   base::Optional<GURL> GetAppScope(const AppId& app_id) const override;
   DisplayMode GetAppDisplayMode(const AppId& app_id) const override;
   DisplayMode GetAppUserDisplayMode(const AppId& app_id) const override;
+  std::vector<WebApplicationIconInfo> GetAppIconInfos(
+      const AppId& app_id) const override;
   std::vector<AppId> GetAppIds() const override;
 
   // Only range-based |for| loop supported. Don't use AppSet directly.