Bug 1195167 part 6: Some code simplification since necko handles fetch recursion. r=bkelly

dom/fetch/FetchDriver.cpp

@@ -50,9 +50,9 @@ FetchDriver::FetchDriver(InternalRequest* aRequest, nsIPrincipal* aPrincipal,
   : mPrincipal(aPrincipal)
   , mLoadGroup(aLoadGroup)
   , mRequest(aRequest)
-  , mFetchRecursionCount(0)
   , mHasBeenCrossSite(false)
   , mResponseAvailableCalled(false)
+  , mFetchCalled(false)
 {
 }
 
@@ -67,34 +67,22 @@ nsresult
 FetchDriver::Fetch(FetchDriverObserver* aObserver)
 {
   workers::AssertIsOnMainThread();
+  MOZ_ASSERT(!mFetchCalled);
+  mFetchCalled = true;
+
   mObserver = aObserver;
 
   Telemetry::Accumulate(Telemetry::SERVICE_WORKER_REQUEST_PASSTHROUGH,
                         mRequest->WasCreatedByFetchEvent());
 
-  return Fetch();
-}
-
-nsresult
-FetchDriver::Fetch()
-{
-  // We do not currently implement parts of the spec that lead to recursion.
-  MOZ_ASSERT(mFetchRecursionCount == 0);
-  mFetchRecursionCount++;
-
   // FIXME(nsm): Deal with HSTS.
 
-  if (!mRequest->IsSynchronous() && mFetchRecursionCount <= 1) {
+  MOZ_RELEASE_ASSERT(!mRequest->IsSynchronous(),
-    nsCOMPtr<nsIRunnable> r =
+                     "Synchronous fetch not supported");
-      NS_NewRunnableMethod(this, &FetchDriver::ContinueFetch);
-    nsresult rv = NS_DispatchToCurrentThread(r);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      FailWithNetworkError();
-    }
-    return rv;
-  }
 
-  MOZ_CRASH("Synchronous fetch not supported");
+  nsCOMPtr<nsIRunnable> r =
+    NS_NewRunnableMethod(this, &FetchDriver::ContinueFetch);
+  return NS_DispatchToCurrentThread(r);
 }
 
 nsresult
@@ -444,16 +432,6 @@ FetchDriver::IsUnsafeRequest()
             !mRequest->Headers()->HasOnlySimpleHeaders())));
 }
 
-nsresult
-FetchDriver::ContinueHttpFetchAfterNetworkFetch()
-{
-  workers::AssertIsOnMainThread();
-  MOZ_ASSERT(mResponse);
-  MOZ_ASSERT(!mResponse->IsError());
-
-  return SucceedWithResponse();
-}
-
 already_AddRefed<InternalResponse>
 FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aFinalURI)
 {
@@ -494,17 +472,6 @@ FetchDriver::BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aF
   return filteredResponse.forget();
 }
 
-nsresult
-FetchDriver::SucceedWithResponse()
-{
-  workers::AssertIsOnMainThread();
-  if (mObserver) {
-    mObserver->OnResponseEnd();
-    mObserver = nullptr;
-  }
-  return NS_OK;
-}
-
 nsresult
 FetchDriver::FailWithNetworkError()
 {
@@ -714,17 +681,23 @@ FetchDriver::OnStopRequest(nsIRequest* aRequest,
     if (outputStream) {
       outputStream->CloseWithStatus(NS_BINDING_FAILED);
     }
+
     // We proceed as usual here, since we've already created a successful response
     // from OnStartRequest.
-    SucceedWithResponse();
+  } else {
-    return aStatusCode;
+    MOZ_ASSERT(mResponse);
+    MOZ_ASSERT(!mResponse->IsError());
+
+    if (mPipeOutputStream) {
+      mPipeOutputStream->Close();
+    }
   }
 
-  if (mPipeOutputStream) {
+  if (mObserver) {
-    mPipeOutputStream->Close();
+    mObserver->OnResponseEnd();
+    mObserver = nullptr;
   }
 
-  ContinueHttpFetchAfterNetworkFetch();
   return NS_OK;
 }
 
@@ -914,7 +887,7 @@ void
 FetchDriver::SetDocument(nsIDocument* aDocument)
 {
   // Cannot set document after Fetch() has been called.
-  MOZ_ASSERT(mFetchRecursionCount == 0);
+  MOZ_ASSERT(!mFetchCalled);
   mDocument = aDocument;
 }
 

dom/fetch/FetchDriver.h

@@ -82,22 +82,20 @@ private:
   nsCOMPtr<nsIOutputStream> mPipeOutputStream;
   RefPtr<FetchDriverObserver> mObserver;
   nsCOMPtr<nsIDocument> mDocument;
-  uint32_t mFetchRecursionCount;
   bool mHasBeenCrossSite;
 
   DebugOnly<bool> mResponseAvailableCalled;
+  DebugOnly<bool> mFetchCalled;
 
   FetchDriver() = delete;
   FetchDriver(const FetchDriver&) = delete;
   FetchDriver& operator=(const FetchDriver&) = delete;
   ~FetchDriver();
 
-  nsresult Fetch();
   nsresult SetTainting();
   nsresult ContinueFetch();
   nsresult HttpFetch();
   bool IsUnsafeRequest();
-  nsresult ContinueHttpFetchAfterNetworkFetch();
   // Returns the filtered response sent to the observer.
   // Callers who don't have access to a channel can pass null for aFinalURI.
   already_AddRefed<InternalResponse>
@@ -105,7 +103,6 @@ private:
   // Utility since not all cases need to do any post processing of the filtered
   // response.
   nsresult FailWithNetworkError();
-  nsresult SucceedWithResponse();
 };
 
 } // namespace dom