Support offline manifest install for default web app configs

This CL adds a new "offline_manifest" field to default web app JSON
configs.

Example:
{
  "app_url": "https://example.org/",
  "launch_container": "window",
  "user_type": ["unmanaged"],
  "offline_manifest": {
    "name": "Example",
    "start_url": "https://example.org/",
    "scope": "https://example.org/",
    "display": "standalone",
    "icon_any_pngs": ["icon.png"],
    "theme_color_argb_hex": "FFFFFFFF"
  }
}

This is to support installing default web apps on Chrome OS without
spawning a WebContents and fetching the install URL of every default app.
Additionally this is to avoid overwhelming the default web apps when
every Chrome user is migrated over to default web apps instead of
the current default Chrome apps.

Design doc:
https://docs.google.com/document/d/1pk8aLnp24AVsWwC6OaCgYZHGGqvyNxekB04rzuTGDVM/edit#heading=h.9f9lhtrsvs1k

Bug: 1119710
Change-Id: Ibe36631e4dbb540165cd67944b5ec6c8a47e8a2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352450
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800066}

chrome/browser/ui/web_applications/test/web_app_browsertest_util.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/app_menu_model.h"
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
+#include "chrome/browser/web_applications/components/app_icon_manager.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/external_install_options.h"
 #include "chrome/browser/web_applications/components/install_finalizer.h"
@@ -28,7 +29,6 @@
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/components/web_app_tab_helper_base.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
-#include "chrome/common/web_application_info.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/security_interstitials/content/security_interstitial_tab_helper.h"
 #include "content/public/browser/notification_service.h"
@@ -217,4 +217,22 @@ void UninstallWebApp(Profile* profile, const AppId& app_id) {
                                                            base::DoNothing());
 }
 
+SkColor ReadAppIconPixel(Profile* profile,
+                         const AppId& app_id,
+                         SquareSizePx size,
+                         int x,
+                         int y) {
+  SkColor result;
+  base::RunLoop run_loop;
+  WebAppProviderBase::GetProviderBase(profile)->icon_manager().ReadIcons(
+      app_id, IconPurpose::ANY, {size},
+      base::BindLambdaForTesting(
+          [&](std::map<SquareSizePx, SkBitmap> icon_bitmaps) {
+            run_loop.Quit();
+            result = icon_bitmaps.at(size).getColor(x, y);
+          }));
+  run_loop.Run();
+  return result;
+}
+
 }  // namespace web_app

chrome/browser/ui/web_applications/test/web_app_browsertest_util.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "chrome/browser/web_applications/components/web_app_id.h"
+#include "chrome/common/web_application_info.h"
 #include "url/gurl.h"
 
 class Browser;
@@ -64,6 +65,13 @@ bool IsBrowserOpen(const Browser* test_browser);
 // Launches the app for |app| in |profile|.
 void UninstallWebApp(Profile* profile, const AppId& app_id);
 
+// Synchronous read of an app icon pixel.
+SkColor ReadAppIconPixel(Profile* profile,
+                         const AppId& app_id,
+                         SquareSizePx size,
+                         int x,
+                         int y);
+
 }  // namespace web_app
 
 #endif  // CHROME_BROWSER_UI_WEB_APPLICATIONS_TEST_WEB_APP_BROWSERTEST_UTIL_H_

chrome/browser/web_applications/BUILD.gn

@@ -59,6 +59,8 @@ source_set("web_applications") {
   sources = [
     "external_web_app_manager.cc",
     "external_web_app_manager.h",
+    "external_web_app_utils.cc",
+    "external_web_app_utils.h",
     "file_utils_wrapper.cc",
     "file_utils_wrapper.h",
     "pending_app_install_task.cc",
@@ -194,6 +196,7 @@ source_set("web_applications_unit_tests") {
   sources = [
     "daily_metrics_helper_unittest.cc",
     "external_web_app_manager_unittest.cc",
+    "external_web_app_utils_unittest.cc",
     "pending_app_manager_impl_unittest.cc",
     "system_web_app_manager_unittest.cc",
     "web_app_database_unittest.cc",

chrome/browser/web_applications/external_web_app_manager.cc

@@ -14,6 +14,7 @@
 #include "base/feature_list.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/json/json_reader.h"
 #include "base/no_destructor.h"
@@ -24,10 +25,10 @@
 #include "build/build_config.h"
 #include "chrome/browser/apps/user_type_filter.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/web_applications/components/external_app_install_features.h"
 #include "chrome/browser/web_applications/components/pending_app_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
+#include "chrome/browser/web_applications/external_web_app_utils.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
@@ -41,43 +42,6 @@ namespace web_app {
 
 namespace {
 
-// kAppUrl is a required string specifying a URL inside the scope of the web
-// app that contains a link to the app manifest.
-constexpr char kAppUrl[] = "app_url";
-
-// kHideFromUser is an optional boolean which controls whether we add
-// a shortcut to the relevant OS surface i.e. Application folder on macOS, Start
-// Menu on Windows and Linux, and launcher on Chrome OS. Defaults to false if
-// missing. If true, we also don't show the app in search or in app management
-// on Chrome OS.
-constexpr char kHideFromUser[] = "hide_from_user";
-
-// kCreateShortcuts is an optional boolean which controls whether OS
-// level shortcuts are created. On Chrome OS this controls whether the app is
-// pinned to the shelf.
-// The default value of kCreateShortcuts if false.
-constexpr char kCreateShortcuts[] = "create_shortcuts";
-
-// kFeatureName is an optional string parameter specifying a feature
-// associated with this app. The feature must be present in
-// |kExternalAppInstallFeatures| to be applicable.
-// If specified:
-//  - if the feature is enabled, the app will be installed
-//  - if the feature is not enabled, the app will be removed.
-constexpr char kFeatureName[] = "feature_name";
-
-// kLaunchContainer is a required string which can be "window" or "tab"
-// and controls what sort of container the web app is launched in.
-constexpr char kLaunchContainer[] = "launch_container";
-constexpr char kLaunchContainerTab[] = "tab";
-constexpr char kLaunchContainerWindow[] = "window";
-
-// kUninstallAndReplace is an optional array of strings which specifies App IDs
-// which the app is replacing. This will transfer OS attributes (e.g the source
-// app's shelf and app list positions on ChromeOS) and then uninstall the source
-// app.
-constexpr char kUninstallAndReplace[] = "uninstall_and_replace";
-
 #if defined(OS_CHROMEOS)
 // The sub-directory of the extensions directory in which to scan for external
 // web apps (as opposed to external extensions or external ARC apps).
@@ -87,124 +51,10 @@ const base::FilePath::CharType kWebAppsSubDirectory[] =
 
 bool g_skip_startup_scan_for_testing_ = false;
 
-base::Optional<ExternalInstallOptions> ParseConfig(
-    base::FilePath file,
-    const std::string& user_type,
-    const base::Value& app_config) {
-  if (app_config.type() != base::Value::Type::DICTIONARY) {
-    LOG(ERROR) << file << " was not a dictionary as the top level";
-    return base::nullopt;
-  }
-
-  if (!apps::UserTypeMatchesJsonUserType(
-          user_type, /*app_id=*/file.MaybeAsASCII(), &app_config,
-          /*default_user_types=*/nullptr)) {
-    // Already logged.
-    return base::nullopt;
-  }
-
-  const base::Value* value =
-      app_config.FindKeyOfType(kFeatureName, base::Value::Type::STRING);
-  if (value) {
-    // TODO(crbug.com/1104696): Add metrics for whether the app was
-    // enabled/disabled by the feature.
-    const std::string& feature_name = value->GetString();
-    VLOG(1) << file << " checking feature " << feature_name;
-    if (!IsExternalAppInstallFeatureEnabled(feature_name)) {
-      VLOG(1) << file << " feature not enabled";
-      return base::nullopt;
-    }
-  }
-
-  value = app_config.FindKeyOfType(kAppUrl, base::Value::Type::STRING);
-  if (!value) {
-    LOG(ERROR) << file << " had a missing " << kAppUrl;
-    return base::nullopt;
-  }
-  GURL app_url(value->GetString());
-  if (!app_url.is_valid()) {
-    LOG(ERROR) << file << " had an invalid " << kAppUrl;
-    return base::nullopt;
-  }
-
-  bool hide_from_user = false;
-  value = app_config.FindKey(kHideFromUser);
-  if (value) {
-    if (!value->is_bool()) {
-      LOG(ERROR) << file << " had an invalid " << kHideFromUser;
-      return base::nullopt;
-    }
-    hide_from_user = value->GetBool();
-  }
-
-  bool create_shortcuts = false;
-  value = app_config.FindKey(kCreateShortcuts);
-  if (value) {
-    if (!value->is_bool()) {
-      LOG(ERROR) << file << " had an invalid " << kCreateShortcuts;
-      return base::nullopt;
-    }
-    create_shortcuts = value->GetBool();
-  }
-
-  // It doesn't make sense to hide the app and also create shortcuts for it.
-  DCHECK(!(hide_from_user && create_shortcuts));
-
-  value = app_config.FindKeyOfType(kLaunchContainer, base::Value::Type::STRING);
-  if (!value) {
-    LOG(ERROR) << file << " had an invalid " << kLaunchContainer;
-    return base::nullopt;
-  }
-  std::string launch_container_str = value->GetString();
-  auto user_display_mode = DisplayMode::kBrowser;
-  if (launch_container_str == kLaunchContainerTab) {
-    user_display_mode = DisplayMode::kBrowser;
-  } else if (launch_container_str == kLaunchContainerWindow) {
-    user_display_mode = DisplayMode::kStandalone;
-  } else {
-    LOG(ERROR) << file << " had an invalid " << kLaunchContainer;
-    return base::nullopt;
-  }
-
-  value = app_config.FindKey(kUninstallAndReplace);
-  std::vector<AppId> uninstall_and_replace_ids;
-  if (value) {
-    if (!value->is_list()) {
-      LOG(ERROR) << file << " had an invalid " << kUninstallAndReplace;
-      return base::nullopt;
-    }
-    base::Value::ConstListView uninstall_and_replace_values = value->GetList();
-
-    bool had_error = false;
-    for (const auto& app_id_value : uninstall_and_replace_values) {
-      if (!app_id_value.is_string()) {
-        had_error = true;
-        LOG(ERROR) << file << " had an invalid " << kUninstallAndReplace
-                   << " entry";
-        break;
-      }
-      uninstall_and_replace_ids.push_back(app_id_value.GetString());
-    }
-    if (had_error)
-      return base::nullopt;
-  }
-
-  ExternalInstallOptions install_options(
-      std::move(app_url), user_display_mode,
-      ExternalInstallSource::kExternalDefault);
-  install_options.add_to_applications_menu = !hide_from_user;
-  install_options.add_to_search = !hide_from_user;
-  install_options.add_to_management = !hide_from_user;
-  install_options.add_to_desktop = create_shortcuts;
-  install_options.add_to_quick_launch_bar = create_shortcuts;
-  install_options.require_manifest = true;
-  install_options.uninstall_and_replace = std::move(uninstall_and_replace_ids);
-
-  return install_options;
-}
-
-std::vector<ExternalInstallOptions> ScanDir(const base::FilePath& dir,
-                                            const std::string& user_type) {
+std::vector<ExternalInstallOptions> ScanDir(
+    std::unique_ptr<FileUtilsWrapper> file_utils,
+    const base::FilePath& dir,
+    const std::string& user_type) {
   std::vector<ExternalInstallOptions> install_options_list;
   if (!base::FeatureList::IsEnabled(features::kDefaultWebAppInstallation))
     return install_options_list;
@@ -231,7 +81,7 @@ std::vector<ExternalInstallOptions> ScanDir(const base::FilePath& dir,
       continue;
     }
     base::Optional<ExternalInstallOptions> install_options =
-        ParseConfig(file, user_type, *app_config);
+        ParseConfig(*file_utils, dir, file, user_type, *app_config);
     if (install_options.has_value())
       install_options_list.push_back(std::move(*install_options));
   }
@@ -270,6 +120,28 @@ void OnExternalWebAppsSynchronized(
                                      install_results);
 }
 
+std::vector<ExternalInstallOptions> SynchronizeAppsBlockingForTesting(
+    std::unique_ptr<FileUtilsWrapper> file_utils,
+    std::vector<std::string> app_configs,
+    const std::string& user_type) {
+  std::vector<ExternalInstallOptions> install_options_list;
+
+  for (const std::string& app_config_string : app_configs) {
+    base::Optional<base::Value> app_config =
+        base::JSONReader::Read(app_config_string);
+    DCHECK(app_config);
+
+    base::Optional<ExternalInstallOptions> install_options =
+        ParseConfig(*file_utils, base::FilePath(FILE_PATH_LITERAL("test_dir")),
+                    base::FilePath(FILE_PATH_LITERAL("test_dir/test.json")),
+                    user_type, *app_config);
+    if (install_options)
+      install_options_list.push_back(std::move(*install_options));
+  }
+
+  return install_options_list;
+}
+
 }  // namespace
 
 ExternalWebAppManager::ExternalWebAppManager(Profile* profile)
@@ -284,18 +156,20 @@ void ExternalWebAppManager::SetSubsystems(
 
 void ExternalWebAppManager::Start() {
   if (!g_skip_startup_scan_for_testing_) {
-    ScanForExternalWebApps(
-        base::BindOnce(&ExternalWebAppManager::OnScanForExternalWebApps,
-                       weak_ptr_factory_.GetWeakPtr()));
+    ScanForExternalWebApps(base::BindOnce(
+        &ExternalWebAppManager::SynchronizeExternalInstallOptions,
+        weak_ptr_factory_.GetWeakPtr(),
+        base::BindOnce(&OnExternalWebAppsSynchronized)));
   }
 }
 
 // static
 std::vector<ExternalInstallOptions>
 ExternalWebAppManager::ScanDirForExternalWebAppsForTesting(
+    std::unique_ptr<FileUtilsWrapper> file_utils,
     const base::FilePath& dir,
     Profile* profile) {
-  return ScanDir(dir, apps::DetermineUserType(profile));
+  return ScanDir(std::move(file_utils), dir, apps::DetermineUserType(profile));
 }
 
 void ExternalWebAppManager::ScanForExternalWebApps(ScanCallback callback) {
@@ -318,7 +192,8 @@ void ExternalWebAppManager::ScanForExternalWebApps(ScanCallback callback) {
       FROM_HERE,
       {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-      base::BindOnce(&ScanDir, dir, apps::DetermineUserType(profile_)),
+      base::BindOnce(&ScanDir, std::make_unique<FileUtilsWrapper>(),
+                     std::move(dir), apps::DetermineUserType(profile_)),
       std::move(callback));
 }
 
@@ -327,33 +202,26 @@ void ExternalWebAppManager::SkipStartupScanForTesting() {
 }
 
 void ExternalWebAppManager::SynchronizeAppsForTesting(
+    std::unique_ptr<FileUtilsWrapper> file_utils,
     std::vector<std::string> app_configs,
     PendingAppManager::SynchronizeCallback callback) {
-  std::vector<ExternalInstallOptions> install_options_list;
-  for (const std::string& app_config_string : app_configs) {
-    base::Optional<base::Value> app_config =
-        base::JSONReader::Read(app_config_string);
-    DCHECK(app_config);
-
-    base::Optional<ExternalInstallOptions> install_options =
-        ParseConfig(base::FilePath().AppendASCII("test"),
-                    apps::DetermineUserType(profile_), *app_config);
-    if (install_options)
-      install_options_list.push_back(std::move(*install_options));
-  }
-
-  pending_app_manager_->SynchronizeInstalledApps(
-      std::move(install_options_list), ExternalInstallSource::kExternalDefault,
-      std::move(callback));
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce(&SynchronizeAppsBlockingForTesting, std::move(file_utils),
+                     std::move(app_configs), apps::DetermineUserType(profile_)),
+      base::BindOnce(&ExternalWebAppManager::SynchronizeExternalInstallOptions,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void ExternalWebAppManager::OnScanForExternalWebApps(
+void ExternalWebAppManager::SynchronizeExternalInstallOptions(
+    PendingAppManager::SynchronizeCallback callback,
     std::vector<ExternalInstallOptions> desired_apps_install_options) {
   DCHECK(pending_app_manager_);
   pending_app_manager_->SynchronizeInstalledApps(
       std::move(desired_apps_install_options),
-      ExternalInstallSource::kExternalDefault,
-      base::BindOnce(&OnExternalWebAppsSynchronized));
+      ExternalInstallSource::kExternalDefault, std::move(callback));
 }
 
 }  //  namespace web_app

chrome/browser/web_applications/external_web_app_manager.h

@@ -12,6 +12,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/web_applications/components/external_install_options.h"
 #include "chrome/browser/web_applications/components/pending_app_manager.h"
+#include "chrome/browser/web_applications/file_utils_wrapper.h"
 
 namespace base {
 class FilePath;
@@ -38,8 +39,10 @@ class ExternalWebAppManager {
   //
   // This function performs file I/O, and must not be scheduled on UI threads.
   static std::vector<ExternalInstallOptions>
-  ScanDirForExternalWebAppsForTesting(const base::FilePath& dir,
-                                      Profile* profile);
+  ScanDirForExternalWebAppsForTesting(
+      std::unique_ptr<FileUtilsWrapper> file_utils,
+      const base::FilePath& dir,
+      Profile* profile);
 
   using ScanCallback =
       base::OnceCallback<void(std::vector<ExternalInstallOptions>)>;
@@ -49,11 +52,14 @@ class ExternalWebAppManager {
   static void SkipStartupScanForTesting();
 
   void SynchronizeAppsForTesting(
+      std::unique_ptr<FileUtilsWrapper> file_utils,
       std::vector<std::string> app_configs,
       PendingAppManager::SynchronizeCallback callback);
 
  private:
-  void OnScanForExternalWebApps(std::vector<ExternalInstallOptions>);
+  void SynchronizeExternalInstallOptions(
+      PendingAppManager::SynchronizeCallback callback,
+      std::vector<ExternalInstallOptions>);
 
   PendingAppManager* pending_app_manager_ = nullptr;
   Profile* const profile_;

chrome/browser/web_applications/external_web_app_manager_browsertest.cc

@@ -4,9 +4,14 @@
 
 #include "chrome/browser/web_applications/external_web_app_manager.h"
 
+#include "base/files/file_path.h"
+#include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/test/bind_test_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "chrome/browser/web_applications/test/test_file_utils.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
@@ -15,13 +20,6 @@
 #include "extensions/browser/test_extension_registry_observer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace {
-
-const char kChromeAppDirectory[] = "app";
-const char kChromeAppName[] = "App Test";
-
-}  // namespace
-
 namespace web_app {
 
 class ExternalWebAppManagerBrowserTest
@@ -35,6 +33,10 @@ class ExternalWebAppManagerBrowserTest
     return embedded_test_server()->GetURL("/web_apps/basic.html");
   }
 
+  const AppRegistrar& registrar() {
+    return WebAppProvider::Get(browser()->profile())->registrar();
+  }
+
   ~ExternalWebAppManagerBrowserTest() override = default;
 };
 
@@ -43,6 +45,8 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, UninstallAndReplace) {
   Profile* profile = browser()->profile();
 
   // Install Chrome app to be replaced.
+  const char kChromeAppDirectory[] = "app";
+  const char kChromeAppName[] = "App Test";
   const extensions::Extension* app = InstallExtensionWithSourceAndFlags(
       test_data_dir_.AppendASCII(kChromeAppDirectory), 1,
       extensions::Manifest::INTERNAL, extensions::Extension::NO_FLAGS);
@@ -57,6 +61,7 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, UninstallAndReplace) {
   WebAppProvider::Get(profile)
       ->external_web_app_manager_for_testing()
       .SynchronizeAppsForTesting(
+          std::make_unique<FileUtilsWrapper>(),
           {base::ReplaceStringPlaceholders(
               R"({
                 "app_url": "$1",
@@ -80,4 +85,71 @@ IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest, UninstallAndReplace) {
   EXPECT_EQ(app, uninstalled_app.get());
 }
 
+// TODO(crbug.com/1119710): Loading icon.png is flaky on Windows.
+#if defined(OS_WIN)
+#define MAYBE_OfflineManifest DISABLED_OfflineManifest
+#else
+#define MAYBE_OfflineManifest OfflineManifest
+#endif
+IN_PROC_BROWSER_TEST_F(ExternalWebAppManagerBrowserTest,
+                       MAYBE_OfflineManifest) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  Profile* profile = browser()->profile();
+
+  constexpr char kAppInstallUrl[] = "https://test.org/install.html";
+  constexpr char kAppName[] = "Offline app name";
+  constexpr char kAppUrl[] = "https://test.org/start.html";
+  constexpr char kAppScope[] = "https://test.org/";
+  AppId app_id = GenerateAppIdFromURL(GURL(kAppUrl));
+
+  base::FilePath source_root_dir;
+  CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &source_root_dir));
+  base::FilePath test_icon_path = source_root_dir.Append(GetChromeTestDataDir())
+                                      .AppendASCII("web_apps/blue-192.png");
+
+  EXPECT_FALSE(registrar().IsInstalled(app_id));
+
+  // Sync default web apps.
+  base::RunLoop sync_run_loop;
+  WebAppProvider::Get(profile)
+      ->external_web_app_manager_for_testing()
+      .SynchronizeAppsForTesting(
+          TestFileUtils::Create(
+              {{base::FilePath(FILE_PATH_LITERAL("test_dir/icon.png")),
+                test_icon_path}}),
+          {base::ReplaceStringPlaceholders(
+              R"({
+                "app_url": "$1",
+                "launch_container": "window",
+                "user_type": ["unmanaged"],
+                "offline_manifest": {
+                  "name": "$2",
+                  "start_url": "$3",
+                  "scope": "$4",
+                  "display": "minimal-ui",
+                  "theme_color_argb_hex": "AABBCCDD",
+                  "icon_any_pngs": ["icon.png"]
+                }
+              })",
+              {kAppInstallUrl, kAppName, kAppUrl, kAppScope}, nullptr)},
+          base::BindLambdaForTesting(
+              [&](std::map<GURL, InstallResultCode> install_results,
+                  std::map<GURL, bool> uninstall_results) {
+                EXPECT_EQ(install_results.at(GURL(kAppInstallUrl)),
+                          InstallResultCode::kSuccessNewInstall);
+                sync_run_loop.Quit();
+              }));
+  sync_run_loop.Run();
+
+  EXPECT_TRUE(registrar().IsInstalled(app_id));
+  EXPECT_EQ(registrar().GetAppShortName(app_id), kAppName);
+  EXPECT_EQ(registrar().GetAppLaunchURL(app_id).spec(), kAppUrl);
+  EXPECT_EQ(registrar().GetAppScope(app_id).spec(), kAppScope);
+  // theme_color must be installed opaque.
+  EXPECT_EQ(registrar().GetAppThemeColor(app_id),
+            SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD));
+  EXPECT_EQ(ReadAppIconPixel(profile, app_id, /*size=*/192, /*x=*/0, /*y=*/0),
+            SK_ColorBLUE);
+}
+
 }  // namespace web_app

chrome/browser/web_applications/external_web_app_manager_unittest.cc

@@ -117,7 +117,8 @@ class ScanDirForExternalWebAppsTest : public testing::Test {
   std::vector<ExternalInstallOptions> ScanTestDirForExternalWebApps(
       const std::string& dir) {
     return ExternalWebAppManager::ScanDirForExternalWebAppsForTesting(
-        GetTestDir(dir), CreateProfile().get());
+        std::make_unique<FileUtilsWrapper>(), GetTestDir(dir),
+        CreateProfile().get());
   }
 
   // Helper that creates simple test profile.

chrome/browser/web_applications/external_web_app_migration_browsertest.cc

@@ -185,7 +185,8 @@ class ExternalWebAppMigrationBrowserTest : public InProcessBrowserTest {
 
     WebAppProvider::Get(profile())
         ->external_web_app_manager_for_testing()
-        .SynchronizeAppsForTesting({external_web_app_config},
+        .SynchronizeAppsForTesting(std::make_unique<FileUtilsWrapper>(),
+                                   {external_web_app_config},
                                    std::move(callback));
 
     run_loop.Run();

chrome/browser/web_applications/external_web_app_utils.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/external_web_app_utils.h"
+
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/apps/user_type_filter.h"
+#include "chrome/browser/web_applications/components/external_app_install_features.h"
+#include "chrome/browser/web_applications/file_utils_wrapper.h"
+#include "third_party/blink/public/common/manifest/manifest_util.h"
+#include "ui/gfx/codec/png_codec.h"
+
+namespace web_app {
+
+namespace {
+
+// kAppUrl is a required string specifying a URL inside the scope of the web
+// app that contains a link to the app manifest.
+constexpr char kAppUrl[] = "app_url";
+
+// kHideFromUser is an optional boolean which controls whether we add
+// a shortcut to the relevant OS surface i.e. Application folder on macOS, Start
+// Menu on Windows and Linux, and launcher on Chrome OS. Defaults to false if
+// missing. If true, we also don't show the app in search or in app management
+// on Chrome OS.
+constexpr char kHideFromUser[] = "hide_from_user";
+
+// kCreateShortcuts is an optional boolean which controls whether OS
+// level shortcuts are created. On Chrome OS this controls whether the app is
+// pinned to the shelf.
+// The default value of kCreateShortcuts if false.
+constexpr char kCreateShortcuts[] = "create_shortcuts";
+
+// kFeatureName is an optional string parameter specifying a feature
+// associated with this app. The feature must be present in
+// |kExternalAppInstallFeatures| to be applicable.
+// If specified:
+//  - if the feature is enabled, the app will be installed
+//  - if the feature is not enabled, the app will be removed.
+constexpr char kFeatureName[] = "feature_name";
+
+// kLaunchContainer is a required string which can be "window" or "tab"
+// and controls what sort of container the web app is launched in.
+constexpr char kLaunchContainer[] = "launch_container";
+constexpr char kLaunchContainerTab[] = "tab";
+constexpr char kLaunchContainerWindow[] = "window";
+
+// kUninstallAndReplace is an optional array of strings which specifies App IDs
+// which the app is replacing. This will transfer OS attributes (e.g the source
+// app's shelf and app list positions on ChromeOS) and then uninstall the source
+// app.
+constexpr char kUninstallAndReplace[] = "uninstall_and_replace";
+
+// kOfflineManifest is a dictionary of manifest field values to use as an
+// install to avoid the expense of fetching the install URL to download the
+// app's true manifest. Next time the user visits the app it will undergo a
+// manifest update check and correct any differences from the site (except for
+// name and start_url).
+//
+// Why not use blink::ManifestParser?
+// blink::ManifestParser depends on substantial sections of the CSS parser which
+// is infeasible to run outside of the renderer process.
+constexpr char kOfflineManifest[] = "offline_manifest";
+
+// "name" manifest value to use for offline install. Cannot be updated.
+// TODO(crbug.com/1119699): Allow updating of name.
+constexpr char kOfflineManifestName[] = "name";
+
+// "start_url" manifest value to use for offline install. Cannot be updated.
+// TODO(crbug.com/1119699): Allow updating of start_url.
+constexpr char kOfflineManifestStartUrl[] = "start_url";
+
+// "scope" manifest value to use for offline install.
+constexpr char kOfflineManifestScope[] = "scope";
+
+// "display" manifest value to use for offline install.
+constexpr char kOfflineManifestDisplay[] = "display";
+
+// List of PNG files in the default web app config directory to use as the
+// icons for offline install. Will be installed with purpose "any".
+constexpr char kOfflineManifestIconAnyPngs[] = "icon_any_pngs";
+
+// Optional 8 value ARGB hex code to use as the "theme_color" manifest value.
+// Example:
+//   "theme_color_argb_hex": "FFFF0000"
+// is equivalent to
+//   "theme_color": "red"
+constexpr char kOfflineManifestThemeColorArgbHex[] = "theme_color_argb_hex";
+
+}  // namespace
+
+base::Optional<ExternalInstallOptions> ParseConfig(
+    FileUtilsWrapper& file_utils,
+    const base::FilePath& dir,
+    const base::FilePath& file,
+    const std::string& user_type,
+    const base::Value& app_config) {
+  if (app_config.type() != base::Value::Type::DICTIONARY) {
+    LOG(ERROR) << file << " was not a dictionary as the top level";
+    return base::nullopt;
+  }
+
+  if (!apps::UserTypeMatchesJsonUserType(
+          user_type, /*app_id=*/file.MaybeAsASCII(), &app_config,
+          /*default_user_types=*/nullptr)) {
+    // Already logged.
+    return base::nullopt;
+  }
+
+  const base::Value* value =
+      app_config.FindKeyOfType(kFeatureName, base::Value::Type::STRING);
+  if (value) {
+    // TODO(crbug.com/1104696): Add metrics for whether the app was
+    // enabled/disabled by the feature.
+    const std::string& feature_name = value->GetString();
+    VLOG(1) << file << " checking feature " << feature_name;
+    if (!IsExternalAppInstallFeatureEnabled(feature_name)) {
+      VLOG(1) << file << " feature not enabled";
+      return base::nullopt;
+    }
+  }
+
+  value = app_config.FindKeyOfType(kAppUrl, base::Value::Type::STRING);
+  if (!value) {
+    LOG(ERROR) << file << " had a missing " << kAppUrl;
+    return base::nullopt;
+  }
+  GURL app_url(value->GetString());
+  if (!app_url.is_valid()) {
+    LOG(ERROR) << file << " had an invalid " << kAppUrl;
+    return base::nullopt;
+  }
+
+  bool hide_from_user = false;
+  value = app_config.FindKey(kHideFromUser);
+  if (value) {
+    if (!value->is_bool()) {
+      LOG(ERROR) << file << " had an invalid " << kHideFromUser;
+      return base::nullopt;
+    }
+    hide_from_user = value->GetBool();
+  }
+
+  bool create_shortcuts = false;
+  value = app_config.FindKey(kCreateShortcuts);
+  if (value) {
+    if (!value->is_bool()) {
+      LOG(ERROR) << file << " had an invalid " << kCreateShortcuts;
+      return base::nullopt;
+    }
+    create_shortcuts = value->GetBool();
+  }
+
+  // It doesn't make sense to hide the app and also create shortcuts for it.
+  DCHECK(!(hide_from_user && create_shortcuts));
+
+  value = app_config.FindKeyOfType(kLaunchContainer, base::Value::Type::STRING);
+  if (!value) {
+    LOG(ERROR) << file << " had an invalid " << kLaunchContainer;
+    return base::nullopt;
+  }
+  std::string launch_container_str = value->GetString();
+  auto user_display_mode = DisplayMode::kBrowser;
+  if (launch_container_str == kLaunchContainerTab) {
+    user_display_mode = DisplayMode::kBrowser;
+  } else if (launch_container_str == kLaunchContainerWindow) {
+    user_display_mode = DisplayMode::kStandalone;
+  } else {
+    LOG(ERROR) << file << " had an invalid " << kLaunchContainer;
+    return base::nullopt;
+  }
+
+  value = app_config.FindKey(kUninstallAndReplace);
+  std::vector<AppId> uninstall_and_replace_ids;
+  if (value) {
+    if (!value->is_list()) {
+      LOG(ERROR) << file << " had an invalid " << kUninstallAndReplace;
+      return base::nullopt;
+    }
+    base::Value::ConstListView uninstall_and_replace_values = value->GetList();
+
+    bool had_error = false;
+    for (const auto& app_id_value : uninstall_and_replace_values) {
+      if (!app_id_value.is_string()) {
+        had_error = true;
+        LOG(ERROR) << file << " had an invalid " << kUninstallAndReplace
+                   << " entry";
+        break;
+      }
+      uninstall_and_replace_ids.push_back(app_id_value.GetString());
+    }
+    if (had_error)
+      return base::nullopt;
+  }
+
+  value = app_config.FindDictKey(kOfflineManifest);
+  WebApplicationInfoFactory app_info_factory;
+  if (value)
+    app_info_factory = ParseOfflineManifest(file_utils, dir, file, *value);
+
+  ExternalInstallOptions install_options(
+      std::move(app_url), user_display_mode,
+      ExternalInstallSource::kExternalDefault);
+  install_options.add_to_applications_menu = !hide_from_user;
+  install_options.add_to_search = !hide_from_user;
+  install_options.add_to_management = !hide_from_user;
+  install_options.add_to_desktop = create_shortcuts;
+  install_options.add_to_quick_launch_bar = create_shortcuts;
+  install_options.require_manifest = true;
+  install_options.uninstall_and_replace = std::move(uninstall_and_replace_ids);
+  install_options.app_info_factory = std::move(app_info_factory);
+
+  return install_options;
+}
+
+WebApplicationInfoFactory ParseOfflineManifest(
+    FileUtilsWrapper& file_utils,
+    const base::FilePath& dir,
+    const base::FilePath& file,
+    const base::Value& offline_manifest) {
+  WebApplicationInfo app_info;
+
+  // name
+  const std::string* name_string =
+      offline_manifest.FindStringKey(kOfflineManifestName);
+  if (!name_string) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " " << kOfflineManifestName
+               << " missing or invalid.";
+    return {};
+  }
+  if (!base::UTF8ToUTF16(name_string->data(), name_string->size(),
+                         &app_info.title) ||
+      app_info.title.empty()) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " " << kOfflineManifestName
+               << " invalid: " << *name_string;
+    return {};
+  }
+
+  // start_url
+  const std::string* start_url_string =
+      offline_manifest.FindStringKey(kOfflineManifestStartUrl);
+  if (!start_url_string) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestStartUrl << " missing or invalid.";
+    return {};
+  }
+  app_info.app_url = GURL(*start_url_string);
+  if (!app_info.app_url.is_valid()) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestStartUrl << " invalid: " << *start_url_string;
+    return {};
+  }
+
+  // scope
+  const std::string* scope_string =
+      offline_manifest.FindStringKey(kOfflineManifestScope);
+  if (!scope_string) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestScope << " missing or invalid.";
+    return {};
+  }
+  app_info.scope = GURL(*scope_string);
+  if (!app_info.scope.is_valid()) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestScope << " invalid: " << *scope_string;
+    return {};
+  }
+  if (!base::StartsWith(app_info.app_url.path(), app_info.scope.path(),
+                        base::CompareCase::SENSITIVE)) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestScope << " (" << app_info.app_url
+               << ") not within " << kOfflineManifestScope << " ("
+               << app_info.scope << ").";
+    return {};
+  }
+
+  // display
+  const std::string* display_string =
+      offline_manifest.FindStringKey(kOfflineManifestDisplay);
+  if (!display_string) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestDisplay << " missing or invalid.";
+    return {};
+  }
+  DisplayMode display = blink::DisplayModeFromString(*display_string);
+  if (display == DisplayMode::kUndefined) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestDisplay << " invalid: " << display_string;
+    return {};
+  }
+  app_info.display_mode = display;
+
+  // icon_any_pngs
+  const base::Value* icon_files =
+      offline_manifest.FindListKey(kOfflineManifestIconAnyPngs);
+  if (!icon_files || icon_files->GetList().empty()) {
+    LOG(ERROR) << file << " " << kOfflineManifest << " "
+               << kOfflineManifestIconAnyPngs << " missing, empty or invalid.";
+    return {};
+  }
+  for (const base::Value& icon_file : icon_files->GetList()) {
+    if (!icon_file.is_string()) {
+      LOG(ERROR) << file << " " << kOfflineManifest << " "
+                 << kOfflineManifestIconAnyPngs << " " << icon_file
+                 << " invalid.";
+      return {};
+    }
+
+    base::FilePath icon_path = dir.AppendASCII(icon_file.GetString());
+    std::string icon_data;
+    if (!file_utils.ReadFileToString(icon_path, &icon_data)) {
+      LOG(ERROR) << file << " " << kOfflineManifest << " "
+                 << kOfflineManifestIconAnyPngs << " " << icon_file
+                 << " failed to read.";
+      return {};
+    }
+
+    SkBitmap bitmap;
+    if (!gfx::PNGCodec::Decode(
+            reinterpret_cast<const unsigned char*>(icon_data.c_str()),
+            icon_data.size(), &bitmap)) {
+      LOG(ERROR) << file << " " << kOfflineManifest << " "
+                 << kOfflineManifestIconAnyPngs << " " << icon_file
+                 << " failed to decode.";
+      return {};
+    }
+
+    if (bitmap.width() != bitmap.height()) {
+      LOG(ERROR) << file << " " << kOfflineManifest << " "
+                 << kOfflineManifestIconAnyPngs << " " << icon_file
+                 << " must be square: " << bitmap.width() << "x"
+                 << bitmap.height();
+      return {};
+    }
+
+    app_info.icon_bitmaps_any[bitmap.width()] = std::move(bitmap);
+  }
+  DCHECK(!app_info.icon_bitmaps_any.empty());
+
+  // theme_color_argb_hex (optional)
+  const base::Value* theme_color_value =
+      offline_manifest.FindKey(kOfflineManifestThemeColorArgbHex);
+  if (theme_color_value) {
+    const std::string* theme_color_argb_hex =
+        theme_color_value->is_string() ? &theme_color_value->GetString()
+                                       : nullptr;
+    SkColor theme_color;
+    if (!theme_color_argb_hex ||
+        !base::HexStringToUInt(*theme_color_argb_hex, &theme_color)) {
+      LOG(ERROR) << file << " " << kOfflineManifest << " "
+                 << kOfflineManifestThemeColorArgbHex
+                 << " invalid: " << *theme_color_value;
+      return {};
+    }
+    app_info.theme_color = SkColorSetA(theme_color, SK_AlphaOPAQUE);
+  }
+
+  return base::BindRepeating(
+      &std::make_unique<WebApplicationInfo, const WebApplicationInfo&>,
+      std::move(app_info));
+}
+
+}  // namespace web_app

chrome/browser/web_applications/external_web_app_utils.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTERNAL_WEB_APP_UTILS_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_EXTERNAL_WEB_APP_UTILS_H_
+
+#include <string>
+
+#include "base/optional.h"
+#include "chrome/browser/web_applications/components/external_install_options.h"
+
+namespace base {
+class FilePath;
+class Value;
+}  // namespace base
+
+namespace web_app {
+
+class FileUtilsWrapper;
+
+base::Optional<ExternalInstallOptions> ParseConfig(
+    FileUtilsWrapper& file_utils,
+    const base::FilePath& dir,
+    const base::FilePath& file,
+    const std::string& user_type,
+    const base::Value& app_config);
+
+WebApplicationInfoFactory ParseOfflineManifest(
+    FileUtilsWrapper& file_utils,
+    const base::FilePath& dir,
+    const base::FilePath& file,
+    const base::Value& offline_manifest);
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_EXTERNAL_WEB_APP_UTILS_H_

chrome/browser/web_applications/external_web_app_utils_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/external_web_app_utils.h"
+
+#include "base/files/file_path.h"
+#include "base/json/json_reader.h"
+#include "base/path_service.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/web_applications/test/test_file_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace web_app {
+
+class ExternalWebAppUtilsTest : public testing::Test {
+ public:
+  ExternalWebAppUtilsTest() = default;
+  ~ExternalWebAppUtilsTest() override = default;
+
+  // testing::Test:
+  void SetUp() override {
+    testing::Test::SetUp();
+
+    base::FilePath source_root_dir;
+    CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &source_root_dir));
+    file_utils_ = TestFileUtils::Create({
+        {base::FilePath(FILE_PATH_LITERAL("test_dir/icon.png")),
+         source_root_dir.AppendASCII("chrome/test/data/web_apps/blue-192.png")},
+        {base::FilePath(FILE_PATH_LITERAL("test_dir/basic.html")),
+         source_root_dir.AppendASCII("chrome/test/data/web_apps/basic.html")},
+    });
+  }
+
+  WebApplicationInfoFactory ParseOfflineManifest(
+      const char* offline_manifest_string) {
+    base::Optional<base::Value> offline_manifest =
+        base::JSONReader::Read(offline_manifest_string);
+    DCHECK(offline_manifest);
+    return ::web_app::ParseOfflineManifest(
+        *file_utils_, base::FilePath(FILE_PATH_LITERAL("test_dir")),
+        base::FilePath(FILE_PATH_LITERAL("test_dir/test.json")),
+        *offline_manifest);
+  }
+
+ protected:
+  std::unique_ptr<TestFileUtils> file_utils_;
+};
+
+// ParseConfig() is tested by ScanDirForExternalWebAppsTest.
+
+// TODO(crbug.com/1119710): Loading icon.png is flaky on Windows.
+#if defined(OS_WIN)
+#define MAYBE_OfflineManifestValid DISABLED_OfflineManifestValid
+#else
+#define MAYBE_OfflineManifestValid OfflineManifestValid
+#endif
+TEST_F(ExternalWebAppUtilsTest, MAYBE_OfflineManifestValid) {
+  std::unique_ptr<WebApplicationInfo> app_info = ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"],
+      "theme_color_argb_hex": "AABBCCDD"
+    }
+  )")
+                                                     .Run();
+  EXPECT_TRUE(app_info);
+  EXPECT_EQ(app_info->title, base::UTF8ToUTF16("Test App"));
+  EXPECT_EQ(app_info->app_url, GURL("https://test.org/start.html"));
+  EXPECT_EQ(app_info->scope, GURL("https://test.org/"));
+  EXPECT_EQ(app_info->display_mode, DisplayMode::kStandalone);
+  EXPECT_EQ(app_info->icon_bitmaps_any.size(), 1u);
+  EXPECT_EQ(app_info->icon_bitmaps_any.at(192).getColor(0, 0), SK_ColorBLUE);
+  EXPECT_EQ(app_info->theme_color, SkColorSetARGB(0xFF, 0xBB, 0xCC, 0xDD));
+}
+
+TEST_F(ExternalWebAppUtilsTest, OfflineManifestName) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "name is required";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": 400,
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "name is string";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "name is non-empty";
+}
+
+TEST_F(ExternalWebAppUtilsTest, OfflineManifestStartUrl) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "start_url is required";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "not a url",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "start_url is valid";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/inner/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "start_url is within scope";
+}
+
+TEST_F(ExternalWebAppUtilsTest, OfflineManifestScope) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "scope is required";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "not a url",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "start_url is valid";
+}
+
+// TODO(crbug.com/1119710): Loading icon.png is flaky on Windows.
+#if defined(OS_WIN)
+#define MAYBE_OfflineManifestDisplay DISABLED_OfflineManifestDisplay
+#else
+#define MAYBE_OfflineManifestDisplay OfflineManifestDisplay
+#endif
+TEST_F(ExternalWebAppUtilsTest, MAYBE_OfflineManifestDisplay) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display is required";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "tab",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display is valid";
+
+  EXPECT_TRUE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display can be standalone";
+  EXPECT_TRUE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "browser",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display can be browser";
+  EXPECT_TRUE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "minimal-ui",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display can be minimal-ui";
+  EXPECT_TRUE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "fullscreen",
+      "icon_any_pngs": ["icon.png"]
+    }
+  )")) << "display can be fullscreen";
+}
+
+TEST_F(ExternalWebAppUtilsTest, OfflineManifestIconAnyPngs) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone"
+    }
+  )")) << "icon_any_pngs is required";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": "icon.png"
+    }
+  )")) << "icon_any_pngs is valid";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": [{
+        "src": "https://test.org/icon.png",
+        "sizes": "144x144",
+        "type": "image/png"
+      }]
+    }
+  )")) << "icon_any_pngs is valid";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["does-not-exist.png"]
+    }
+  )")) << "icon_any_pngs exists";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["basic.html"]
+    }
+  )")) << "icon_any_pngs is a PNG";
+}
+
+TEST_F(ExternalWebAppUtilsTest, OfflineManifestThemeColorArgbHex) {
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"],
+      "theme_color_argb_hex": 12345
+    }
+  )")) << "theme_color_argb_hex is valid";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"],
+      "theme_color_argb_hex": "blue"
+    }
+  )")) << "theme_color_argb_hex is valid";
+
+  EXPECT_FALSE(ParseOfflineManifest(R"(
+    {
+      "name": "Test App",
+      "start_url": "https://test.org/start.html",
+      "scope": "https://test.org/",
+      "display": "standalone",
+      "icon_any_pngs": ["icon.png"],
+      "theme_color_argb_hex": "#ff0000"
+    }
+  )")) << "theme_color_argb_hex is valid";
+}
+
+}  // namespace web_app

chrome/browser/web_applications/file_utils_wrapper.h

@@ -54,7 +54,8 @@ class FileUtilsWrapper {
 
   bool IsDirectoryEmpty(const base::FilePath& dir_path);
 
-  bool ReadFileToString(const base::FilePath& path, std::string* contents);
+  virtual bool ReadFileToString(const base::FilePath& path,
+                                std::string* contents);
 
   bool DeleteFile(const base::FilePath& path, bool recursive);
 

chrome/browser/web_applications/manifest_update_manager_browsertest.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
 #include "chrome/browser/web_applications/components/app_icon_manager.h"
 #include "chrome/browser/web_applications/components/app_registry_controller.h"
 #include "chrome/browser/web_applications/components/app_shortcut_manager.h"
@@ -1051,16 +1052,9 @@ IN_PROC_BROWSER_TEST_P(ManifestUpdateManagerBrowserTest,
                                       ManifestUpdateResult::kAppUpdated, 1);
   AwaitShortcutsUpdated(SK_ColorBLUE);
 
-  // Check that the installed icon is now blue.
-  base::RunLoop run_loop;
-  GetProvider().icon_manager().ReadIcons(
-      app_id, IconPurpose::ANY, {192},
-      base::BindLambdaForTesting(
-          [&run_loop](std::map<SquareSizePx, SkBitmap> icon_bitmaps) {
-            run_loop.Quit();
-            EXPECT_EQ(icon_bitmaps.at(192).getColor(0, 0), SK_ColorBLUE);
-          }));
-  run_loop.Run();
+  EXPECT_EQ(ReadAppIconPixel(browser()->profile(), app_id, /*size=*/192,
+                             /*x=*/0, /*y=*/0),
+            SK_ColorBLUE);
 }
 
 IN_PROC_BROWSER_TEST_P(ManifestUpdateManagerBrowserTest,
@@ -1114,17 +1108,12 @@ IN_PROC_BROWSER_TEST_P(ManifestUpdateManagerBrowserTest,
   histogram_tester_.ExpectBucketCount(
       kUpdateHistogramName, ManifestUpdateResult::kIconDownloadFailed, 1);
 
-  // Check that the installed icon is still black.
-  base::RunLoop run_loop;
-  GetProvider().icon_manager().ReadIcons(
-      app_id, IconPurpose::ANY, {48, 192},
-      base::BindLambdaForTesting(
-          [&run_loop](std::map<SquareSizePx, SkBitmap> icon_bitmaps) {
-            run_loop.Quit();
-            EXPECT_EQ(icon_bitmaps.at(48).getColor(0, 0), SK_ColorBLACK);
-            EXPECT_EQ(icon_bitmaps.at(192).getColor(0, 0), SK_ColorBLACK);
-          }));
-  run_loop.Run();
+  EXPECT_EQ(ReadAppIconPixel(browser()->profile(), app_id, /*size=*/48, /*x=*/0,
+                             /*y=*/0),
+            SK_ColorBLACK);
+  EXPECT_EQ(ReadAppIconPixel(browser()->profile(), app_id, /*size=*/192,
+                             /*x=*/0, /*y=*/0),
+            SK_ColorBLACK);
 }
 
 class ManifestUpdateManagerSystemAppBrowserTest

chrome/browser/web_applications/pending_app_manager_impl.cc

@@ -281,7 +281,13 @@ void PendingAppManagerImpl::CurrentInstallationFinished(
     bool is_local_resource =
         launch_url.scheme() == content::kChromeUIScheme ||
         launch_url.scheme() == content::kChromeUIUntrustedScheme;
-    if (!launch_url.is_empty() && !is_local_resource)
+    // TODO(crbug.com/809304): Call CreateWebContentsIfNecessary() instead of
+    // checking web_contents_ once major migration of default hosted apps to web
+    // apps has completed.
+    // Temporarily using offline manifest migrations (in which |web_contents_|
+    // is nullptr) in order to avoid overwhelming migrated-to web apps with hits
+    // for service worker registrations.
+    if (!launch_url.is_empty() && !is_local_resource && web_contents_)
       pending_registrations_.push_back(launch_url);
   }
 

chrome/browser/web_applications/test/test_file_utils.cc

@@ -2,13 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/web_applications/test/test_file_utils.h"
+
 #include <utility>
 
-#include "chrome/browser/web_applications/test/test_file_utils.h"
+#include "base/files/file_path.h"
 
 namespace web_app {
 
-TestFileUtils::TestFileUtils() = default;
+std::unique_ptr<TestFileUtils> TestFileUtils::Create(
+    std::map<base::FilePath, base::FilePath> read_file_rerouting) {
+  return std::make_unique<TestFileUtils>(read_file_rerouting);
+}
+
+TestFileUtils::TestFileUtils(
+    std::map<base::FilePath, base::FilePath> read_file_rerouting)
+    : read_file_rerouting_(read_file_rerouting) {}
 
 TestFileUtils::TestFileUtils(const TestFileUtils&) = default;
 
@@ -46,6 +55,16 @@ int TestFileUtils::WriteFile(const base::FilePath& filename,
   return FileUtilsWrapper::WriteFile(filename, data, size);
 }
 
+bool TestFileUtils::ReadFileToString(const base::FilePath& path,
+                                     std::string* contents) {
+  for (const std::pair<const base::FilePath, base::FilePath>& route :
+       read_file_rerouting_) {
+    if (route.first == path)
+      return FileUtilsWrapper::ReadFileToString(route.second, contents);
+  }
+  return FileUtilsWrapper::ReadFileToString(path, contents);
+}
+
 bool TestFileUtils::DeleteFileRecursively(const base::FilePath& path) {
   return delete_file_recursively_result_
              ? *delete_file_recursively_result_

chrome/browser/web_applications/test/test_file_utils.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_FILE_UTILS_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_FILE_UTILS_H_
 
+#include <map>
 #include <memory>
 
 #include "base/macros.h"
@@ -16,7 +17,13 @@ namespace web_app {
 // A testing implementation to intercept calls to the file system.
 class TestFileUtils : public FileUtilsWrapper {
  public:
-  TestFileUtils();
+  // Initializer list type deduction does not work through std::make_unique so
+  // provide this helper function.
+  static std::unique_ptr<TestFileUtils> Create(
+      std::map<base::FilePath, base::FilePath> read_file_rerouting);
+
+  explicit TestFileUtils(
+      std::map<base::FilePath, base::FilePath> read_file_rerouting = {});
   TestFileUtils(const TestFileUtils&);
   ~TestFileUtils() override;
 
@@ -25,6 +32,8 @@ class TestFileUtils : public FileUtilsWrapper {
   int WriteFile(const base::FilePath& filename,
                 const char* data,
                 int size) override;
+  bool ReadFileToString(const base::FilePath& path,
+                        std::string* contents) override;
   bool DeleteFileRecursively(const base::FilePath& path) override;
 
   static constexpr int kNoLimit = -1;
@@ -35,6 +44,7 @@ class TestFileUtils : public FileUtilsWrapper {
   void SetNextDeleteFileRecursivelyResult(base::Optional<bool> delete_result);
 
  private:
+  std::map<base::FilePath, base::FilePath> read_file_rerouting_;
   base::Optional<bool> delete_file_recursively_result_;
   int remaining_disk_space_ = kNoLimit;