Commit 19acb64c authored by falken's avatar falken Committed by Commit bot

Add CHECKs to investigate ServiceWorkerControlleeRequestHandler crash.

If the crash still occurs and the CHECKs pass, we know the null
active version is due to SWRegistration::DeleteVersion.

BUG=655910

Review-Url: https://codereview.chromium.org/2570453002
Cr-Commit-Position: refs/heads/master@{#437830}
parent c55a2886
...@@ -404,7 +404,13 @@ void ServiceWorkerControlleeRequestHandler::OnUpdatedVersionStatusChanged( ...@@ -404,7 +404,13 @@ void ServiceWorkerControlleeRequestHandler::OnUpdatedVersionStatusChanged(
void ServiceWorkerControlleeRequestHandler::PrepareForSubResource() { void ServiceWorkerControlleeRequestHandler::PrepareForSubResource() {
DCHECK(job_.get()); DCHECK(job_.get());
DCHECK(context_); DCHECK(context_);
DCHECK(provider_host_->active_version());
if (!provider_host_->active_version()) {
// TODO(falken): For debugging. Remove once https://crbug.com/655910 is
// resolved.
CHECK(provider_host_->controller_was_deleted());
}
MaybeForwardToServiceWorker(job_.get(), provider_host_->active_version()); MaybeForwardToServiceWorker(job_.get(), provider_host_->active_version());
} }
...@@ -441,7 +447,7 @@ bool ServiceWorkerControlleeRequestHandler::RequestStillValid( ...@@ -441,7 +447,7 @@ bool ServiceWorkerControlleeRequestHandler::RequestStillValid(
void ServiceWorkerControlleeRequestHandler::MainResourceLoadFailed() { void ServiceWorkerControlleeRequestHandler::MainResourceLoadFailed() {
DCHECK(provider_host_); DCHECK(provider_host_);
// Detach the controller so subresource requests also skip the worker. // Detach the controller so subresource requests also skip the worker.
provider_host_->NotifyControllerLost(); provider_host_->NotifyControllerLost(false /* was_deleted */);
} }
void ServiceWorkerControlleeRequestHandler::ClearJob() { void ServiceWorkerControlleeRequestHandler::ClearJob() {
......
...@@ -116,7 +116,8 @@ ServiceWorkerProviderHost::ServiceWorkerProviderHost( ...@@ -116,7 +116,8 @@ ServiceWorkerProviderHost::ServiceWorkerProviderHost(
parent_frame_security_level_(parent_frame_security_level), parent_frame_security_level_(parent_frame_security_level),
context_(context), context_(context),
dispatcher_host_(dispatcher_host), dispatcher_host_(dispatcher_host),
allow_association_(true) { allow_association_(true),
controller_was_deleted_(false) {
DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, provider_type_); DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, provider_type_);
// PlzNavigate // PlzNavigate
...@@ -208,6 +209,9 @@ void ServiceWorkerProviderHost::OnSkippedWaiting( ...@@ -208,6 +209,9 @@ void ServiceWorkerProviderHost::OnSkippedWaiting(
if (!controlling_version_) if (!controlling_version_)
return; return;
ServiceWorkerVersion* active_version = registration->active_version(); ServiceWorkerVersion* active_version = registration->active_version();
// TODO(falken): Change to DCHECK once https://crbug.com/655910 is
// resolved.
CHECK(active_version);
DCHECK_EQ(active_version->status(), ServiceWorkerVersion::ACTIVATING); DCHECK_EQ(active_version->status(), ServiceWorkerVersion::ACTIVATING);
SetControllerVersionAttribute(active_version, SetControllerVersionAttribute(active_version,
true /* notify_controllerchange */); true /* notify_controllerchange */);
...@@ -354,7 +358,9 @@ ServiceWorkerProviderHost::MatchRegistration() const { ...@@ -354,7 +358,9 @@ ServiceWorkerProviderHost::MatchRegistration() const {
return nullptr; return nullptr;
} }
void ServiceWorkerProviderHost::NotifyControllerLost() { void ServiceWorkerProviderHost::NotifyControllerLost(bool was_deleted) {
if (was_deleted)
controller_was_deleted_ = true;
SetControllerVersionAttribute(nullptr, true /* notify_controllerchange */); SetControllerVersionAttribute(nullptr, true /* notify_controllerchange */);
} }
...@@ -450,7 +456,8 @@ void ServiceWorkerProviderHost::AddScopedProcessReferenceToPattern( ...@@ -450,7 +456,8 @@ void ServiceWorkerProviderHost::AddScopedProcessReferenceToPattern(
void ServiceWorkerProviderHost::ClaimedByRegistration( void ServiceWorkerProviderHost::ClaimedByRegistration(
ServiceWorkerRegistration* registration) { ServiceWorkerRegistration* registration) {
DCHECK(registration->active_version()); // TODO(falken): Change to DCHECK once https://crbug.com/655910 is resolved.
CHECK(registration->active_version());
if (registration == associated_registration_) { if (registration == associated_registration_) {
SetControllerVersionAttribute(registration->active_version(), SetControllerVersionAttribute(registration->active_version(),
true /* notify_controllerchange */); true /* notify_controllerchange */);
......
...@@ -260,7 +260,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -260,7 +260,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
// Called when our controller has been terminated and doomed due to an // Called when our controller has been terminated and doomed due to an
// exceptional condition like it could no longer be read from the script // exceptional condition like it could no longer be read from the script
// cache. // cache.
void NotifyControllerLost(); void NotifyControllerLost(bool was_deleted);
bool controller_was_deleted() { return controller_was_deleted_; }
private: private:
friend class ForeignFetchRequestHandlerTest; friend class ForeignFetchRequestHandlerTest;
...@@ -374,6 +375,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -374,6 +375,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
base::WeakPtr<ServiceWorkerContextCore> context_; base::WeakPtr<ServiceWorkerContextCore> context_;
ServiceWorkerDispatcherHost* dispatcher_host_; ServiceWorkerDispatcherHost* dispatcher_host_;
bool allow_association_; bool allow_association_;
// TODO(falken): Remove once https://crbug.com/655910 is resolved.
bool controller_was_deleted_;
std::vector<base::Closure> queued_events_; std::vector<base::Closure> queued_events_;
......
...@@ -357,7 +357,7 @@ void ServiceWorkerRegistration::DeleteVersion( ...@@ -357,7 +357,7 @@ void ServiceWorkerRegistration::DeleteVersion(
!it->IsAtEnd(); it->Advance()) { !it->IsAtEnd(); it->Advance()) {
ServiceWorkerProviderHost* host = it->GetProviderHost(); ServiceWorkerProviderHost* host = it->GetProviderHost();
if (host->controlling_version() == version) if (host->controlling_version() == version)
host->NotifyControllerLost(); host->NotifyControllerLost(true /* was_deleted */);
} }
version->Doom(); version->Doom();
......
...@@ -291,7 +291,7 @@ class ServiceWorkerURLRequestJobTest ...@@ -291,7 +291,7 @@ class ServiceWorkerURLRequestJobTest
void MainResourceLoadFailed() override { void MainResourceLoadFailed() override {
CHECK(provider_host_); CHECK(provider_host_);
// Detach the controller so subresource requests also skip the worker. // Detach the controller so subresource requests also skip the worker.
provider_host_->NotifyControllerLost(); provider_host_->NotifyControllerLost(false /* was_deleted */);
} }
// Runs a request where the active worker starts a request in ACTIVATING state // Runs a request where the active worker starts a request in ACTIVATING state
......
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