Add service worker registration options for default web app config

This CL adds two fields to the default web app config JSON:
 - load_and_await_service_worker_registration: bool
 - service_worker_install_url: URL string

These allow default apps to control their service worker registration
behaviour during the install flow.

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

chrome/browser/web_applications/components/external_install_options.cc

@@ -40,7 +40,9 @@ bool ExternalInstallOptions::operator==(
                   is_disabled, override_previous_user_uninstall,
                   bypass_service_worker_check, require_manifest,
                   force_reinstall, wait_for_windows_closed, install_placeholder,
-                  reinstall_placeholder, uninstall_and_replace,
+                  reinstall_placeholder,
+                  load_and_await_service_worker_registration,
+                  service_worker_registration_url, uninstall_and_replace,
                   additional_search_terms) ==
          std::tie(other.install_url, other.user_display_mode,
                   other.install_source, other.add_to_applications_menu,
@@ -50,6 +52,8 @@ bool ExternalInstallOptions::operator==(
                   other.bypass_service_worker_check, other.require_manifest,
                   other.force_reinstall, other.wait_for_windows_closed,
                   other.install_placeholder, other.reinstall_placeholder,
+                  other.load_and_await_service_worker_registration,
+                  other.service_worker_registration_url,
                   other.uninstall_and_replace, other.additional_search_terms);
 }
 
@@ -80,6 +84,10 @@ std::ostream& operator<<(std::ostream& out,
              << install_options.install_placeholder
              << "\n reinstall_placeholder: "
              << install_options.reinstall_placeholder
+             << "\n load_and_await_service_worker_registration: "
+             << install_options.load_and_await_service_worker_registration
+             << "\n service_worker_registration_url: "
+             << install_options.service_worker_registration_url.value_or(GURL())
              << "\n uninstall_and_replace:\n  "
              << base::JoinString(install_options.uninstall_and_replace, "\n  ")
              << "\n additional_search_terms:\n "

chrome/browser/web_applications/components/external_install_options.h

@@ -103,6 +103,17 @@ struct ExternalInstallOptions {
   // it.
   bool reinstall_placeholder = false;
 
+  // Whether we should load |service_worker_registration_url| after successful
+  // installation to allow the site to install its service worker and set up
+  // offline caching.
+  bool load_and_await_service_worker_registration = true;
+
+  // The URL to use for service worker registration. This is
+  // configurable by sites that wish to be able to track install metrics of the
+  // install_url separate from the service worker registration step. Defaults to
+  // install_url if unset.
+  base::Optional<GURL> service_worker_registration_url;
+
   // A list of app_ids that the Web App System should attempt to uninstall and
   // replace with this app (e.g maintain shelf pins, app list positions).
   std::vector<AppId> uninstall_and_replace;

chrome/browser/web_applications/components/pending_app_manager.cc

@@ -113,6 +113,11 @@ void PendingAppManager::ClearRegistrationCallbackForTesting() {
   registration_callback_ = RegistrationCallback();
 }
 
+void PendingAppManager::SetRegistrationsCompleteCallbackForTesting(
+    base::OnceClosure callback) {
+  registrations_complete_callback_ = std::move(callback);
+}
+
 void PendingAppManager::OnRegistrationFinished(const GURL& install_url,
                                                RegistrationResultCode result) {
   if (registration_callback_)

chrome/browser/web_applications/components/pending_app_manager.h

@@ -112,6 +112,7 @@ class PendingAppManager {
 
   void SetRegistrationCallbackForTesting(RegistrationCallback callback);
   void ClearRegistrationCallbackForTesting();
+  void SetRegistrationsCompleteCallbackForTesting(base::OnceClosure callback);
   void ClearSynchronizeRequestsForTesting();
 
   virtual void Shutdown() = 0;
@@ -128,6 +129,8 @@ class PendingAppManager {
   virtual void OnRegistrationFinished(const GURL& launch_url,
                                       RegistrationResultCode result);
 
+  base::OnceClosure registrations_complete_callback_;
+
  private:
   struct SynchronizeRequest {
     SynchronizeRequest(SynchronizeCallback callback, int remaining_requests);

chrome/browser/web_applications/external_web_app_utils.cc

@@ -51,6 +51,22 @@ constexpr char kLaunchContainer[] = "launch_container";
 constexpr char kLaunchContainerTab[] = "tab";
 constexpr char kLaunchContainerWindow[] = "window";
 
+// kLoadAndAwaitServiceWorkerRegistration is an optional bool that specifies
+// whether to fetch the |kServiceWorkerRegistrationUrl| after installation to
+// allow time for the app to register its service worker. This is done as a
+// second pass after install in order to not block the installation of other
+// background installed apps. No fetch is made if the service worker has already
+// been registered by the |kAppUrl|.
+// Defaults to true.
+constexpr char kLoadAndAwaitServiceWorkerRegistration[] =
+    "load_and_await_service_worker_registration";
+
+// kServiceWorkerRegistrationUrl is an optional string specifying the URL to use
+// for the above |kLoadAndAwaitServiceWorkerRegistration|.
+// Defaults to the |kAppUrl|.
+constexpr char kServiceWorkerRegistrationUrl[] =
+    "service_worker_registration_url";
+
 // 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
@@ -176,6 +192,36 @@ base::Optional<ExternalInstallOptions> ParseConfig(
     return base::nullopt;
   }
 
+  bool load_and_await_service_worker_registration = true;
+  value = app_config.FindKey(kLoadAndAwaitServiceWorkerRegistration);
+  if (value) {
+    if (!value->is_bool()) {
+      LOG(ERROR) << file << " had an invalid "
+                 << kLoadAndAwaitServiceWorkerRegistration;
+      return base::nullopt;
+    }
+    load_and_await_service_worker_registration = value->GetBool();
+  }
+
+  base::Optional<GURL> service_worker_registration_url;
+  value = app_config.FindKey(kServiceWorkerRegistrationUrl);
+  if (value) {
+    if (!load_and_await_service_worker_registration) {
+      LOG(ERROR) << file << " should not specify a "
+                 << kServiceWorkerRegistrationUrl << " while "
+                 << kLoadAndAwaitServiceWorkerRegistration << " is disabled";
+    }
+    if (!value->is_string()) {
+      LOG(ERROR) << file << " had an invalid " << kServiceWorkerRegistrationUrl;
+      return base::nullopt;
+    }
+    service_worker_registration_url.emplace(value->GetString());
+    if (!service_worker_registration_url->is_valid()) {
+      LOG(ERROR) << file << " had an invalid " << kServiceWorkerRegistrationUrl;
+      return base::nullopt;
+    }
+  }
+
   value = app_config.FindKey(kUninstallAndReplace);
   std::vector<AppId> uninstall_and_replace_ids;
   if (value) {
@@ -214,6 +260,10 @@ base::Optional<ExternalInstallOptions> ParseConfig(
   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.load_and_await_service_worker_registration =
+      load_and_await_service_worker_registration;
+  install_options.service_worker_registration_url =
+      service_worker_registration_url;
   install_options.app_info_factory = std::move(app_info_factory);
 
   return install_options;

chrome/browser/web_applications/pending_app_manager_impl.cc

@@ -243,8 +243,11 @@ void PendingAppManagerImpl::OnWebContentsReady(WebAppUrlLoader::Result) {
 }
 
 bool PendingAppManagerImpl::RunNextRegistration() {
-  if (pending_registrations_.empty())
+  if (pending_registrations_.empty()) {
+    if (registrations_complete_callback_)
+      std::move(registrations_complete_callback_).Run();
     return false;
+  }
 
   GURL url_to_check = std::move(pending_registrations_.front());
   pending_registrations_.pop_front();
@@ -275,22 +278,9 @@ void PendingAppManagerImpl::OnInstalled(PendingAppInstallTask::Result result) {
 void PendingAppManagerImpl::CurrentInstallationFinished(
     const base::Optional<AppId>& app_id,
     InstallResultCode code) {
-  if (app_id && code == InstallResultCode::kSuccessNewInstall &&
-      base::FeatureList::IsEnabled(
-          features::kDesktopPWAsCacheDuringDefaultInstall)) {
-    const GURL& install_url =
-        current_install_->task->install_options().install_url;
-    bool is_local_resource =
-        install_url.scheme() == content::kChromeUIScheme ||
-        install_url.scheme() == content::kChromeUIUntrustedScheme;
-    // 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 (!install_url.is_empty() && !is_local_resource && web_contents_)
-      pending_registrations_.push_back(install_url);
+  if (app_id && code == InstallResultCode::kSuccessNewInstall) {
+    MaybeEnqueueServiceWorkerRegistration(
+        current_install_->task->install_options());
   }
 
   // Post a task to avoid InstallableManager crashing and do so before
@@ -304,4 +294,35 @@ void PendingAppManagerImpl::CurrentInstallationFinished(
       .Run(task_and_callback->task->install_options().install_url, code);
 }
 
+void PendingAppManagerImpl::MaybeEnqueueServiceWorkerRegistration(
+    const ExternalInstallOptions& install_options) {
+  if (!base::FeatureList::IsEnabled(
+          features::kDesktopPWAsCacheDuringDefaultInstall)) {
+    return;
+  }
+
+  if (!install_options.load_and_await_service_worker_registration)
+    return;
+
+  // 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 (!web_contents_)
+    return;
+
+  GURL url = install_options.service_worker_registration_url.value_or(
+      install_options.install_url);
+  if (url.is_empty())
+    return;
+  if (url.scheme() == content::kChromeUIScheme)
+    return;
+  if (url.scheme() == content::kChromeUIUntrustedScheme)
+    return;
+
+  pending_registrations_.push_back(url);
+}
+
 }  // namespace web_app

chrome/browser/web_applications/pending_app_manager_impl.h

@@ -87,6 +87,9 @@ class PendingAppManagerImpl : public PendingAppManager {
   void CurrentInstallationFinished(const base::Optional<std::string>& app_id,
                                    InstallResultCode code);
 
+  void MaybeEnqueueServiceWorkerRegistration(
+      const ExternalInstallOptions& install_options);
+
   Profile* const profile_;
   ExternallyInstalledWebAppPrefs externally_installed_app_prefs_;
 

chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc

@@ -317,6 +317,48 @@ IN_PROC_BROWSER_TEST_P(PendingAppManagerImplBrowserTest, RegistrationSucceeds) {
       content::ServiceWorkerCapability::SERVICE_WORKER_WITH_FETCH_HANDLER);
 }
 
+IN_PROC_BROWSER_TEST_P(PendingAppManagerImplBrowserTest,
+                       RegistrationAlternateUrlSucceeds) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  GURL install_url(
+      embedded_test_server()->GetURL("/web_apps/no_service_worker.html"));
+  GURL registration_url =
+      embedded_test_server()->GetURL("/web_apps/basic.html");
+
+  ExternalInstallOptions install_options = CreateInstallOptions(install_url);
+  install_options.bypass_service_worker_check = true;
+  install_options.service_worker_registration_url = registration_url;
+  InstallApp(std::move(install_options));
+  EXPECT_EQ(InstallResultCode::kSuccessNewInstall, result_code_.value());
+  WebAppRegistrationWaiter(&pending_app_manager())
+      .AwaitNextRegistration(registration_url,
+                             RegistrationResultCode::kSuccess);
+  CheckServiceWorkerStatus(
+      install_url,
+      content::ServiceWorkerCapability::SERVICE_WORKER_WITH_FETCH_HANDLER);
+}
+
+IN_PROC_BROWSER_TEST_P(PendingAppManagerImplBrowserTest, RegistrationSkipped) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Delay service worker registration to second load to simulate it not loading
+  // during the initial install pass.
+  GURL install_url(embedded_test_server()->GetURL(
+      "/web_apps/service_worker_on_second_load.html"));
+
+  ExternalInstallOptions install_options = CreateInstallOptions(install_url);
+  install_options.bypass_service_worker_check = true;
+  install_options.load_and_await_service_worker_registration = false;
+  WebAppRegistrationWaiter waiter(&pending_app_manager());
+  InstallApp(std::move(install_options));
+  waiter.AwaitRegistrationsComplete();
+
+  EXPECT_EQ(InstallResultCode::kSuccessNewInstall, result_code_.value());
+  CheckServiceWorkerStatus(install_url,
+                           content::ServiceWorkerCapability::NO_SERVICE_WORKER);
+}
+
 IN_PROC_BROWSER_TEST_P(PendingAppManagerImplBrowserTest, AlreadyRegistered) {
   ASSERT_TRUE(embedded_test_server()->Start());
   {

chrome/browser/web_applications/test/web_app_registration_waiter.cc

@@ -16,6 +16,8 @@ WebAppRegistrationWaiter::WebAppRegistrationWaiter(PendingAppManager* manager)
         CHECK_EQ(code_, code);
         run_loop_.Quit();
       }));
+  manager_->SetRegistrationsCompleteCallbackForTesting(
+      complete_run_loop_.QuitClosure());
 }
 
 WebAppRegistrationWaiter::~WebAppRegistrationWaiter() {
@@ -30,4 +32,8 @@ void WebAppRegistrationWaiter::AwaitNextRegistration(
   run_loop_.Run();
 }
 
+void WebAppRegistrationWaiter::AwaitRegistrationsComplete() {
+  complete_run_loop_.Run();
+}
+
 }  // namespace web_app

chrome/browser/web_applications/test/web_app_registration_waiter.h

@@ -19,11 +19,15 @@ class WebAppRegistrationWaiter {
   void AwaitNextRegistration(const GURL& install_url,
                              RegistrationResultCode code);
 
+  void AwaitRegistrationsComplete();
+
  private:
   PendingAppManager* const manager_;
   base::RunLoop run_loop_;
   GURL install_url_;
   RegistrationResultCode code_;
+
+  base::RunLoop complete_run_loop_;
 };
 
 }  // namespace web_app

chrome/test/data/web_apps/service_worker.js

@@ -9,6 +9,7 @@ const urlsToCache = [
   'basic-48.png',
   'basic.html',
   'basic.json',
+  'no_service_worker.html',
 ];
 
 self.addEventListener('install', (event) => {