Commit 72cb7dab authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

service worker: Make sure a redundant version doesn't have controllees

This CL temporary adds some CHECK to make sure that a
ServiceWorkerVersion doesn't have any controllees when it becomes
redundant. These CHECKs will be removed once we identified the cause
of some crashes.

This CL also add DEBUG_ALIAS_FOR_CSTR in
ServiceWorkerVersion::AddControllee() to put how a ServiceWorkerVersion
became redundant on the stack so that we can retrieve the information
from reports.

Bug: 1021718
Change-Id: I5c62e475d85aa327852b72d577faf8c5ccafb53d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2114478Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753436}
parent e5f0b1f7
...@@ -659,6 +659,9 @@ void ServiceWorkerRegisterJob::OnStoreRegistrationComplete( ...@@ -659,6 +659,9 @@ void ServiceWorkerRegisterJob::OnStoreRegistrationComplete(
// TODO(falken): Move this further down. The spec says to set status to // TODO(falken): Move this further down. The spec says to set status to
// 'redundant' after promoting the new version to .waiting attribute and // 'redundant' after promoting the new version to .waiting attribute and
// 'installed' status. // 'installed' status.
// TODO(crbug.com/951571): Remove this once we identified the cause of
// crash.
CHECK(!registration()->waiting_version()->HasControllee());
registration()->waiting_version()->SetStatus( registration()->waiting_version()->SetStatus(
ServiceWorkerVersion::REDUNDANT); ServiceWorkerVersion::REDUNDANT);
} }
......
...@@ -505,6 +505,11 @@ void ServiceWorkerRegistration::ActivateWaitingVersion(bool delay) { ...@@ -505,6 +505,11 @@ void ServiceWorkerRegistration::ActivateWaitingVersion(bool delay) {
observer.OnSkippedWaiting(this); observer.OnSkippedWaiting(this);
} }
// TODO(crbug.com/951571): Remove this once we identified the cause of crash.
if (exiting_version) {
CHECK(!exiting_version->HasControllee());
}
// "10. Queue a task to fire an event named activate..." // "10. Queue a task to fire an event named activate..."
// The browser could be shutting down. To avoid spurious start worker // The browser could be shutting down. To avoid spurious start worker
// failures, wait a bit before continuing. // failures, wait a bit before continuing.
......
...@@ -376,6 +376,9 @@ void ServiceWorkerVersion::SetStatus(Status status) { ...@@ -376,6 +376,9 @@ void ServiceWorkerVersion::SetStatus(Status status) {
} else if (status == REDUNDANT) { } else if (status == REDUNDANT) {
embedded_worker_->OnWorkerVersionDoomed(); embedded_worker_->OnWorkerVersionDoomed();
// TODO(crbug.com/1021718): Remove this check after we identified the cause.
CHECK(bfcached_controllee_map_.empty());
// TODO(crbug.com/951571): Remove this once we figured out the cause of // TODO(crbug.com/951571): Remove this once we figured out the cause of
// invalid controller status. // invalid controller status.
redundant_state_callstack_ = base::debug::StackTrace(); redundant_state_callstack_ = base::debug::StackTrace();
...@@ -763,6 +766,10 @@ void ServiceWorkerVersion::AddControllee( ...@@ -763,6 +766,10 @@ void ServiceWorkerVersion::AddControllee(
// TODO(yuzus, crbug.com/951571): Remove these CHECKs once we figure out the // TODO(yuzus, crbug.com/951571): Remove these CHECKs once we figure out the
// cause of crash. // cause of crash.
if (status_ == REDUNDANT) {
DEBUG_ALIAS_FOR_CSTR(redundant_callstack_str,
redundant_state_callstack_.ToString().c_str(), 1024);
}
CHECK_NE(status_, NEW); CHECK_NE(status_, NEW);
CHECK_NE(status_, INSTALLING); CHECK_NE(status_, INSTALLING);
CHECK_NE(status_, INSTALLED); CHECK_NE(status_, INSTALLED);
...@@ -951,7 +958,9 @@ void ServiceWorkerVersion::Doom() { ...@@ -951,7 +958,9 @@ void ServiceWorkerVersion::Doom() {
container_host->NotifyControllerLost(); container_host->NotifyControllerLost();
} }
// Any controllee this version had should have removed itself. // Any controllee this version had should have removed itself.
DCHECK(!HasControllee()); // TODO(crbug.com/951571): Change to DCHECK once we identified the cause of
// crash.
CHECK(!HasControllee());
SetStatus(REDUNDANT); SetStatus(REDUNDANT);
if (running_status() == EmbeddedWorkerStatus::STARTING || if (running_status() == EmbeddedWorkerStatus::STARTING ||
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment