diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index dea4e51317b9..b24029bedd4f 100644 --- a/dom/media/MediaManager.cpp +++ b/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 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> 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 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")) { diff --git a/dom/media/systemservices/MediaChild.cpp b/dom/media/systemservices/MediaChild.cpp index 45507987196b..adca365fa38b 100644 --- a/dom/media/systemservices/MediaChild.cpp +++ b/dom/media/systemservices/MediaChild.cpp @@ -24,22 +24,16 @@ PRLogModuleInfo *gMediaChildLog; namespace mozilla { namespace media { -static PMediaChild* sChild = nullptr; - -template -ChildPledge::ChildPledge() -{ - MOZ_ASSERT(MediaManager::IsInMediaThread()); -} +static Child* sChild; template void ChildPledge::ActorCreated(PBackgroundChild* aActor) { if (!sChild) { // Create PMedia by sending a message to the parent - sChild = aActor->SendPMediaConstructor(); + sChild = static_cast(aActor->SendPMediaConstructor()); } - Run(static_cast(sChild)); + Run(sChild); } template 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(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& aPledge) { - return new Child(); + uint32_t id = ++sRequestCounter; + nsRefPtr> ptr(&aPledge); + mRequestPledges.AppendElement(PledgeEntry(id, ptr)); + return id; +} + +already_AddRefed> +Child::RemoveRequestPledge(uint32_t aRequestId) +{ + for (PledgeEntry& entry : mRequestPledges) { + if (entry.first == aRequestId) { + nsRefPtr> 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> 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(aActor)->Release(); + return true; } } diff --git a/dom/media/systemservices/MediaChild.h b/dom/media/systemservices/MediaChild.h index 7645726ba4a5..044f8a159623 100644 --- a/dom/media/systemservices/MediaChild.h +++ b/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 class ChildPledge : public Pledge, 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& aPledge); + already_AddRefed> RemoveRequestPledge(uint32_t aRequestId); +private: virtual ~Child(); + + typedef std::pair>> PledgeEntry; + static uint32_t sRequestCounter; + nsTArray mRequestPledges; }; -PMediaChild* CreateMediaChild(); +PMediaChild* AllocPMediaChild(); +bool DeallocPMediaChild(PMediaChild *aActor); } // namespace media } // namespace mozilla diff --git a/dom/media/systemservices/MediaParent.cpp b/dom/media/systemservices/MediaParent.cpp index 1fffc92e96b1..5e59fecb0476 100644 --- a/dom/media/systemservices/MediaParent.cpp +++ b/dom/media/systemservices/MediaParent.cpp @@ -7,6 +7,7 @@ #include "MediaParent.h" #include "mozilla/Base64.h" +#include #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 sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); + MOZ_ASSERT(sts); + nsRefPtr singleton(mSingleton); + + nsRefPtr> p = PledgeRunnable::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 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 sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); + MOZ_ASSERT(sts); + nsRefPtr singleton(mSingleton); + + nsRefPtr> p = PledgeRunnable::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(aActor)->Release(); + return true; } } diff --git a/dom/media/systemservices/MediaParent.h b/dom/media/systemservices/MediaParent.h index 98a8e66a30cb..b94c872f5a81 100644 --- a/dom/media/systemservices/MediaParent.h +++ b/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 mSingleton; + bool mDestroyed; }; -PMediaParent* CreateParent(); +PMediaParent* AllocPMediaParent(); +bool DeallocPMediaParent(PMediaParent *aActor); } // namespace media } // namespace mozilla diff --git a/dom/media/systemservices/MediaUtils.cpp b/dom/media/systemservices/MediaUtils.cpp index ecc109752ef9..17fb500efb67 100644 --- a/dom/media/systemservices/MediaUtils.cpp +++ b/dom/media/systemservices/MediaUtils.cpp @@ -6,18 +6,8 @@ #include "MediaUtils.h" -#include "mozilla/MediaManager.h" - namespace mozilla { namespace media { -template -Pledge::Pledge() - : mDone(false) - , mResult(NS_OK) -{ - MOZ_ASSERT(MediaManager::IsInMediaThread()); -} - } } diff --git a/dom/media/systemservices/MediaUtils.h b/dom/media/systemservices/MediaUtils.h index f1fc5c41314a..a6a5a833438b 100644 --- a/dom/media/systemservices/MediaUtils.h +++ b/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 + void Then(OnSuccessType aOnSuccess) + { + Then(aOnSuccess, [](nsresult){}); + } template 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 mFunctors; }; +// General purpose runnable that also acts as a Pledge for the resulting value. +// Use PledgeRunnable<>::New() factory function to use with lambdas. + +template +class PledgeRunnable : public Pledge, public nsRunnable +{ +public: + template + static PledgeRunnable* + New(OnRunType aOnRun) + { + class P : public PledgeRunnable + { + 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 mOriginThread; + OnRunType mOnRun; + bool mHasRun; + }; + + return new P(aOnRun); + } + +protected: + virtual ~PledgeRunnable() {} +}; + +// General purpose runnable with an eye toward lambdas + +namespace CallbackRunnable +{ +template +class Impl : public nsRunnable +{ +public: + explicit Impl(OnRunType& aOnRun) : mOnRun(aOnRun) {} +private: + NS_IMETHODIMP + Run() + { + return mOnRun(); + } + OnRunType mOnRun; +}; + +template +Impl* +New(OnRunType aOnRun) +{ + return new Impl(aOnRun); +} +} + } } diff --git a/dom/media/systemservices/PMedia.ipdl b/dom/media/systemservices/PMedia.ipdl index a0f0ab88b9bf..b25423be3599 100644 --- a/dom/media/systemservices/PMedia.ipdl +++ b/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 diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp index baf9ff54f89f..7d4c61f2f214 100644 --- a/ipc/glue/BackgroundChildImpl.cpp +++ b/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 diff --git a/ipc/glue/BackgroundParentImpl.cpp b/ipc/glue/BackgroundParentImpl.cpp index f3428bf5aae8..09417ea1f225 100644 --- a/ipc/glue/BackgroundParentImpl.cpp +++ b/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 {