Bug 1444680: Follow-up: Fix StreamFilter race that turns up when running tests in parallel. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Mon, 12 Mar 2018 18:31:58 -0700
changeset 407739 09006f4350b625fdd91babb1c9940c260bb30dd3
parent 407738 c8ab2755d6fab269d39b2432617473726d0027a9
child 407740 f280eacb2abefbef7857fa55efeb7a436d72f46f
push id100765
push usermaglione.k@gmail.com
push dateTue, 13 Mar 2018 01:38:09 +0000
treeherdermozilla-inbound@09006f4350b6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1444680
milestone61.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 1444680: Follow-up: Fix StreamFilter race that turns up when running tests in parallel. r=mixedpuppy MozReview-Commit-ID: 9qGEmtq5J4H
toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
toolkit/components/extensions/webrequest/StreamFilterParent.cpp
--- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini
@@ -77,17 +77,16 @@ skip-if = os == "android"
 [test_ext_storage_sync_crypto.js]
 skip-if = os == "android"
 [test_ext_storage_telemetry.js]
 skip-if = os == "android" # checking for telemetry needs to be updated: 1384923
 [test_ext_trustworthy_origin.js]
 [test_ext_topSites.js]
 skip-if = os == "android"
 [test_ext_webRequest_filterResponseData.js]
-skip-if = debug
 [test_ext_webRequest_permission.js]
 [test_ext_webRequest_responseBody.js]
 [test_ext_webRequest_set_cookie.js]
 [test_ext_webRequest_suspend.js]
 [test_ext_webRequest_webSocket.js]
 [test_native_manifests.js]
 subprocess = true
 skip-if = os == "android"
--- a/toolkit/components/extensions/webrequest/StreamFilterParent.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilterParent.cpp
@@ -367,17 +367,16 @@ StreamFilterParent::RecvFlushedData()
  * Data output
  *****************************************************************************/
 
 IPCResult
 StreamFilterParent::RecvWrite(Data&& aData)
 {
   AssertIsActorThread();
 
-
   RunOnIOThread(
     NewRunnableMethod<Data&&>("StreamFilterParent::WriteMove",
                               this,
                               &StreamFilterParent::WriteMove,
                               Move(aData)));
   return IPC_OK();
 }
 
@@ -425,38 +424,42 @@ StreamFilterParent::OnStartRequest(nsIRe
     RunOnActorThread(FUNC, [=] {
       if (self->IPCActive()) {
         self->mState = State::Disconnected;
         CheckResult(self->SendError(NS_LITERAL_CSTRING("Channel redirected")));
       }
     });
   }
 
-  if (!mDisconnected) {
-    RefPtr<StreamFilterParent> self(this);
-    RunOnActorThread(FUNC, [=] {
-      if (self->IPCActive()) {
-        self->mState = State::TransferringData;
-        self->CheckResult(self->SendStartRequest());
-      }
-    });
-  }
-
   nsresult rv = mOrigListener->OnStartRequest(aRequest, aContext);
 
   // Important: Do this only *after* running the next listener in the chain, so
   // that we get the final delivery target after any retargeting that it may do.
   if (nsCOMPtr<nsIThreadRetargetableRequest> req = do_QueryInterface(aRequest)) {
     nsCOMPtr<nsIEventTarget> thread;
     Unused << req->GetDeliveryTarget(getter_AddRefs(thread));
     if (thread) {
       mIOThread = Move(thread);
     }
   }
 
+  // Important: Do this *after* we have set the thread delivery target, or it is
+  // possible in rare circumstances for an extension to attempt to write data
+  // before the thread has been set up, even though there are several layers of
+  // asynchrony involved.
+  if (!mDisconnected) {
+    RefPtr<StreamFilterParent> self(this);
+    RunOnActorThread(FUNC, [=] {
+      if (self->IPCActive()) {
+        self->mState = State::TransferringData;
+        self->CheckResult(self->SendStartRequest());
+      }
+    });
+  }
+
   return rv;
 }
 
 NS_IMETHODIMP
 StreamFilterParent::OnStopRequest(nsIRequest* aRequest,
                                   nsISupports* aContext,
                                   nsresult aStatusCode)
 {