Skip to content

Commit

Permalink
Avoid ServiceWorkerStorageControl mojo reconnection during DeleteAndS…
Browse files Browse the repository at this point in the history
…tartOver()

ServiceWorkerContextWrapper::DidDeleteAndStartOver() destroys both
ServiceWorkerRegistration (via ServiceWorkerContextCore) and
ServiceWorkerStorage (via ServiceWorkerStorageControlImpl).

The destructor of ServiceWorkerStorage posts a task to clear session
only registrations. The destructor doesn't wait for the task completion.

Destroying ServiceWorkerStorageControlImpl also triggers a mojo
connection error on ServiceWorkerRegistry::remote_storage_control_.

There seems a case where another service worker storage operation
(e.g. find a registration) is interleaved while executing the above
process. In such operations ServiceWorkerRegistry attempts to reconnect
the mojo connection but it seems to cause a crash as the remote
(ServiceWorkerStorageControlImpl) was already destroyed.

This CL checks whether ServiceWorkerRegistration is being destroyed
before attempting to reconnect the mojo connection to avoid such
situation.

(cherry picked from commit 7d41101)

Bug: 1282869
Change-Id: If30cb2d0d3f90a43ca61269e7f70c792e7224692
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3495885
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#976612}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3508114
Auto-Submit: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4896@{#361}
Cr-Branched-From: 1f63ff4-refs/heads/main@{#972766}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Mar 8, 2022
1 parent b8a64c8 commit f050034
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions content/browser/service_worker/service_worker_registry.cc
Expand Up @@ -1557,6 +1557,15 @@ void ServiceWorkerRegistry::OnRemoteStorageDisconnected() {
if (!context_)
return;

if (is_storage_disabled_) {
// When the storage is disabled a storage error recovery process is ongoing
// and the storage control will be destroyed soon. Don't try to reconnect
// storage control remote but flush inflight calls. These calls will check
// `is_storage_disabled_` and return errors.
DidRecover();
return;
}

if (connection_state_ == ConnectionState::kRecovering) {
++recovery_retry_counts_;
if (recovery_retry_counts_ > kMaxRetryCounts) {
Expand Down

0 comments on commit f050034

Please sign in to comment.