WebApp: Use vector<WebApplicationIconInfo> icon infos in WebAppIconManager.

Decouple icons pixel data from WebApplicationInfo metadata.

The Update and Sync-Initiated installs will use it to override icon pixel
data. See next CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1858025

Bug: 860583
Change-Id: Ief987de76bfd1389835c1318e57fb7d79ac4728d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1861443
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706698}

chrome/browser/web_applications/web_app_icon_manager.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/web_applications/file_utils_wrapper.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
+#include "chrome/common/web_application_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/codec/png_codec.h"
@@ -84,14 +85,14 @@ bool WriteIcon(FileUtilsWrapper* utils,
 
 bool WriteIcons(FileUtilsWrapper* utils,
                 const base::FilePath& app_dir,
-                const WebApplicationInfo& web_app_info) {
+                const std::vector<WebApplicationIconInfo>& icon_infos) {
   const base::FilePath icons_dir = app_dir.Append(kIconsDirectoryName);
   if (!utils->CreateDirectory(icons_dir)) {
     LOG(ERROR) << "Could not create icons directory.";
     return false;
   }
 
-  for (const WebApplicationIconInfo& icon_info : web_app_info.icons) {
+  for (const WebApplicationIconInfo& icon_info : icon_infos) {
     // Skip unfetched bitmaps.
     if (icon_info.data.colorType() == kUnknown_SkColorType)
       continue;
@@ -104,11 +105,11 @@ bool WriteIcons(FileUtilsWrapper* utils,
 }
 
 // Performs blocking I/O. May be called on another thread.
-// Returns true if no errors occured.
+// Returns true if no errors occurred.
 bool WriteDataBlocking(std::unique_ptr<FileUtilsWrapper> utils,
                        base::FilePath web_apps_directory,
                        AppId app_id,
-                       std::unique_ptr<WebApplicationInfo> web_app_info) {
+                       std::vector<WebApplicationIconInfo> icon_infos) {
   const base::FilePath temp_dir = GetTempDir(utils.get(), web_apps_directory);
   if (temp_dir.empty()) {
     LOG(ERROR)
@@ -122,7 +123,7 @@ bool WriteDataBlocking(std::unique_ptr<FileUtilsWrapper> utils,
     return false;
   }
 
-  if (!WriteIcons(utils.get(), app_temp_dir.GetPath(), *web_app_info))
+  if (!WriteIcons(utils.get(), app_temp_dir.GetPath(), icon_infos))
     return false;
 
   // Commit: move whole app data dir to final destination in one mv operation.
@@ -137,7 +138,7 @@ bool WriteDataBlocking(std::unique_ptr<FileUtilsWrapper> utils,
 }
 
 // Performs blocking I/O. May be called on another thread.
-// Returns empty SkBitmap if any errors occured.
+// Returns empty SkBitmap if any errors occurred.
 SkBitmap ReadIconBlocking(std::unique_ptr<FileUtilsWrapper> utils,
                           base::FilePath web_apps_directory,
                           AppId app_id,
@@ -182,14 +183,14 @@ WebAppIconManager::~WebAppIconManager() = default;
 
 void WebAppIconManager::WriteData(
     AppId app_id,
-    std::unique_ptr<WebApplicationInfo> web_app_info,
+    std::vector<WebApplicationIconInfo> icon_infos,
     WriteDataCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   base::PostTaskAndReplyWithResult(
       FROM_HERE, kTaskTraits,
       base::BindOnce(WriteDataBlocking, utils_->Clone(), web_apps_directory_,
-                     std::move(app_id), std::move(web_app_info)),
+                     std::move(app_id), std::move(icon_infos)),
       std::move(callback));
 }
 

chrome/browser/web_applications/web_app_icon_manager.h

@@ -6,13 +6,14 @@
 #define CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_ICON_MANAGER_H_
 
 #include <memory>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "chrome/browser/web_applications/components/app_icon_manager.h"
-#include "chrome/common/web_application_info.h"
 
 class Profile;
+struct WebApplicationIconInfo;
 
 namespace web_app {
 
@@ -30,7 +31,7 @@ class WebAppIconManager : public AppIconManager {
   // Writes all data (icons) for an app.
   using WriteDataCallback = base::OnceCallback<void(bool success)>;
   void WriteData(AppId app_id,
-                 std::unique_ptr<WebApplicationInfo> web_app_info,
+                 std::vector<WebApplicationIconInfo> icon_infos,
                  WriteDataCallback callback);
 
   // AppIconManager:

chrome/browser/web_applications/web_app_icon_manager_unittest.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_sync_bridge.h"
+#include "chrome/common/web_application_info.h"
 #include "chrome/test/base/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
@@ -48,17 +49,18 @@ class WebAppIconManagerTest : public WebAppTest {
                   const std::vector<int>& sizes_px,
                   const std::vector<SkColor>& colors) {
     DCHECK_EQ(sizes_px.size(), colors.size());
-    auto web_app_info = std::make_unique<WebApplicationInfo>();
-    web_app_info->icons.reserve(sizes_px.size());
+
+    std::vector<WebApplicationIconInfo> icon_infos;
+    icon_infos.reserve(sizes_px.size());
+
     for (size_t i = 0; i < sizes_px.size(); ++i) {
       std::string icon_name = base::StringPrintf("app-%d.ico", sizes_px[i]);
       GURL icon_url = app_url.Resolve(icon_name);
-      web_app_info->icons.push_back(
-          GenerateIconInfo(icon_url, sizes_px[i], colors[i]));
+      icon_infos.push_back(GenerateIconInfo(icon_url, sizes_px[i], colors[i]));
     }
 
     base::RunLoop run_loop;
-    icon_manager_->WriteData(app_id, std::move(web_app_info),
+    icon_manager_->WriteData(app_id, std::move(icon_infos),
                              base::BindLambdaForTesting([&](bool success) {
                                EXPECT_TRUE(success);
                                run_loop.Quit();

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -64,10 +64,11 @@ Source::Type InferSourceFromMetricsInstallSource(
   }
 }
 
-void SetIcons(const WebApplicationInfo& web_app_info, WebApp* web_app) {
+void SetWebAppIcons(const std::vector<WebApplicationIconInfo>& icon_infos,
+                    WebApp* web_app) {
   WebApp::Icons web_app_icons;
 
-  for (const WebApplicationIconInfo& icon_info : web_app_info.icons) {
+  for (const WebApplicationIconInfo& icon_info : icon_infos) {
     // Skip unfetched bitmaps.
     if (icon_info.data.colorType() == kUnknown_SkColorType)
       continue;
@@ -128,7 +129,7 @@ void WebAppInstallFinalizer::FinalizeInstall(
                               : blink::mojom::DisplayMode::kBrowser);
   web_app->SetIsLocallyInstalled(options.locally_installed);
 
-  SetIcons(web_app_info, web_app.get());
+  SetWebAppIcons(web_app_info.icons, web_app.get());
 
   web_app->SetIsInSyncInstall(false);
   WebApp::SyncData sync_data;
@@ -137,7 +138,7 @@ void WebAppInstallFinalizer::FinalizeInstall(
   web_app->SetSyncData(std::move(sync_data));
 
   icon_manager_->WriteData(
-      std::move(app_id), std::make_unique<WebApplicationInfo>(web_app_info),
+      std::move(app_id), web_app_info.icons,
       base::BindOnce(&WebAppInstallFinalizer::OnIconsDataWritten,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback),
                      std::move(web_app)));