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}
diff --git a/chrome/browser/web_applications/components/external_install_options.cc b/chrome/browser/web_applications/components/external_install_options.cc
index 6777c57..3adde60 100644
--- a/chrome/browser/web_applications/components/external_install_options.cc
+++ b/chrome/browser/web_applications/components/external_install_options.cc
@@ -40,7 +40,9 @@
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 @@
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 @@
<< 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 "
diff --git a/chrome/browser/web_applications/components/external_install_options.h b/chrome/browser/web_applications/components/external_install_options.h
index 97172ac..b8ddf2ee 100644
--- a/chrome/browser/web_applications/components/external_install_options.h
+++ b/chrome/browser/web_applications/components/external_install_options.h
@@ -103,6 +103,17 @@
// 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;
diff --git a/chrome/browser/web_applications/components/pending_app_manager.cc b/chrome/browser/web_applications/components/pending_app_manager.cc
index 4434e50..d90b2a0 100644
--- a/chrome/browser/web_applications/components/pending_app_manager.cc
+++ b/chrome/browser/web_applications/components/pending_app_manager.cc
@@ -113,6 +113,11 @@
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_)
diff --git a/chrome/browser/web_applications/components/pending_app_manager.h b/chrome/browser/web_applications/components/pending_app_manager.h
index d8e724dc..accdc57 100644
--- a/chrome/browser/web_applications/components/pending_app_manager.h
+++ b/chrome/browser/web_applications/components/pending_app_manager.h
@@ -112,6 +112,7 @@
void SetRegistrationCallbackForTesting(RegistrationCallback callback);
void ClearRegistrationCallbackForTesting();
+ void SetRegistrationsCompleteCallbackForTesting(base::OnceClosure callback);
void ClearSynchronizeRequestsForTesting();
virtual void Shutdown() = 0;
@@ -128,6 +129,8 @@
virtual void OnRegistrationFinished(const GURL& launch_url,
RegistrationResultCode result);
+ base::OnceClosure registrations_complete_callback_;
+
private:
struct SynchronizeRequest {
SynchronizeRequest(SynchronizeCallback callback, int remaining_requests);
diff --git a/chrome/browser/web_applications/external_web_app_utils.cc b/chrome/browser/web_applications/external_web_app_utils.cc
index a77e513e..64d74e36 100644
--- a/chrome/browser/web_applications/external_web_app_utils.cc
+++ b/chrome/browser/web_applications/external_web_app_utils.cc
@@ -51,6 +51,22 @@
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 @@
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 @@
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;
diff --git a/chrome/browser/web_applications/pending_app_manager_impl.cc b/chrome/browser/web_applications/pending_app_manager_impl.cc
index 36b8ac99..e0322c6 100644
--- a/chrome/browser/web_applications/pending_app_manager_impl.cc
+++ b/chrome/browser/web_applications/pending_app_manager_impl.cc
@@ -243,8 +243,11 @@
}
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::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 @@
.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
diff --git a/chrome/browser/web_applications/pending_app_manager_impl.h b/chrome/browser/web_applications/pending_app_manager_impl.h
index 6b68851..1041f3e 100644
--- a/chrome/browser/web_applications/pending_app_manager_impl.h
+++ b/chrome/browser/web_applications/pending_app_manager_impl.h
@@ -87,6 +87,9 @@
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_;
diff --git a/chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc b/chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc
index 8c0929c1..6ea5758 100644
--- a/chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc
+++ b/chrome/browser/web_applications/pending_app_manager_impl_browsertest.cc
@@ -317,6 +317,48 @@
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());
{
diff --git a/chrome/browser/web_applications/test/web_app_registration_waiter.cc b/chrome/browser/web_applications/test/web_app_registration_waiter.cc
index 1911161..251b4ed 100644
--- a/chrome/browser/web_applications/test/web_app_registration_waiter.cc
+++ b/chrome/browser/web_applications/test/web_app_registration_waiter.cc
@@ -16,6 +16,8 @@
CHECK_EQ(code_, code);
run_loop_.Quit();
}));
+ manager_->SetRegistrationsCompleteCallbackForTesting(
+ complete_run_loop_.QuitClosure());
}
WebAppRegistrationWaiter::~WebAppRegistrationWaiter() {
@@ -30,4 +32,8 @@
run_loop_.Run();
}
+void WebAppRegistrationWaiter::AwaitRegistrationsComplete() {
+ complete_run_loop_.Run();
+}
+
} // namespace web_app
diff --git a/chrome/browser/web_applications/test/web_app_registration_waiter.h b/chrome/browser/web_applications/test/web_app_registration_waiter.h
index dab0dad8..f22d452 100644
--- a/chrome/browser/web_applications/test/web_app_registration_waiter.h
+++ b/chrome/browser/web_applications/test/web_app_registration_waiter.h
@@ -19,11 +19,15 @@
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
