Commit 2ad1f83a authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

S13nSW: Don't delete ServiceWorkerSubresourceLoader twice

Before this CL, DeleteSoon() could be called twice when a
connection error happens on |url_loader_binding_|.
- ServiceWorkerSubresourceLoader sets DeleteSoon() as the connection
  error handler.
- The class also calls DeleteSoon() when the loader is no longer needed.
  For example, it delete itself when a request falls back to network.

After this CL we just call delete directly as needed.
Existing DeleteSoon() is renamed to OnConnectionError() as it is only
used as the connection error handler.

Bug: 828257
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I85737d0d6c33b747a4f70f5156b2b3b4b512f873
Reviewed-on: https://chromium-review.googlesource.com/997256
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548708}
parent 32d7610b
...@@ -189,8 +189,9 @@ ServiceWorkerSubresourceLoader::ServiceWorkerSubresourceLoader( ...@@ -189,8 +189,9 @@ ServiceWorkerSubresourceLoader::ServiceWorkerSubresourceLoader(
response_head_.load_timing.request_start = base::TimeTicks::Now(); response_head_.load_timing.request_start = base::TimeTicks::Now();
response_head_.load_timing.request_start_time = base::Time::Now(); response_head_.load_timing.request_start_time = base::Time::Now();
// base::Unretained() is safe since |url_loader_binding_| is owned by |this|. // base::Unretained() is safe since |url_loader_binding_| is owned by |this|.
url_loader_binding_.set_connection_error_handler(base::BindOnce( url_loader_binding_.set_connection_error_handler(
&ServiceWorkerSubresourceLoader::DeleteSoon, base::Unretained(this))); base::BindOnce(&ServiceWorkerSubresourceLoader::OnConnectionError,
base::Unretained(this)));
StartRequest(resource_request); StartRequest(resource_request);
} }
...@@ -198,8 +199,8 @@ ServiceWorkerSubresourceLoader::~ServiceWorkerSubresourceLoader() { ...@@ -198,8 +199,8 @@ ServiceWorkerSubresourceLoader::~ServiceWorkerSubresourceLoader() {
SettleInflightFetchRequestIfNeeded(); SettleInflightFetchRequestIfNeeded();
}; };
void ServiceWorkerSubresourceLoader::DeleteSoon() { void ServiceWorkerSubresourceLoader::OnConnectionError() {
base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this); delete this;
} }
void ServiceWorkerSubresourceLoader::StartRequest( void ServiceWorkerSubresourceLoader::StartRequest(
...@@ -245,7 +246,7 @@ void ServiceWorkerSubresourceLoader::DispatchFetchEvent() { ...@@ -245,7 +246,7 @@ void ServiceWorkerSubresourceLoader::DispatchFetchEvent() {
url_loader_binding_.Unbind(), routing_id_, request_id_, options_, url_loader_binding_.Unbind(), routing_id_, request_id_, options_,
resource_request_, std::move(url_loader_client_), resource_request_, std::move(url_loader_client_),
traffic_annotation_); traffic_annotation_);
DeleteSoon(); delete this;
return; return;
} }
DCHECK_EQ(ControllerServiceWorkerConnector::State::kNoContainerHost, DCHECK_EQ(ControllerServiceWorkerConnector::State::kNoContainerHost,
...@@ -402,7 +403,7 @@ void ServiceWorkerSubresourceLoader::OnFallback( ...@@ -402,7 +403,7 @@ void ServiceWorkerSubresourceLoader::OnFallback(
traffic_annotation_); traffic_annotation_);
// Per spec, redirects after this point are not intercepted by the service // Per spec, redirects after this point are not intercepted by the service
// worker again (https://crbug.com/517364). So this loader is done. // worker again (https://crbug.com/517364). So this loader is done.
DeleteSoon(); delete this;
} }
void ServiceWorkerSubresourceLoader::StartResponse( void ServiceWorkerSubresourceLoader::StartResponse(
......
...@@ -61,7 +61,7 @@ class CONTENT_EXPORT ServiceWorkerSubresourceLoader ...@@ -61,7 +61,7 @@ class CONTENT_EXPORT ServiceWorkerSubresourceLoader
private: private:
class StreamWaiter; class StreamWaiter;
void DeleteSoon(); void OnConnectionError();
void StartRequest(const network::ResourceRequest& resource_request); void StartRequest(const network::ResourceRequest& resource_request);
void DispatchFetchEvent(); void DispatchFetchEvent();
......
...@@ -19,7 +19,6 @@ Bug(none) external/wpt/service-workers/service-worker/fetch-request-xhr-sync-on- ...@@ -19,7 +19,6 @@ Bug(none) external/wpt/service-workers/service-worker/fetch-request-xhr-sync-on-
crbug.com/771118 external/wpt/service-workers/service-worker/mime-sniffing.https.html [ Failure ] crbug.com/771118 external/wpt/service-workers/service-worker/mime-sniffing.https.html [ Failure ]
Bug(none) external/wpt/service-workers/service-worker/registration-updateviacache.https.html [ Failure Timeout ] Bug(none) external/wpt/service-workers/service-worker/registration-updateviacache.https.html [ Failure Timeout ]
Bug(none) external/wpt/service-workers/service-worker/shared-worker-controlled.https.html [ Timeout ] Bug(none) external/wpt/service-workers/service-worker/shared-worker-controlled.https.html [ Timeout ]
Bug(none) external/wpt/service-workers/service-worker/worker-interception.https.html [ Pass Crash Failure ]
Bug(none) external/wpt/service-workers/service-worker/navigation-preload/broken-chunked-encoding.https.html [ Failure ] Bug(none) external/wpt/service-workers/service-worker/navigation-preload/broken-chunked-encoding.https.html [ Failure ]
Bug(none) external/wpt/service-workers/service-worker/ready.https.html [ Pass Crash Failure ] Bug(none) external/wpt/service-workers/service-worker/ready.https.html [ Pass Crash Failure ]
Bug(none) http/tests/appcache/top-frame-3.html [ Pass Timeout ] Bug(none) http/tests/appcache/top-frame-3.html [ Pass Timeout ]
......
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