From 9fd5acd0bddc00d2b0b5e0785af2521e795c1e28 Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Sat, 16 May 2015 09:09:25 -0700 Subject: [PATCH] Back out 3 changesets (bug 1162720) for mozilla::media::Parent::Release crashes CLOSED TREE Backed out changeset 05306872093a (bug 1162720) Backed out changeset 94a7098042fb (bug 1162720) Backed out changeset 7c2f391a7fdd (bug 1162720) --- dom/media/MediaManager.cpp | 2 +- dom/media/systemservices/MediaParent.cpp | 118 +++++------------- dom/media/systemservices/MediaUtils.h | 73 +++++++++-- dom/media/tests/mochitest/mochitest.ini | 2 - .../mochitest/test_enumerateDevices.html | 32 ----- 5 files changed, 96 insertions(+), 131 deletions(-) delete mode 100644 dom/media/tests/mochitest/test_enumerateDevices.html diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index 3a2a1c321369..f33d1a2f9369 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -2288,7 +2288,7 @@ MediaManager::Observe(nsISupports* aSubject, const char* aTopic, // cleared until the lambda function clears it. MediaManager::GetMessageLoop()->PostTask(FROM_HERE, new ShutdownTask( - media::NewRunnableFrom([this]() mutable { + media::CallbackRunnable::New([this]() mutable { // Close off any remaining active windows. MutexAutoLock lock(mMutex); GetActiveWindows()->Clear(); diff --git a/dom/media/systemservices/MediaParent.cpp b/dom/media/systemservices/MediaParent.cpp index 633b1763ccc3..5239233afb28 100644 --- a/dom/media/systemservices/MediaParent.cpp +++ b/dom/media/systemservices/MediaParent.cpp @@ -104,7 +104,10 @@ class ParentSingleton : public nsISupports class OriginKeysLoader : public OriginKeysTable { public: - OriginKeysLoader() {} + OriginKeysLoader() + { + Load(); + } nsresult GetOriginKey(const nsACString& aOrigin, nsCString& result) @@ -120,7 +123,13 @@ class ParentSingleton : public nsISupports already_AddRefed GetFile() { - MOZ_ASSERT(mProfileDir); + if (!mProfileDir) { + nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, + getter_AddRefs(mProfileDir)); + if (NS_WARN_IF(NS_FAILED(rv))) { + return nullptr; + } + } nsCOMPtr file; nsresult rv = mProfileDir->Clone(getter_AddRefs(file)); if (NS_WARN_IF(NS_FAILED(rv))) { @@ -308,17 +317,6 @@ class ParentSingleton : public nsISupports return NS_OK; } - void - SetProfileDir(nsIFile* aProfileDir) - { - MOZ_ASSERT(!NS_IsMainThread()); - bool first = !mProfileDir; - mProfileDir = aProfileDir; - // Load from disk when we first get a profileDir, but not subsequently. - if (first) { - Load(); - } - } private: nsCOMPtr mProfileDir; }; @@ -357,60 +355,25 @@ Parent::RecvGetOriginKey(const uint32_t& aRequestId, const nsCString& aOrigin, const bool& aPrivateBrowsing) { - // TODO: Replace all this when moving MediaParent to PContent soon (1037389) + // Hand over to stream-transport thread. + nsCOMPtr sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); + MOZ_ASSERT(sts); nsRefPtr singleton(mSingleton); - nsCOMPtr returnThread = NS_GetCurrentThread(); - nsRefPtr> p = new Pledge(); - nsresult rv; - // First, over to main thread to get profile dir. - - rv = NS_DispatchToMainThread(NewRunnableFrom([p, returnThread, singleton, aOrigin, - aPrivateBrowsing]() -> nsresult { - MOZ_ASSERT(NS_IsMainThread()); - nsCOMPtr profileDir; - nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, - getter_AddRefs(profileDir)); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - // Then from there over to stream-transport thread to do the actual file io. - - nsCOMPtr sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); - MOZ_ASSERT(sts); - rv = sts->Dispatch(NewRunnableFrom([profileDir, p, returnThread, singleton, - aOrigin, aPrivateBrowsing]() -> nsresult { - MOZ_ASSERT(!NS_IsMainThread()); - singleton->mOriginKeys.SetProfileDir(profileDir); - nsCString result; - if (aPrivateBrowsing) { - singleton->mPrivateBrowsingOriginKeys.GetOriginKey(aOrigin, result); - } else { - singleton->mOriginKeys.GetOriginKey(aOrigin, result); - } - - // Pass result back to original thread. - nsresult rv; - rv = returnThread->Dispatch(NewRunnableFrom([p, result]() -> nsresult { - p->Resolve(result); - return NS_OK; - }), NS_DISPATCH_NORMAL); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - return NS_OK; - }), NS_DISPATCH_NORMAL); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; + 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) { @@ -424,36 +387,19 @@ Parent::RecvGetOriginKey(const uint32_t& aRequestId, bool Parent::RecvSanitizeOriginKeys(const uint64_t& aSinceWhen) { + // Hand over to stream-transport thread. + + nsCOMPtr sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); + MOZ_ASSERT(sts); nsRefPtr singleton(mSingleton); - // First, over to main to get profile dir. - nsresult rv; - - rv = NS_DispatchToMainThread(NewRunnableFrom([singleton, - aSinceWhen]() -> nsresult { - MOZ_ASSERT(NS_IsMainThread()); - nsCOMPtr profileDir; - nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, - getter_AddRefs(profileDir)); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - // Then from there over to stream-transport thread to do the file io. - - nsCOMPtr sts = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID); - MOZ_ASSERT(sts); - rv = sts->Dispatch(NewRunnableFrom([profileDir, singleton, aSinceWhen]() -> nsresult { - MOZ_ASSERT(!NS_IsMainThread()); - singleton->mOriginKeys.SetProfileDir(profileDir); - singleton->mPrivateBrowsingOriginKeys.Clear(aSinceWhen); - singleton->mOriginKeys.Clear(aSinceWhen); - return NS_OK; - }), NS_DISPATCH_NORMAL); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } + 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; } diff --git a/dom/media/systemservices/MediaUtils.h b/dom/media/systemservices/MediaUtils.h index 03b3bf039a80..a6a5a833438b 100644 --- a/dom/media/systemservices/MediaUtils.h +++ b/dom/media/systemservices/MediaUtils.h @@ -37,7 +37,6 @@ class Pledge }; public: - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Pledge); explicit Pledge() : mDone(false), mResult(NS_OK) {} template @@ -78,12 +77,13 @@ public: } } +protected: void Resolve(const ValueType& aValue) { mValue = aValue; Resolve(); } -protected: + void Resolve() { if (!mDone) { @@ -108,20 +108,72 @@ protected: ValueType mValue; protected: - ~Pledge() {}; bool mDone; nsresult mResult; private: nsAutoPtr mFunctors; }; -// General purpose runnable with an eye toward lambdas +// General purpose runnable that also acts as a Pledge for the resulting value. +// Use PledgeRunnable<>::New() factory function to use with lambdas. -template -class LambdaRunnable : public nsRunnable +template +class PledgeRunnable : public Pledge, public nsRunnable { public: - explicit LambdaRunnable(OnRunType& aOnRun) : mOnRun(aOnRun) {} + 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() @@ -132,10 +184,11 @@ private: }; template -LambdaRunnable* -NewRunnableFrom(OnRunType aOnRun) +Impl* +New(OnRunType aOnRun) { - return new LambdaRunnable(aOnRun); + return new Impl(aOnRun); +} } } diff --git a/dom/media/tests/mochitest/mochitest.ini b/dom/media/tests/mochitest/mochitest.ini index cfdb3948d86a..ef497f1eb668 100644 --- a/dom/media/tests/mochitest/mochitest.ini +++ b/dom/media/tests/mochitest/mochitest.ini @@ -29,8 +29,6 @@ skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g(Bug 960442, video suppo skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g emulator seems to be too slow (Bug 1016498 and 1008080) [test_dataChannel_noOffer.html] skip-if = toolkit == 'gonk' || buildapp == 'mulet' # b2g (Bug 1059867) -[test_enumerateDevices.html] -skip-if = buildapp == 'mulet' [test_getUserMedia_basicAudio.html] skip-if = (toolkit == 'gonk' || buildapp == 'mulet' && debug) # debug-only failure [test_getUserMedia_basicVideo.html] diff --git a/dom/media/tests/mochitest/test_enumerateDevices.html b/dom/media/tests/mochitest/test_enumerateDevices.html deleted file mode 100644 index d91e4b37b197..000000000000 --- a/dom/media/tests/mochitest/test_enumerateDevices.html +++ /dev/null @@ -1,32 +0,0 @@ - - - - - - -
-
-
- -