Increase the lifetime of Navigation Preload related objects.

content/renderer/service_worker/service_worker_context_client.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/service_worker/service_worker_context_client.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/lazy_instance.h"
@@ skipping 249 matching lines @@
  public:
   NavigationPreloadRequest(int fetch_event_id,
                            const GURL& url,
                            mojom::FetchEventPreloadHandlePtr preload_handle)
       : fetch_event_id_(fetch_event_id),
         url_(url),
         url_loader_(std::move(preload_handle->url_loader)),
         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(
       const ResourceResponseHead& response_head,
       mojom::DownloadedTempFilePtr downloaded_file) override {
     DCHECK(!response_);
     DCHECK(!downloaded_file);
     response_ = base::MakeUnique<blink::WebURLResponse>();
     // TODO(horo): Set report_security_info to true when DevTools is attached.
     const bool report_security_info = false;
     WebURLLoaderImpl::PopulateURLResponse(url_, response_head, response_.get(),
                                           report_security_info);
     MaybeReportResponseToClient();
   }
 
   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.");
   }
 
   void OnDataDownloaded(int64_t data_length,
                         int64_t encoded_data_length) override {
     NOTREACHED();
   }
 
   void OnTransferSizeUpdated(int32_t transfer_size_diff) override {
     NOTREACHED();
   }
 
   void OnStartLoadingResponseBody(
       mojo::ScopedDataPipeConsumerHandle body) override {
     DCHECK(!body_.is_valid());
     body_ = std::move(body);
     MaybeReportResponseToClient();
   }
 
   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;
-    DCHECK_NE(0, status.error_code);
-    ReportErrorToClient("Service Worker navigation preload network error.");
+    }
+
+    ServiceWorkerContextClient* client =
+        ServiceWorkerContextClient::ThreadSpecificInstance();
+    if (!client)
+      return;
+    // This will delete itself.
+    client->OnNavigationPreloadComplete(fetch_event_id_);
   }
 
  private:
   void MaybeReportResponseToClient() {
-    DCHECK(!result_reported_);
     if (!response_ || !body_.is_valid())
       return;
     ServiceWorkerContextClient* client =
         ServiceWorkerContextClient::ThreadSpecificInstance();
     if (!client)
       return;
 
     client->OnNavigationPreloadResponse(
         fetch_event_id_, std::move(response_),
         base::MakeUnique<WebDataConsumerHandleImpl>(std::move(body_)));
-    result_reported_ = true;
   }
 
   void ReportErrorToClient(const char* error_message) {
     ServiceWorkerContextClient* client =
         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_;
   const GURL url_;
   mojom::URLLoaderPtr url_loader_;
   mojo::Binding<mojom::URLLoaderClient> binding_;
 
   std::unique_ptr<blink::WebURLResponse> response_;
   mojo::ScopedDataPipeConsumerHandle body_;
-  bool result_reported_ = false;
 };
 
 ServiceWorkerContextClient*
 ServiceWorkerContextClient::ThreadSpecificInstance() {
   return g_worker_client_tls.Pointer()->Get();
 }
 
 ServiceWorkerContextClient::ServiceWorkerContextClient(
     int embedded_worker_id,
     int64_t service_worker_version_id,
@@ skipping 336 matching lines @@
       GetRoutingID(), fetch_event_id,
       SERVICE_WORKER_FETCH_EVENT_RESULT_RESPONSE,
       GetServiceWorkerResponseFromWebResponse(web_response),
       base::Time::FromDoubleT(event_dispatch_time)));
 }
 
 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);
   callback->Run(EventResultToStatus(result),
                 base::Time::FromDoubleT(event_dispatch_time));
   context_->fetch_event_callbacks.Remove(fetch_event_id);
 }
 
 void ServiceWorkerContextClient::didHandleNotificationClickEvent(
     int request_id,
@@ skipping 523 matching lines @@
     std::unique_ptr<blink::WebURLResponse> response,
     std::unique_ptr<blink::WebDataConsumerHandle> data_consumer_handle) {
   proxy_->onNavigationPreloadResponse(fetch_event_id, std::move(response),
                                       std::move(data_consumer_handle));
 }
 
 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, but not both, right? Could be
worth in the header file.

[horo, 2017/01/18 16:14:57]

Done.

 }
 
 void ServiceWorkerContextClient::BindEventDispatcher(
     mojom::ServiceWorkerEventDispatcherRequest request) {
   DCHECK(context_);
   DCHECK(!context_->event_dispatcher_binding.is_bound());
   context_->event_dispatcher_binding.Bind(std::move(request));
 }
 
 base::WeakPtr<ServiceWorkerContextClient>
 ServiceWorkerContextClient::GetWeakPtr() {
   DCHECK(worker_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(context_);
   return context_->weak_factory.GetWeakPtr();
 }
 
 }  // namespace content