[dPWA] Add Execute tests for ShortcutSubManager

This CL adds icon reading functionality for Linux and ChromeOS in the
OsIntegrationTestOverride and adds Execute() logic tests for the
ShortcutSubManager using the logic. This also rewires
WebAppIntegrationTestDriver shortcut icon color comparisons to use
the newly added logic.

Bug: 1295044, b/269623320
Change-Id: If4930c9a3a0f4b0e31c04fb02409e505e75ede65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4287787
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Camden King <camdenking@google.com>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115858}

chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc

@@ -90,7 +90,6 @@
 #include "chrome/browser/web_applications/web_app_constants.h"
 #include "chrome/browser/web_applications/web_app_helpers.h"
 #include "chrome/browser/web_applications/web_app_icon_generator.h"
-#include "chrome/browser/web_applications/web_app_icon_manager.h"
 #include "chrome/browser/web_applications/web_app_id.h"
 #include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
@@ -583,26 +582,6 @@ std::vector<std::wstring> GetFileExtensionsForProgId(
 }
 #endif
 
-#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
-bool IconManagerCheckIconTopLeftColor(WebAppIconManager& icon_manager,
-                                      const AppId& app_id,
-                                      std::vector<int> sizes_px,
-                                      SkColor expected_icon_pixel_color) {
-  bool icons_exist = icon_manager.HasIcons(app_id, IconPurpose::ANY, sizes_px);
-  if (icons_exist) {
-    for (int size_px : sizes_px) {
-      SkColor icon_pixel_color =
-          IconManagerReadAppIconPixel(icon_manager, app_id, size_px, 0, 0);
-      if (icon_pixel_color != expected_icon_pixel_color) {
-        return false;
-      }
-    }
-    return true;
-  }
-  return false;
-}
-#endif
-
 absl::optional<ProfileState> GetStateForProfile(StateSnapshot* state_snapshot,
                                                 Profile* profile) {
   DCHECK(state_snapshot);
@@ -3585,9 +3564,19 @@ bool WebAppIntegrationTestDriver::DoIconColorsMatch(Profile* profile,
 #elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
   SkColor expected_icon_pixel_color =
       GetSiteConfigurationFromAppName(name).icon_color;
-  do_icon_colors_match = IconManagerCheckIconTopLeftColor(
-      provider()->icon_manager(), id, {kLauncherIconSize, kInstallIconSize},
-      expected_icon_pixel_color);
+  absl::optional<SkColor> actual_color_install_icon_size =
+      override_registration_->test_override->GetShortcutIconTopLeftColor(
+          profile, base::FilePath(), id, name, kInstallIconSize);
+
+  absl::optional<SkColor> actual_color_launcher_icon_size =
+      override_registration_->test_override->GetShortcutIconTopLeftColor(
+          profile, base::FilePath(), id, name, kLauncherIconSize);
+  if (actual_color_install_icon_size.has_value() &&
+      actual_color_launcher_icon_size.has_value()) {
+    do_icon_colors_match =
+        (expected_icon_pixel_color == actual_color_install_icon_size.value() &&
+         expected_icon_pixel_color == actual_color_launcher_icon_size.value());
+  }
 #endif
   return do_icon_colors_match;
 }

chrome/browser/web_applications/os_integration/os_integration_test_override.cc

@@ -29,8 +29,11 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/web_applications/os_integration/web_app_file_handler_registration.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "chrome/browser/web_applications/web_app_icon_generator.h"
+#include "chrome/browser/web_applications/web_app_icon_manager.h"
 #include "chrome/browser/web_applications/web_app_id.h"
 #include "chrome/browser/web_applications/web_app_install_info.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
@@ -129,6 +132,31 @@ std::vector<std::wstring> GetFileExtensionsForProgId(
 }
 #endif
 
+#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+// Performs a blocking read of app icons from the disk.
+SkColor IconManagerReadIconTopLeftColorForSize(WebAppIconManager& icon_manager,
+                                               const AppId& app_id,
+                                               SquareSizePx size_px) {
+  SkColor result = SK_ColorTRANSPARENT;
+  if (!icon_manager.HasIcons(app_id, IconPurpose::ANY, {size_px})) {
+    return result;
+  }
+  base::RunLoop run_loop;
+  icon_manager.ReadIcons(
+      app_id, IconPurpose::ANY, {size_px},
+      base::BindOnce(
+          [](base::RunLoop* run_loop, SkColor* result, SquareSizePx size_px,
+             std::map<SquareSizePx, SkBitmap> icon_bitmaps) {
+            DCHECK(base::Contains(icon_bitmaps, size_px));
+            *result = icon_bitmaps.at(size_px).getColor(0, 0);
+            run_loop->Quit();
+          },
+          &run_loop, &result, size_px));
+  run_loop.Run();
+  return result;
+}
+#endif
+
 }  // namespace
 
 OsIntegrationTestOverride::BlockingRegistration::BlockingRegistration() =
@@ -256,20 +284,31 @@ bool OsIntegrationTestOverride::IsFileExtensionHandled(
   return is_file_handled;
 }
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
 absl::optional<SkColor> OsIntegrationTestOverride::GetShortcutIconTopLeftColor(
     Profile* profile,
     base::FilePath shortcut_dir,
     const AppId& app_id,
-    const std::string& app_name) {
+    const std::string& app_name,
+    SquareSizePx size_px) {
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
   base::FilePath shortcut_path =
       GetShortcutPath(profile, shortcut_dir, app_id, app_name);
   if (!base::PathExists(shortcut_path)) {
     return absl::nullopt;
   }
   return GetIconTopLeftColorFromShortcutFile(shortcut_path);
-}
+#elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
+  WebAppProvider* provider = WebAppProvider::GetForLocalAppsUnchecked(profile);
+  if (!provider) {
+    return absl::nullopt;
+  }
+  return IconManagerReadIconTopLeftColorForSize(provider->icon_manager(),
+                                                app_id, size_px);
+#else
+  NOTREACHED() << "Not implemented on Fuchsia";
+  return absl::nullopt;
 #endif
+}
 
 #if BUILDFLAG(IS_WIN)
 void OsIntegrationTestOverride::AddShortcutsMenuJumpListEntryForApp(

chrome/browser/web_applications/os_integration/os_integration_test_override.h

@@ -17,7 +17,9 @@
 #include "base/functional/callback_helpers.h"
 #include "base/memory/ref_counted.h"
 #include "build/build_config.h"
+#include "chrome/browser/web_applications/web_app_icon_generator.h"
 #include "chrome/browser/web_applications/web_app_id.h"
+#include "chrome/browser/web_applications/web_app_install_info.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
 
@@ -115,14 +117,20 @@ class OsIntegrationTestOverride
                               std::string app_name,
                               std::string file_extension);
 
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
-  // Reads the icon color for a specific shortcut created
+  // Reads the icon color for a specific shortcut that has been created.
+  // For Mac and Win, the shortcut_dir field is mandatory. For all other OSes,
+  // this can be an empty base::FilePath().
+  // For ChromeOS and Linux, the size_px field is mandatory to prevent erroneous
+  // results. For all other OSes, the field can be skipped. The default value of
+  // size_px is usually filled up with kLauncherIconSize (see
+  // chrome/browser/web_applications/web_app_icon_generator.h for more
+  // information), which is 128.
   absl::optional<SkColor> GetShortcutIconTopLeftColor(
       Profile* profile,
       base::FilePath shortcut_dir,
       const AppId& app_id,
-      const std::string& app_name);
-#endif
+      const std::string& app_name,
+      SquareSizePx size_px = icon_size::k128);
 
 #if BUILDFLAG(IS_WIN)
   // These should not be called from tests, these are automatically