Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2189)

Unified Diff: content/renderer/service_worker/service_worker_context_client.cc

Issue 2645493002: Increase the lifetime of Navigation Preload related objects. (Closed)
Patch Set: check fetchEvent in ServiceWorkerGlobalScopeProxy::onNavigationPreloadError Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/renderer/service_worker/service_worker_context_client.cc
diff --git a/content/renderer/service_worker/service_worker_context_client.cc b/content/renderer/service_worker/service_worker_context_client.cc
index 4569c4b15c118023841d135470d3d5cf9a0d5b78..1a358ce0e371450bf58e702b643d61f7f1c2e31a 100644
--- a/content/renderer/service_worker/service_worker_context_client.cc
+++ b/content/renderer/service_worker/service_worker_context_client.cc
@@ -267,19 +267,6 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
binding_(this, std::move(preload_handle->url_loader_client_request)) {}
~NavigationPreloadRequest() override {
- if (result_reported_)
- return;
- ServiceWorkerContextClient* client =
- ServiceWorkerContextClient::ThreadSpecificInstance();
- if (!client)
- return;
- client->OnNavigationPreloadError(
- fetch_event_id_,
- base::MakeUnique<blink::WebServiceWorkerError>(
- blink::WebServiceWorkerError::ErrorTypeAbort,
- blink::WebString::fromASCII(
- "Service Worker navigation preload aborted. Need to guard with "
- "respondWith or waitUntil.")));
}
void OnReceiveResponse(
@@ -297,8 +284,7 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
void OnReceiveRedirect(const net::RedirectInfo& redirect_info,
const ResourceResponseHead& response_head) override {
- // Cancel the request.
- url_loader_ = nullptr;
+ // This will delete itself.
falken 2017/01/18 14:24:58 nit: Slightly prefer |this| instead of "itself"
horo 2017/01/18 16:14:57 Done.
ReportErrorToClient(
"Service Worker navigation preload doesn't suport redirect.");
}
@@ -320,17 +306,21 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
}
void OnComplete(const ResourceRequestCompletionStatus& status) override {
- // We don't report to |client| if OnStartLoadingResponseBody() has already
- // called OnNavigationPreloadResponse().
- if (result_reported_)
+ if (status.error_code != net::OK) {
+ ReportErrorToClient("Service Worker navigation preload network error.");
falken 2017/01/18 14:24:58 Does this also delete |this|?
horo 2017/01/18 16:14:57 Done.
+ return;
+ }
+
+ ServiceWorkerContextClient* client =
+ ServiceWorkerContextClient::ThreadSpecificInstance();
+ if (!client)
return;
- DCHECK_NE(0, status.error_code);
- ReportErrorToClient("Service Worker navigation preload network error.");
+ // This will delete itself.
+ client->OnNavigationPreloadComplete(fetch_event_id_);
}
private:
void MaybeReportResponseToClient() {
- DCHECK(!result_reported_);
if (!response_ || !body_.is_valid())
return;
ServiceWorkerContextClient* client =
@@ -341,7 +331,6 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
client->OnNavigationPreloadResponse(
fetch_event_id_, std::move(response_),
base::MakeUnique<WebDataConsumerHandleImpl>(std::move(body_)));
- result_reported_ = true;
}
void ReportErrorToClient(const char* error_message) {
@@ -349,11 +338,11 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
ServiceWorkerContextClient::ThreadSpecificInstance();
if (!client)
return;
+ // This will delete itself.
client->OnNavigationPreloadError(
fetch_event_id_, base::MakeUnique<blink::WebServiceWorkerError>(
blink::WebServiceWorkerError::ErrorTypeNetwork,
blink::WebString::fromUTF8(error_message)));
- result_reported_ = true;
}
const int fetch_event_id_;
@@ -363,7 +352,6 @@ class ServiceWorkerContextClient::NavigationPreloadRequest final
std::unique_ptr<blink::WebURLResponse> response_;
mojo::ScopedDataPipeConsumerHandle body_;
- bool result_reported_ = false;
};
ServiceWorkerContextClient*
@@ -720,12 +708,6 @@ void ServiceWorkerContextClient::didHandleFetchEvent(
int fetch_event_id,
blink::WebServiceWorkerEventResult result,
double event_dispatch_time) {
- if (context_->preload_requests.Lookup(fetch_event_id)) {
- // Deletes NavigationPreloadRequest. If the network request is ongoing, it
- // will be canceled by deleting the mojom::URLLoaderPtr in the
- // NavigationPreloadRequest.
- context_->preload_requests.Remove(fetch_event_id);
- }
const FetchCallback* callback =
context_->fetch_event_callbacks.Lookup(fetch_event_id);
DCHECK(callback);
@@ -1269,6 +1251,12 @@ void ServiceWorkerContextClient::OnNavigationPreloadError(
int fetch_event_id,
std::unique_ptr<blink::WebServiceWorkerError> error) {
proxy_->onNavigationPreloadError(fetch_event_id, std::move(error));
+ context_->preload_requests.Remove(fetch_event_id);
+}
+
+void ServiceWorkerContextClient::OnNavigationPreloadComplete(
+ int fetch_event_id) {
+ context_->preload_requests.Remove(fetch_event_id);
falken 2017/01/18 14:24:58 So either OnError or OnComplete must be called, bu
horo 2017/01/18 16:14:57 Done.
}
void ServiceWorkerContextClient::BindEventDispatcher(

Powered by Google App Engine
This is Rietveld 408576698