Bug 1198544 - Prevent FetchDriver from creating multiple responses if OnStopRequest yields a failing status code. r=nsm
☠☠ backed out by 9ef95c4879fa ☠ ☠
authorJosh Matthews <josh@joshmatthews.net>
Fri, 11 Sep 2015 13:23:29 -0400
changeset 294674 132aa442af953fab46644e34c60c6782350b703b
parent 294673 d2457f064ba320b6b32b3b5995d124ec134115fb
child 294675 1fd3662ece10b3de40a53fd092479f7ff9c48cbc
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
@@ -878,25 +878,27 @@ FetchDriver::OnDataAvailable(nsIRequest*
 }
 
 NS_IMETHODIMP
 FetchDriver::OnStopRequest(nsIRequest* aRequest,
                            nsISupports* aContext,
                            nsresult aStatusCode)
 {
   workers::AssertIsOnMainThread();
-  if (mPipeOutputStream) {
+  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.
+  } else if (mPipeOutputStream) {
     mPipeOutputStream->Close();
   }
 
-  if (NS_FAILED(aStatusCode)) {
-    FailWithNetworkError();
-    return aStatusCode;
-  }
-
   ContinueHttpFetchAfterNetworkFetch();
   return NS_OK;
 }
 
 // This is called when the channel is redirected.
 NS_IMETHODIMP
 FetchDriver::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
                                     nsIChannel* aNewChannel,
--- 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