Bug 1046245 - IO on STS thread + cleanup BackgroundChild on shutdown. r=jesup

dom/media/MediaManager.cpp

@@ -41,6 +41,7 @@
 #include "mozilla/dom/GetUserMediaRequestBinding.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Base64.h"
+#include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/media/MediaChild.h"
 #include "MediaTrackConstraints.h"
 #include "VideoUtils.h"
@@ -1497,9 +1498,12 @@ public:
         result->AppendElement(source);
       }
     }
-    nsRefPtr<DeviceSuccessCallbackRunnable> runnable
-        (new DeviceSuccessCallbackRunnable(mWindowId, mOnSuccess, mOnFailure,
-                                           result.forget()));
+    // In the case of failure with this newly allocated runnable, we
+    // intentionally leak the runnable, because we've pawned mOnSuccess and
+    // mOnFailure onto it which are main thread objects unsafe to release here.
+    DeviceSuccessCallbackRunnable* runnable =
+        new DeviceSuccessCallbackRunnable(mWindowId, mOnSuccess, mOnFailure,
+                                          result.forget());
     if (mPrivileged) {
       NS_DispatchToMainThread(runnable);
     } else {
@@ -1508,13 +1512,13 @@ public:
       // GetOriginKey is an async API that returns a pledge (as promise-like
       // pattern). We use .Then() to pass in a lambda to run back on this
       // thread once GetOriginKey resolves asynchronously . The "runnable"
-      // nsRefPtr is "captured" (passed by value) into the lambda.
+      // pointer is "captured" (passed by value) into the lambda.
       nsRefPtr<media::ChildPledge<nsCString>> p =
           media::GetOriginKey(mOrigin, mInPrivateBrowsing);
       p->Then([runnable](nsCString result) mutable {
         runnable->mOriginKey = result;
         NS_DispatchToMainThread(runnable);
-      }, [](nsresult rv){});
+      });
     }
     // One of the Runnables have taken these.
     MOZ_ASSERT(!mOnSuccess && !mOnFailure);
@@ -2205,8 +2209,34 @@ MediaManager::Observe(nsISupports* aSubject, const char* aTopic,
       prefs->RemoveObserver("media.navigator.video.default_minfps", this);
     }
 
-    // Close off any remaining active windows.
+    // Because mMediaThread is not an nsThread, we must dispatch to it so it can
+    // clean up BackgroundChild. Continue stopping thread once this is done.
+
+    class ShutdownTask : public Task
     {
+    public:
+      explicit ShutdownTask(nsRunnable* aReply) : mReply(aReply) {}
+    private:
+      virtual void
+      Run()
+      {
+        MOZ_ASSERT(MediaManager::IsInMediaThread());
+        mozilla::ipc::BackgroundChild::CloseForCurrentThread();
+        NS_DispatchToMainThread(mReply);
+      }
+      nsRefPtr<nsRunnable> mReply;
+    };
+
+    // Post ShutdownTask to execute on mMediaThread and pass in a lambda
+    // callback to be executed back on this thread once it is done.
+    //
+    // The lambda callback "captures" the 'this' pointer for member access.
+    // This is safe since this is guaranteed to be here since sSingleton isn't
+    // cleared until the lambda function clears it.
+
+    MediaManager::GetMessageLoop()->PostTask(FROM_HERE, new ShutdownTask(
+        media::CallbackRunnable::New([this]() mutable {
+      // Close off any remaining active windows.
       MutexAutoLock lock(mMutex);
       GetActiveWindows()->Clear();
       mActiveCallbacks.Clear();
@@ -2218,8 +2248,8 @@ MediaManager::Observe(nsISupports* aSubject, const char* aTopic,
         mMediaThread->Stop();
       }
       mBackend = nullptr;
-    }
-
+      return NS_OK;
+    })));
     return NS_OK;
 
   } else if (!strcmp(aTopic, "getUserMedia:response:allow")) {

dom/media/systemservices/MediaChild.cpp

@@ -24,22 +24,16 @@ PRLogModuleInfo *gMediaChildLog;
 namespace mozilla {
 namespace media {
 
-static PMediaChild* sChild = nullptr;
-
-template<typename ValueType>
-ChildPledge<ValueType>::ChildPledge()
-{
-  MOZ_ASSERT(MediaManager::IsInMediaThread());
-}
+static Child* sChild;
 
 template<typename ValueType> void
 ChildPledge<ValueType>::ActorCreated(PBackgroundChild* aActor)
 {
   if (!sChild) {
     // Create PMedia by sending a message to the parent
-    sChild = aActor->SendPMediaConstructor();
+    sChild = static_cast<Child*>(aActor->SendPMediaConstructor());
   }
-  Run(static_cast<Child*>(sChild));
+  Run(sChild);
 }
 
 template<typename ValueType> void
@@ -64,11 +58,12 @@ GetOriginKey(const nsCString& aOrigin, bool aPrivateBrowsing)
     : mOrigin(aOrigin), mPrivateBrowsing(aPrivateBrowsing) {}
   private:
     ~Pledge() {}
-    void Run(PMediaChild* aMedia)
+    void Run(PMediaChild* aChild)
     {
-      aMedia->SendGetOriginKey(mOrigin, mPrivateBrowsing, &mValue);
-      LOG(("GetOriginKey for %s, result:", mOrigin.get(), mValue.get()));
-      Resolve();
+      Child* child = static_cast<Child*>(aChild);
+
+      uint32_t id = child->AddRequestPledge(*this);
+      child->SendGetOriginKey(id, mOrigin, mPrivateBrowsing);
     }
     const nsCString mOrigin;
     const bool mPrivateBrowsing;
@@ -124,10 +119,55 @@ Child::~Child()
   MOZ_COUNT_DTOR(Child);
 }
 
-PMediaChild*
-CreateMediaChild()
+uint32_t Child::sRequestCounter = 0;
+
+uint32_t
+Child::AddRequestPledge(ChildPledge<nsCString>& aPledge)
 {
-  return new Child();
+  uint32_t id = ++sRequestCounter;
+  nsRefPtr<ChildPledge<nsCString>> ptr(&aPledge);
+  mRequestPledges.AppendElement(PledgeEntry(id, ptr));
+  return id;
+}
+
+already_AddRefed<ChildPledge<nsCString>>
+Child::RemoveRequestPledge(uint32_t aRequestId)
+{
+  for (PledgeEntry& entry : mRequestPledges) {
+    if (entry.first == aRequestId) {
+      nsRefPtr<ChildPledge<nsCString>> ref;
+      ref.swap(entry.second);
+      mRequestPledges.RemoveElement(entry);
+      return ref.forget();
+    }
+  }
+  MOZ_ASSERT_UNREACHABLE("Received response with no matching media::ChildPledge!");
+  return nullptr;
+}
+
+bool
+Child::RecvGetOriginKeyResponse(const uint32_t& aRequestId, const nsCString& aKey)
+{
+  nsRefPtr<ChildPledge<nsCString>> pledge = RemoveRequestPledge(aRequestId);
+  if (pledge) {
+    pledge->Resolve(aKey);
+  }
+  return true;
+}
+
+PMediaChild*
+AllocPMediaChild()
+{
+  Child* obj = new Child();
+  obj->AddRef();
+  return obj;
+}
+
+bool
+DeallocPMediaChild(media::PMediaChild *aActor)
+{
+  static_cast<Child*>(aActor)->Release();
+  return true;
 }
 
 }

dom/media/systemservices/MediaChild.h

@@ -25,14 +25,17 @@ namespace media {
 // GetOriginKey and SanitizeOriginKeys are asynchronous APIs that return pledges
 // (promise-like objects) with the future value. Use pledge.Then(func) to access.
 
+class Child;
+
 template<typename ValueType>
 class ChildPledge : public Pledge<ValueType>,
                     public nsIIPCBackgroundChildCreateCallback
 {
+  friend Child;
   NS_DECL_NSIIPCBACKGROUNDCHILDCREATECALLBACK
   NS_DECL_ISUPPORTS
 public:
-  explicit ChildPledge();
+  explicit ChildPledge() {};
 protected:
   virtual ~ChildPledge() {}
   virtual void Run(PMediaChild* aMedia) = 0;
@@ -46,12 +49,24 @@ SanitizeOriginKeys(const uint64_t& aSinceWhen);
 
 class Child : public PMediaChild
 {
+  NS_INLINE_DECL_REFCOUNTING(Child)
 public:
   Child();
+
+  bool RecvGetOriginKeyResponse(const uint32_t& aRequestId, const nsCString& aKey);
+
+  uint32_t AddRequestPledge(ChildPledge<nsCString>& aPledge);
+  already_AddRefed<ChildPledge<nsCString>> RemoveRequestPledge(uint32_t aRequestId);
+private:
   virtual ~Child();
+
+  typedef std::pair<uint32_t,nsRefPtr<ChildPledge<nsCString>>> PledgeEntry;
+  static uint32_t sRequestCounter;
+  nsTArray<PledgeEntry> mRequestPledges;
 };
 
-PMediaChild* CreateMediaChild();
+PMediaChild* AllocPMediaChild();
+bool DeallocPMediaChild(PMediaChild *aActor);
 
 } // namespace media
 } // namespace mozilla

dom/media/systemservices/MediaParent.cpp

@@ -7,6 +7,7 @@
 #include "MediaParent.h"
 
 #include "mozilla/Base64.h"
+#include <mozilla/StaticMutex.h>
 
 #include "MediaUtils.h"
 #include "MediaEngine.h"
@@ -35,11 +36,12 @@ PRLogModuleInfo *gMediaParentLog;
 namespace mozilla {
 namespace media {
 
+static StaticMutex gMutex;
 static ParentSingleton* sParentSingleton = nullptr;
 
 class ParentSingleton : public nsISupports
 {
-  NS_DECL_ISUPPORTS
+  NS_DECL_THREADSAFE_ISUPPORTS
 
   class OriginKey
   {
@@ -332,6 +334,13 @@ private:
 public:
   static ParentSingleton* Get()
   {
+    // Protect creation of singleton and access from multiple Background threads.
+    //
+    // Multiple Background threads happen because sanitize.js calls us from the
+    // chrome process and gets a thread separate from the one servicing ipc from
+    // the content process.
+
+    StaticMutexAutoLock lock(gMutex);
     if (!sParentSingleton) {
       sParentSingleton = new ParentSingleton();
     }
@@ -345,23 +354,58 @@ public:
 NS_IMPL_ISUPPORTS0(ParentSingleton)
 
 bool
-Parent::RecvGetOriginKey(const nsCString& aOrigin,
-                         const bool& aPrivateBrowsing,
-                         nsCString* aKey)
+Parent::RecvGetOriginKey(const uint32_t& aRequestId,
+                         const nsCString& aOrigin,
+                         const bool& aPrivateBrowsing)
 {
-  if (aPrivateBrowsing) {
-    mSingleton->mPrivateBrowsingOriginKeys.GetOriginKey(aOrigin, *aKey);
-  } else {
-    mSingleton->mOriginKeys.GetOriginKey(aOrigin, *aKey);
+  // Hand over to stream-transport thread.
+
+  nsCOMPtr<nsIEventTarget> sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
+  MOZ_ASSERT(sts);
+  nsRefPtr<ParentSingleton> singleton(mSingleton);
+
+  nsRefPtr<PledgeRunnable<nsCString>> p = PledgeRunnable<nsCString>::New(
+      [singleton, aOrigin, aPrivateBrowsing](nsCString& aResult) {
+    if (aPrivateBrowsing) {
+      singleton->mPrivateBrowsingOriginKeys.GetOriginKey(aOrigin, aResult);
+    } else {
+      singleton->mOriginKeys.GetOriginKey(aOrigin, aResult);
+    }
+    return NS_OK;
+  });
+  nsresult rv = sts->Dispatch(p, NS_DISPATCH_NORMAL);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return false;
   }
+  nsRefPtr<media::Parent> keepAlive(this);
+  p->Then([this, keepAlive, aRequestId](const nsCString& aKey) mutable {
+    if (!mDestroyed) {
+      unused << SendGetOriginKeyResponse(aRequestId, aKey);
+    }
+    return NS_OK;
+  });
   return true;
 }
 
 bool
 Parent::RecvSanitizeOriginKeys(const uint64_t& aSinceWhen)
 {
-  mSingleton->mPrivateBrowsingOriginKeys.Clear(aSinceWhen);
-  mSingleton->mOriginKeys.Clear(aSinceWhen);
+  // Hand over to stream-transport thread.
+
+  nsCOMPtr<nsIEventTarget> sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
+  MOZ_ASSERT(sts);
+  nsRefPtr<ParentSingleton> singleton(mSingleton);
+
+  nsRefPtr<PledgeRunnable<bool>> p = PledgeRunnable<bool>::New(
+      [singleton, aSinceWhen](bool) {
+    singleton->mPrivateBrowsingOriginKeys.Clear(aSinceWhen);
+    singleton->mOriginKeys.Clear(aSinceWhen);
+    return NS_OK;
+  });
+  nsresult rv = sts->Dispatch(p, NS_DISPATCH_NORMAL);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return false;
+  }
   return true;
 }
 
@@ -369,11 +413,13 @@ void
 Parent::ActorDestroy(ActorDestroyReason aWhy)
 {
   // No more IPC from here
+  mDestroyed = true;
   LOG((__FUNCTION__));
 }
 
 Parent::Parent()
   : mSingleton(ParentSingleton::Get())
+  , mDestroyed(false)
 {
 #if defined(PR_LOGGING)
   if (!gMediaParentLog)
@@ -391,9 +437,19 @@ Parent::~Parent()
   MOZ_COUNT_DTOR(Parent);
 }
 
-PMediaParent* CreateParent()
+PMediaParent*
+AllocPMediaParent()
 {
-  return new Parent();
+  Parent* obj = new Parent();
+  obj->AddRef();
+  return obj;
+}
+
+bool
+DeallocPMediaParent(media::PMediaParent *aActor)
+{
+  static_cast<Parent*>(aActor)->Release();
+  return true;
 }
 
 }

dom/media/systemservices/MediaParent.h

@@ -21,20 +21,24 @@ class ParentSingleton;
 
 class Parent : public PMediaParent
 {
+  NS_INLINE_DECL_REFCOUNTING(Parent)
 public:
-  virtual bool RecvGetOriginKey(const nsCString& aOrigin,
-                                const bool& aPrivateBrowsing,
-                                nsCString* aKey) override;
+  virtual bool RecvGetOriginKey(const uint32_t& aRequestId,
+                                const nsCString& aOrigin,
+                                const bool& aPrivateBrowsing) override;
   virtual bool RecvSanitizeOriginKeys(const uint64_t& aSinceWhen) override;
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   Parent();
-  virtual ~Parent();
 private:
+  virtual ~Parent();
+
   nsRefPtr<ParentSingleton> mSingleton;
+  bool mDestroyed;
 };
 
-PMediaParent* CreateParent();
+PMediaParent* AllocPMediaParent();
+bool DeallocPMediaParent(PMediaParent *aActor);
 
 } // namespace media
 } // namespace mozilla

dom/media/systemservices/MediaUtils.cpp

@@ -6,18 +6,8 @@
 
 #include "MediaUtils.h"
 
-#include "mozilla/MediaManager.h"
-
 namespace mozilla {
 namespace media {
 
-template<typename ValueType>
-Pledge<ValueType>::Pledge()
-  : mDone(false)
-  , mResult(NS_OK)
-{
-  MOZ_ASSERT(MediaManager::IsInMediaThread());
-}
-
 }
 }

dom/media/systemservices/MediaUtils.h

@@ -8,6 +8,7 @@
 #define mozilla_MediaUtils_h
 
 #include "nsAutoPtr.h"
+#include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace media {
@@ -36,7 +37,13 @@ class Pledge
   };
 
 public:
-  explicit Pledge();
+  explicit Pledge() : mDone(false), mResult(NS_OK) {}
+
+  template<typename OnSuccessType>
+  void Then(OnSuccessType aOnSuccess)
+  {
+    Then(aOnSuccess, [](nsresult){});
+  }
 
   template<typename OnSuccessType, typename OnFailureType>
   void Then(OnSuccessType aOnSuccess, OnFailureType aOnFailure)
@@ -71,6 +78,12 @@ public:
   }
 
 protected:
+  void Resolve(const ValueType& aValue)
+  {
+    mValue = aValue;
+    Resolve();
+  }
+
   void Resolve()
   {
     if (!mDone) {
@@ -94,12 +107,90 @@ protected:
   }
 
   ValueType mValue;
-private:
+protected:
   bool mDone;
   nsresult mResult;
+private:
   nsAutoPtr<FunctorsBase> mFunctors;
 };
 
+// General purpose runnable that also acts as a Pledge for the resulting value.
+// Use PledgeRunnable<>::New() factory function to use with lambdas.
+
+template<typename ValueType>
+class PledgeRunnable : public Pledge<ValueType>, public nsRunnable
+{
+public:
+  template<typename OnRunType>
+  static PledgeRunnable<ValueType>*
+  New(OnRunType aOnRun)
+  {
+    class P : public PledgeRunnable<ValueType>
+    {
+    public:
+      explicit P(OnRunType& aOnRun)
+      : mOriginThread(NS_GetCurrentThread())
+      , mOnRun(aOnRun)
+      , mHasRun(false) {}
+    private:
+      virtual ~P() {}
+      NS_IMETHODIMP
+      Run()
+      {
+        if (!mHasRun) {
+          P::mResult = mOnRun(P::mValue);
+          mHasRun = true;
+          return mOriginThread->Dispatch(this, NS_DISPATCH_NORMAL);
+        }
+        bool on;
+        MOZ_RELEASE_ASSERT(NS_SUCCEEDED(mOriginThread->IsOnCurrentThread(&on)));
+        MOZ_RELEASE_ASSERT(on);
+
+        if (NS_SUCCEEDED(P::mResult)) {
+          P::Resolve();
+        } else {
+          P::Reject(P::mResult);
+        }
+        return NS_OK;
+      }
+      nsCOMPtr<nsIThread> mOriginThread;
+      OnRunType mOnRun;
+      bool mHasRun;
+    };
+
+    return new P(aOnRun);
+  }
+
+protected:
+  virtual ~PledgeRunnable() {}
+};
+
+// General purpose runnable with an eye toward lambdas
+
+namespace CallbackRunnable
+{
+template<typename OnRunType>
+class Impl : public nsRunnable
+{
+public:
+  explicit Impl(OnRunType& aOnRun) : mOnRun(aOnRun) {}
+private:
+  NS_IMETHODIMP
+  Run()
+  {
+    return mOnRun();
+  }
+  OnRunType mOnRun;
+};
+
+template<typename OnRunType>
+Impl<OnRunType>*
+New(OnRunType aOnRun)
+{
+  return new Impl<OnRunType>(aOnRun);
+}
+}
+
 }
 }
 

dom/media/systemservices/PMedia.ipdl

@@ -8,25 +8,27 @@ include protocol PBackground;
 namespace mozilla {
 namespace media {
 
-sync protocol PMedia
+protocol PMedia
 {
   manager PBackground;
 
 parent:
   /**
-   * Returns a persistent unique secret key for each origin.
+   * Requests a persistent unique secret key for each origin.
    * Has no expiry, but is cleared by age along with cookies.
    * This is needed by mediaDevices.enumerateDevices() to produce persistent
    * deviceIds that wont work cross-origin.
    */
-  sync GetOriginKey(nsCString origin, bool privateBrowsing) returns (nsCString key);
+  GetOriginKey(uint32_t aRequestId, nsCString aOrigin, bool aPrivateBrowsing);
 
   /**
-   * On clear cookies.
+   * On clear cookies. Fire and forget.
    */
-  sync SanitizeOriginKeys(uint64_t sinceWhen);
+  SanitizeOriginKeys(uint64_t aSinceWhen);
 
-  async __delete__();
+child:
+  GetOriginKeyResponse(uint32_t aRequestId, nsCString key);
+  __delete__();
 };
 
 } // namespace media

ipc/glue/BackgroundChildImpl.cpp

@@ -285,14 +285,13 @@ BackgroundChildImpl::DeallocPCacheStreamControlChild(PCacheStreamControlChild* a
 media::PMediaChild*
 BackgroundChildImpl::AllocPMediaChild()
 {
-  return media::CreateMediaChild();
+  return media::AllocPMediaChild();
 }
 
 bool
 BackgroundChildImpl::DeallocPMediaChild(media::PMediaChild *aActor)
 {
-  delete aActor;
-  return true;
+  return media::DeallocPMediaChild(aActor);
 }
 
 } // namespace ipc

ipc/glue/BackgroundParentImpl.cpp

@@ -364,14 +364,13 @@ BackgroundParentImpl::DeallocPBroadcastChannelParent(
 media::PMediaParent*
 BackgroundParentImpl::AllocPMediaParent()
 {
-  return media::CreateParent();
+  return media::AllocPMediaParent();
 }
 
 bool
 BackgroundParentImpl::DeallocPMediaParent(media::PMediaParent *aActor)
 {
-  delete aActor;
-  return true;
+  return media::DeallocPMediaParent(aActor);
 }
 
 namespace {