Bug 1195167 part 6: Some code simplification since necko handles fetch recursion. r=bkelly
authorJonas Sicking <jonas@sicking.cc>
Mon, 19 Oct 2015 18:24:36 -0700
changeset 303616 d90d10c6e134eb8f924860141c9fd86269d30447
parent 303615 82ab1c1e478dabd3ed9f18f00c0956ef8976c8d6
child 303617 6af7ffd34eccf1ccaa1409f1f7884f04b6a4ad19
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1195167
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1195167 part 6: Some code simplification since necko handles fetch recursion. r=bkelly
dom/fetch/FetchDriver.cpp
dom/fetch/FetchDriver.h
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -45,61 +45,49 @@ NS_IMPL_ISUPPORTS(FetchDriver,
                   nsIStreamListener, nsIChannelEventSink, nsIInterfaceRequestor,
                   nsIThreadRetargetableStreamListener)
 
 FetchDriver::FetchDriver(InternalRequest* aRequest, nsIPrincipal* aPrincipal,
                          nsILoadGroup* aLoadGroup)
   : mPrincipal(aPrincipal)
   , mLoadGroup(aLoadGroup)
   , mRequest(aRequest)
-  , mFetchRecursionCount(0)
   , mHasBeenCrossSite(false)
   , mResponseAvailableCalled(false)
+  , mFetchCalled(false)
 {
 }
 
 FetchDriver::~FetchDriver()
 {
   // We assert this since even on failures, we should call
   // FailWithNetworkError().
   MOZ_ASSERT(mResponseAvailableCalled);
 }
 
 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) {
-    nsCOMPtr<nsIRunnable> r =
-      NS_NewRunnableMethod(this, &FetchDriver::ContinueFetch);
-    nsresult rv = NS_DispatchToCurrentThread(r);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      FailWithNetworkError();
-    }
-    return rv;
-  }
+  MOZ_RELEASE_ASSERT(!mRequest->IsSynchronous(),
+                     "Synchronous fetch not supported");
 
-  MOZ_CRASH("Synchronous fetch not supported");
+  nsCOMPtr<nsIRunnable> r =
+    NS_NewRunnableMethod(this, &FetchDriver::ContinueFetch);
+  return NS_DispatchToCurrentThread(r);
 }
 
 nsresult
 FetchDriver::SetTainting()
 {
   workers::AssertIsOnMainThread();
 
   // If we've already been cross-site then we should be fully updated
@@ -439,26 +427,16 @@ FetchDriver::IsUnsafeRequest()
 {
   return mHasBeenCrossSite &&
          (mRequest->Mode() == RequestMode::Cors_with_forced_preflight ||
           (mRequest->UnsafeRequest() &&
            (!mRequest->HasSimpleMethod() ||
             !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)
 {
   MOZ_ASSERT(aResponse);
   nsAutoCString reqURL;
   if (aFinalURI) {
     aFinalURI->GetSpec(reqURL);
   } else {
@@ -490,27 +468,16 @@ FetchDriver::BeginAndGetFilteredResponse
   MOZ_ASSERT(filteredResponse);
   MOZ_ASSERT(mObserver);
   mObserver->OnResponseAvailable(filteredResponse);
   mResponseAvailableCalled = true;
   return filteredResponse.forget();
 }
 
 nsresult
-FetchDriver::SucceedWithResponse()
-{
-  workers::AssertIsOnMainThread();
-  if (mObserver) {
-    mObserver->OnResponseEnd();
-    mObserver = nullptr;
-  }
-  return NS_OK;
-}
-
-nsresult
 FetchDriver::FailWithNetworkError()
 {
   workers::AssertIsOnMainThread();
   RefPtr<InternalResponse> error = InternalResponse::NetworkError();
   if (mObserver) {
     mObserver->OnResponseAvailable(error);
     mResponseAvailableCalled = true;
     mObserver->OnResponseEnd();
@@ -709,27 +676,33 @@ FetchDriver::OnStopRequest(nsIRequest* a
                            nsresult aStatusCode)
 {
   workers::AssertIsOnMainThread();
   if (NS_FAILED(aStatusCode)) {
     nsCOMPtr<nsIAsyncOutputStream> outputStream = do_QueryInterface(mPipeOutputStream);
     if (outputStream) {
       outputStream->CloseWithStatus(NS_BINDING_FAILED);
     }
+
     // We proceed as usual here, since we've already created a successful response
     // from OnStartRequest.
-    SucceedWithResponse();
-    return aStatusCode;
+  } else {
+    MOZ_ASSERT(mResponse);
+    MOZ_ASSERT(!mResponse->IsError());
+
+    if (mPipeOutputStream) {
+      mPipeOutputStream->Close();
+    }
   }
 
-  if (mPipeOutputStream) {
-    mPipeOutputStream->Close();
+  if (mObserver) {
+    mObserver->OnResponseEnd();
+    mObserver = nullptr;
   }
 
-  ContinueHttpFetchAfterNetworkFetch();
   return NS_OK;
 }
 
 // This is called when the channel is redirected.
 NS_IMETHODIMP
 FetchDriver::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
                                     nsIChannel* aNewChannel,
                                     uint32_t aFlags,
@@ -909,14 +882,14 @@ FetchDriver::GetInterface(const nsIID& a
 
   return QueryInterface(aIID, aResult);
 }
 
 void
 FetchDriver::SetDocument(nsIDocument* aDocument)
 {
   // Cannot set document after Fetch() has been called.
-  MOZ_ASSERT(mFetchRecursionCount == 0);
+  MOZ_ASSERT(!mFetchCalled);
   mDocument = aDocument;
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/fetch/FetchDriver.h
+++ b/dom/fetch/FetchDriver.h
@@ -77,38 +77,35 @@ public:
 private:
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsILoadGroup> mLoadGroup;
   RefPtr<InternalRequest> mRequest;
   RefPtr<InternalResponse> mResponse;
   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>
   BeginAndGetFilteredResponse(InternalResponse* aResponse, nsIURI* aFinalURI);
   // Utility since not all cases need to do any post processing of the filtered
   // response.
   nsresult FailWithNetworkError();
-  nsresult SucceedWithResponse();
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_FetchDriver_h