Bug 1399646: Part 3 - Improve handling of StreamFilters at shutdown. r=mixedpuppy
authorKris Maglione <maglione.k@gmail.com>
Wed, 13 Sep 2017 13:40:08 -0700
changeset 430326 f87e1d6810536ab0dc940f95563869d86aa937d3
parent 430325 8220fde3db60b243e9d8b8abd82562bb31509deb
child 430327 c7c638b31626ace9d0951e7a69450615673c9656
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1399646
milestone57.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 1399646: Part 3 - Improve handling of StreamFilters at shutdown. r=mixedpuppy The main change here is to disconnect stream filters immediately if we try to send start or data events to a window that's already been destroyed. It also fixes a race where we end up in the wrong state if a stop event arrives while the channel is being disconnected. MozReview-Commit-ID: LwxXxoRUDgQ
toolkit/components/extensions/webrequest/StreamFilter.cpp
toolkit/components/extensions/webrequest/StreamFilter.h
toolkit/components/extensions/webrequest/StreamFilterChild.cpp
--- a/toolkit/components/extensions/webrequest/StreamFilter.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilter.cpp
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "StreamFilter.h"
 
 #include "jsapi.h"
 #include "jsfriendapi.h"
+#include "xpcpublic.h"
 
 #include "mozilla/AbstractThread.h"
 #include "mozilla/HoldDropJSObjects.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/extensions/StreamFilterChild.h"
 #include "mozilla/extensions/StreamFilterEvents.h"
 #include "mozilla/extensions/StreamFilterParent.h"
 #include "mozilla/dom/ContentChild.h"
@@ -120,16 +121,29 @@ StreamFilter::FinishConnect(mozilla::ipc
 
     // IPC now owns this reference.
     Unused << do_AddRef(mActor);
   } else {
     mActor->RecvInitialized(false);
   }
 }
 
+bool
+StreamFilter::CheckAlive()
+{
+  // Check whether the global that owns this StreamFitler is still scriptable
+  // and, if not, disconnect the actor so that it can be cleaned up.
+  JSObject* wrapper = GetWrapperPreserveColor();
+  if (!wrapper || !xpc::Scriptability::Get(wrapper).Allowed()) {
+    ForgetActor();
+    return false;
+  }
+  return true;
+}
+
 /*****************************************************************************
  * Binding methods
  *****************************************************************************/
 
 template <typename T>
 static inline bool
 ReadTypedArrayData(nsTArray<uint8_t>& aData, const T& aArray, ErrorResult& aRv)
 {
--- a/toolkit/components/extensions/webrequest/StreamFilter.h
+++ b/toolkit/components/extensions/webrequest/StreamFilter.h
@@ -74,16 +74,18 @@ protected:
   virtual ~StreamFilter();
 
   void FireEvent(const nsAString& aType);
 
   void FireDataEvent(const nsTArray<uint8_t>& aData);
 
   void FireErrorEvent(const nsAString& aError);
 
+  bool CheckAlive();
+
 private:
   void
   Connect();
 
   void
   FinishConnect(mozilla::ipc::Endpoint<PStreamFilterChild>&& aEndpoint);
 
   void ForgetActor();
--- a/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilterChild.cpp
@@ -251,22 +251,29 @@ StreamFilterChild::SetNextState()
 
 void
 StreamFilterChild::MaybeStopRequest()
 {
   if (!mReceivedOnStop || !mBufferedData.isEmpty()) {
     return;
   }
 
+  if (mStreamFilter) {
+    Unused << mStreamFilter->CheckAlive();
+  }
+
   switch (mState) {
   case State::Suspending:
   case State::Resuming:
     mNextState = State::FinishedTransferringData;
     return;
 
+  case State::Disconnecting:
+    break;
+
   default:
     mState = State::FinishedTransferringData;
     if (mStreamFilter) {
       mStreamFilter->FireEvent(NS_LITERAL_STRING("stop"));
       // We don't need access to the stream filter after this point, so break our
       // reference cycle, so that it can be collected if we're the last reference.
       mStreamFilter = nullptr;
     }
@@ -425,16 +432,17 @@ IPCResult
 StreamFilterChild::RecvStartRequest()
 {
   MOZ_ASSERT(mState == State::Initialized);
 
   mState = State::TransferringData;
 
   if (mStreamFilter) {
     mStreamFilter->FireEvent(NS_LITERAL_STRING("start"));
+    Unused << mStreamFilter->CheckAlive();
   }
   return IPC_OK();
 }
 
 IPCResult
 StreamFilterChild::RecvStopRequest(const nsresult& aStatus)
 {
   mReceivedOnStop = true;
@@ -467,16 +475,20 @@ StreamFilterChild::FlushBufferedData()
   }
 }
 
 IPCResult
 StreamFilterChild::RecvData(Data&& aData)
 {
   MOZ_ASSERT(!mReceivedOnStop);
 
+  if (mStreamFilter) {
+    Unused << mStreamFilter->CheckAlive();
+  }
+
   switch (mState) {
   case State::TransferringData:
   case State::Resuming:
     EmitData(aData);
     break;
 
   case State::FinishedTransferringData:
     MOZ_ASSERT_UNREACHABLE("Received data in unexpected state");