Bug 1198544 - Prevent FetchDriver from creating multiple responses if OnStopRequest yields a failing status code. r=nsm
authorJosh Matthews <josh@joshmatthews.net>
Mon, 14 Sep 2015 11:05:35 -0400
changeset 294931 5af5ff26d3858268b19ddfbd15306b6ed07a3816
parent 294930 edabe4800045ffe74d5d794f427945cb9345620d
child 294932 15753ee8232fa1585dad78af9569a3d237cda675
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnsm
bugs1198544
milestone43.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 1198544 - Prevent FetchDriver from creating multiple responses if OnStopRequest yields a failing status code. r=nsm
dom/fetch/Fetch.cpp
dom/fetch/FetchDriver.cpp
dom/fetch/FetchDriver.h
dom/workers/test/serviceworkers/fetch/fetch_tests.js
dom/workers/test/serviceworkers/fetch/interrupt.sjs
dom/workers/test/serviceworkers/mochitest.ini
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -67,17 +67,17 @@ public:
       return nullptr;
     }
 
     nsRefPtr<WorkerFetchResolver> r = new WorkerFetchResolver(proxy);
     return r.forget();
   }
 
   void
-  OnResponseAvailable(InternalResponse* aResponse) override;
+  OnResponseAvailableInternal(InternalResponse* aResponse) override;
 
   void
   OnResponseEnd() override;
 
 private:
   explicit WorkerFetchResolver(PromiseWorkerProxy* aProxy)
     : mPromiseProxy(aProxy)
   {
@@ -94,17 +94,17 @@ class MainThreadFetchResolver final : pu
   nsRefPtr<Promise> mPromise;
   nsRefPtr<Response> mResponse;
 
   NS_DECL_OWNINGTHREAD
 public:
   explicit MainThreadFetchResolver(Promise* aPromise);
 
   void
-  OnResponseAvailable(InternalResponse* aResponse) override;
+  OnResponseAvailableInternal(InternalResponse* aResponse) override;
 
 private:
   ~MainThreadFetchResolver();
 };
 
 class MainThreadFetchRunnable : public nsRunnable
 {
   nsRefPtr<WorkerFetchResolver> mResolver;
@@ -232,17 +232,17 @@ FetchRequest(nsIGlobalObject* aGlobal, c
 }
 
 MainThreadFetchResolver::MainThreadFetchResolver(Promise* aPromise)
   : mPromise(aPromise)
 {
 }
 
 void
-MainThreadFetchResolver::OnResponseAvailable(InternalResponse* aResponse)
+MainThreadFetchResolver::OnResponseAvailableInternal(InternalResponse* aResponse)
 {
   NS_ASSERT_OWNINGTHREAD(MainThreadFetchResolver);
   AssertIsOnMainThread();
 
   if (aResponse->Type() != ResponseType::Error) {
     nsCOMPtr<nsIGlobalObject> go = mPromise->GetParentObject();
     mResponse = new Response(go, aResponse);
     mPromise->MaybeResolve(mResponse);
@@ -312,17 +312,17 @@ public:
     aWorkerPrivate->AssertIsOnWorkerThread();
 
     mResolver->mPromiseProxy->CleanUp(aCx);
     return true;
   }
 };
 
 void
-WorkerFetchResolver::OnResponseAvailable(InternalResponse* aResponse)
+WorkerFetchResolver::OnResponseAvailableInternal(InternalResponse* aResponse)
 {
   AssertIsOnMainThread();
 
   MutexAutoLock lock(mPromiseProxy->Lock());
   if (mPromiseProxy->CleanedUp()) {
     return;
   }
 
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -879,23 +879,29 @@ FetchDriver::OnDataAvailable(nsIRequest*
 }
 
 NS_IMETHODIMP
 FetchDriver::OnStopRequest(nsIRequest* aRequest,
                            nsISupports* aContext,
                            nsresult aStatusCode)
 {
   workers::AssertIsOnMainThread();
-  if (mPipeOutputStream) {
-    mPipeOutputStream->Close();
+  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;
   }
 
-  if (NS_FAILED(aStatusCode)) {
-    FailWithNetworkError();
-    return aStatusCode;
+  if (mPipeOutputStream) {
+    mPipeOutputStream->Close();
   }
 
   ContinueHttpFetchAfterNetworkFetch();
   return NS_OK;
 }
 
 // This is called when the channel is redirected.
 NS_IMETHODIMP
--- a/dom/fetch/FetchDriver.h
+++ b/dom/fetch/FetchDriver.h
@@ -27,24 +27,37 @@ namespace mozilla {
 namespace dom {
 
 class InternalRequest;
 class InternalResponse;
 
 class FetchDriverObserver
 {
 public:
+  FetchDriverObserver() : mGotResponseAvailable(false)
+  { }
+
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FetchDriverObserver);
-  virtual void OnResponseAvailable(InternalResponse* aResponse) = 0;
+  void OnResponseAvailable(InternalResponse* aResponse)
+  {
+    MOZ_ASSERT(!mGotResponseAvailable);
+    mGotResponseAvailable = true;
+    OnResponseAvailableInternal(aResponse);
+  }
   virtual void OnResponseEnd()
   { };
 
 protected:
   virtual ~FetchDriverObserver()
   { };
+
+  virtual void OnResponseAvailableInternal(InternalResponse* aResponse) = 0;
+
+private:
+  bool mGotResponseAvailable;
 };
 
 class FetchDriver final : public nsIStreamListener,
                           public nsIChannelEventSink,
                           public nsIInterfaceRequestor,
                           public nsIAsyncVerifyRedirectCallback,
                           public nsIThreadRetargetableStreamListener
 {
--- a/dom/workers/test/serviceworkers/fetch/fetch_tests.js
+++ b/dom/workers/test/serviceworkers/fetch/fetch_tests.js
@@ -314,16 +314,33 @@ expectAsyncResult();
 fetch(new Request('body-blob', {method: 'POST', body: new Blob(new String('my body'))}))
 .then(function(res) {
   return res.text();
 }).then(function(body) {
   my_ok(body == 'my bodymy body', "the Blob body of the intercepted fetch should be visible in the SW");
   finish();
 });
 
+expectAsyncResult();
+fetch('interrupt.sjs')
+.then(function(res) {
+  my_ok(true, "interrupted fetch succeeded");
+  res.text().then(function(body) {
+    my_ok(false, "interrupted fetch shouldn't have complete body");
+    finish();
+  },
+  function() {
+    my_ok(true, "interrupted fetch shouldn't have complete body");
+    finish();
+  })
+}, function(e) {
+  my_ok(false, "interrupted fetch failed");
+  finish();
+});
+
 ['DELETE', 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT'].forEach(function(method) {
   fetchXHRWithMethod('xhr-method-test.txt', method, function(xhr) {
     my_ok(xhr.status == 200, method + " load should be successful");
     my_ok(xhr.responseText == ("intercepted " + method), method + " load should have synthesized response");
     finish();
   });
 });
 
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/serviceworkers/fetch/interrupt.sjs
@@ -0,0 +1,20 @@
+function handleRequest(request, response) {
+  var body = "a";
+  for (var i = 0; i < 20; i++) {
+    body += body;
+  }
+
+  response.seizePower();
+  response.write("HTTP/1.1 200 OK\r\n")
+  var count = 10;
+  response.write("Content-Length: " + body.length * count + "\r\n");
+  response.write("Content-Type: text/plain; charset=utf-8\r\n");
+  response.write("Cache-Control: no-cache, must-revalidate\r\n");
+  response.write("\r\n");
+
+  for (var i = 0; i < count; i++) {
+    response.write(body);
+  }
+
+  throw Components.results.NS_BINDING_ABORTED;
+}
--- a/dom/workers/test/serviceworkers/mochitest.ini
+++ b/dom/workers/test/serviceworkers/mochitest.ini
@@ -45,16 +45,17 @@ support-files =
   fetch/https/index.html
   fetch/https/register.html
   fetch/https/unregister.html
   fetch/https/https_test.js
   fetch/https/clonedresponse/index.html
   fetch/https/clonedresponse/register.html
   fetch/https/clonedresponse/unregister.html
   fetch/https/clonedresponse/https_test.js
+  fetch/interrupt.sjs
   fetch/origin/index.sjs
   fetch/origin/index-to-https.sjs
   fetch/origin/realindex.html
   fetch/origin/realindex.html^headers^
   fetch/origin/register.html
   fetch/origin/unregister.html
   fetch/origin/origin_test.js
   fetch/origin/https/index-https.sjs