Race OnStartLoadingResponseBody and OnReceiveResponse of NavigationPreloadRequest.

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 224 matching lines @@
   base::WeakPtrFactory<blink::WebServiceWorkerContextProxy> proxy_weak_factory;
 };
 
 class ServiceWorkerContextClient::NavigationPreloadRequest final
     : public mojom::URLLoaderClient {
  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)),
-        response_(base::MakeUnique<blink::WebServiceWorkerResponse>()) {
-    response_->setURL(url);
-  }
+        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::fromUTF8(
                 "Service Worker navigation preload aborted. Need to guard with "
                 "respondWith or waitUntil.")));
   }
 
   void OnReceiveResponse(const ResourceResponseHead& response_head) override {
-    DCHECK(response_);
+    DCHECK(!response_);
+    response_ = base::MakeUnique<blink::WebServiceWorkerResponse>();
+    response_->setURL(url_);
     DCHECK(response_head.headers);
     response_->setStatus(response_head.headers->response_code());
     response_->setStatusText(
         blink::WebString::fromUTF8(response_head.headers->GetStatusText()));
     response_->setResponseType(blink::WebServiceWorkerResponseTypeBasic);
     size_t iter = 0;
     std::string header_name;
     std::string header_value;
     while (response_head.headers->EnumerateHeaderLines(&iter, &header_name,
                                                        &header_value)) {
       response_->appendHeader(blink::WebString::fromUTF8(header_name),
                               blink::WebString::fromUTF8(header_value));
     }
     response_->setResponseTime(response_head.response_time.ToInternalValue());
+    MaybeReportToClient();
   }
 
   void OnStartLoadingResponseBody(
       mojo::ScopedDataPipeConsumerHandle body) override {
-    DCHECK(!result_reported_);
-    ServiceWorkerContextClient* client =
-        ServiceWorkerContextClient::ThreadSpecificInstance();
-    if (!client)
-      return;
-    client->OnNavigationPreloadResponse(
-        fetch_event_id_, std::move(response_),
-        base::MakeUnique<WebDataConsumerHandleImpl>(std::move(body)));
-    result_reported_ = true;
+    DCHECK(!body_.is_valid());
+    body_ = std::move(body);
+    MaybeReportToClient();
   }
 
   void OnComplete(const ResourceRequestCompletionStatus& status) override {
     // We don't report to |client| if OnStartLoadingResponseBody() has already
     // called OnNavigationPreloadResponse().
     if (result_reported_)
       return;
     DCHECK_NE(0, status.error_code);
     ServiceWorkerContextClient* client =
         ServiceWorkerContextClient::ThreadSpecificInstance();
     if (!client)
       return;
     client->OnNavigationPreloadError(
         fetch_event_id_,
         base::MakeUnique<blink::WebServiceWorkerError>(
             blink::WebServiceWorkerError::ErrorTypeNetwork,
             blink::WebString::fromUTF8(
                 "Service Worker navigation preload network error.")));
     result_reported_ = true;
   }
 
  private:
+  void MaybeReportToClient() {
+    DCHECK(!result_reported_);

[falken, 2016/11/04 09:36:50]

I'm missing something... how does this DCHECK pass since both
OnReceiveRersponse() and OnStartLoadingResposneBody() call
MaybeReportToClient()? Does it need to be moved under the if below?

[horo, 2016/11/04 09:41:48]

Both OnReceiveRersponse() and OnStartLoadingResposneBody() are called only once.
OnReceiveRersponse() sets |response_|.
OnStartLoadingResposneBody() sets |body_|.
So this check "if (!response_ || !body_.is_valid())" pass only once.

[falken, 2016/11/04 09:46:55]

Ah yes... I was totally missing something.

+    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;
+  }
+
   const int fetch_event_id_;
+  const GURL url_;
   mojom::URLLoaderPtr url_loader_;
   mojo::Binding<mojom::URLLoaderClient> binding_;
+
   std::unique_ptr<blink::WebServiceWorkerResponse> response_;
+  mojo::ScopedDataPipeConsumerHandle body_;
   bool result_reported_ = false;
 };
 
 class ServiceWorkerContextClient::FetchEventDispatcherImpl
     : public NON_EXPORTED_BASE(mojom::FetchEventDispatcher) {
  public:
   static void Create(mojom::FetchEventDispatcherRequest request) {
     mojo::MakeStrongBinding(base::MakeUnique<FetchEventDispatcherImpl>(),
                             std::move(request));
   }
@@ skipping 946 matching lines @@
 }
 
 base::WeakPtr<ServiceWorkerContextClient>
 ServiceWorkerContextClient::GetWeakPtr() {
   DCHECK(worker_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(context_);
   return context_->weak_factory.GetWeakPtr();
 }
 
 }  // namespace content