diff --git a/dom/workers/WorkerDebuggerManager.cpp b/dom/workers/WorkerDebuggerManager.cpp index 8f6ff9f12db5..28c0a878dfb0 100644 --- a/dom/workers/WorkerDebuggerManager.cpp +++ b/dom/workers/WorkerDebuggerManager.cpp @@ -12,27 +12,53 @@ USING_WORKERS_NAMESPACE -class RegisterDebuggerRunnable final : public nsRunnable +class RegisterDebuggerMainThreadRunnable final : public nsRunnable { RefPtr mManager; - RefPtr mDebugger; - bool mHasListeners; + WorkerPrivate* mWorkerPrivate; + bool mNotifyListeners; public: - RegisterDebuggerRunnable(WorkerDebuggerManager* aManager, - WorkerDebugger* aDebugger, - bool aHasListeners) - : mManager(aManager), mDebugger(aDebugger), mHasListeners(aHasListeners) + RegisterDebuggerMainThreadRunnable(WorkerDebuggerManager* aManager, + WorkerPrivate* aWorkerPrivate, + bool aNotifyListeners) + : mManager(aManager), + mWorkerPrivate(aWorkerPrivate), + mNotifyListeners(aNotifyListeners) { } private: - ~RegisterDebuggerRunnable() + ~RegisterDebuggerMainThreadRunnable() { } NS_IMETHOD Run() override { - mManager->RegisterDebuggerOnMainThread(mDebugger, mHasListeners); + mManager->RegisterDebuggerMainThread(mWorkerPrivate, mNotifyListeners); + + return NS_OK; + } +}; + +class UnregisterDebuggerMainThreadRunnable final : public nsRunnable +{ + RefPtr mManager; + WorkerPrivate* mWorkerPrivate; + +public: + UnregisterDebuggerMainThreadRunnable(WorkerDebuggerManager* aManager, + WorkerPrivate* aWorkerPrivate) + : mManager(aManager), mWorkerPrivate(aWorkerPrivate) + { } + +private: + ~UnregisterDebuggerMainThreadRunnable() + { } + + NS_IMETHOD + Run() override + { + mManager->UnregisterDebuggerMainThread(mWorkerPrivate); return NS_OK; } @@ -42,14 +68,14 @@ BEGIN_WORKERS_NAMESPACE class WorkerDebuggerEnumerator final : public nsISimpleEnumerator { - nsTArray> mDebuggers; + nsTArray> mDebuggers; uint32_t mIndex; public: - explicit WorkerDebuggerEnumerator(const nsTArray& aDebuggers) - : mIndex(0) + explicit WorkerDebuggerEnumerator( + const nsTArray>& aDebuggers) + : mDebuggers(aDebuggers), mIndex(0) { - mDebuggers.AppendElements(aDebuggers); } NS_DECL_ISUPPORTS @@ -75,8 +101,7 @@ WorkerDebuggerEnumerator::GetNext(nsISupports** aResult) return NS_ERROR_FAILURE; } - nsCOMPtr element = mDebuggers.ElementAt(mIndex++); - element.forget(aResult); + mDebuggers.ElementAt(mIndex++).forget(aResult); return NS_OK; }; @@ -99,8 +124,6 @@ WorkerDebuggerManager::GetWorkerDebuggerEnumerator( { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - RefPtr enumerator = new WorkerDebuggerEnumerator(mDebuggers); enumerator.forget(aResult); @@ -149,95 +172,126 @@ WorkerDebuggerManager::ClearListeners() } void -WorkerDebuggerManager::RegisterDebugger(WorkerDebugger* aDebugger) +WorkerDebuggerManager::RegisterDebugger(WorkerPrivate* aWorkerPrivate) { - // May be called on any thread! - - bool hasListeners = false; - - { - MutexAutoLock lock(mMutex); - - hasListeners = !mListeners.IsEmpty(); - } + aWorkerPrivate->AssertIsOnParentThread(); if (NS_IsMainThread()) { - RegisterDebuggerOnMainThread(aDebugger, hasListeners); + // When the parent thread is the main thread, it will always block until all + // register liseners have been called, since it cannot continue until the + // call to RegisterDebuggerMainThread returns. + // + // In this case, it is always safe to notify all listeners on the main + // thread, even if there were no listeners at the time this method was + // called, so we can always pass true for the value of aNotifyListeners. + // This avoids having to lock mMutex to check whether mListeners is empty. + RegisterDebuggerMainThread(aWorkerPrivate, true); } else { + // We guarantee that if any register listeners are called, the worker does + // not start running until all register listeners have been called. To + // guarantee this, the parent thread should block until all register + // listeners have been called. + // + // However, to avoid overhead when the debugger is not being used, the + // parent thread will only block if there were any listeners at the time + // this method was called. As a result, we should not notify any listeners + // on the main thread if there were no listeners at the time this method was + // called, because the parent will not be blocking in that case. + bool hasListeners = false; + { + MutexAutoLock lock(mMutex); + + hasListeners = !mListeners.IsEmpty(); + } + nsCOMPtr runnable = - new RegisterDebuggerRunnable(this, aDebugger, hasListeners); + new RegisterDebuggerMainThreadRunnable(this, aWorkerPrivate, + hasListeners); MOZ_ALWAYS_TRUE(NS_SUCCEEDED( NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL))); if (hasListeners) { - aDebugger->WaitIsEnabled(true); + aWorkerPrivate->WaitForIsDebuggerRegistered(true); } } } void -WorkerDebuggerManager::UnregisterDebugger(WorkerDebugger* aDebugger) +WorkerDebuggerManager::UnregisterDebugger(WorkerPrivate* aWorkerPrivate) { - // May be called on any thread! + aWorkerPrivate->AssertIsOnParentThread(); if (NS_IsMainThread()) { - UnregisterDebuggerOnMainThread(aDebugger); + UnregisterDebuggerMainThread(aWorkerPrivate); } else { nsCOMPtr runnable = - NS_NewRunnableMethodWithArg>(this, - &WorkerDebuggerManager::UnregisterDebuggerOnMainThread, aDebugger); + new UnregisterDebuggerMainThreadRunnable(this, aWorkerPrivate); MOZ_ALWAYS_TRUE(NS_SUCCEEDED( NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL))); - aDebugger->WaitIsEnabled(false); + aWorkerPrivate->WaitForIsDebuggerRegistered(false); } } void -WorkerDebuggerManager::RegisterDebuggerOnMainThread(WorkerDebugger* aDebugger, - bool aHasListeners) +WorkerDebuggerManager::RegisterDebuggerMainThread(WorkerPrivate* aWorkerPrivate, + bool aNotifyListeners) { AssertIsOnMainThread(); - MOZ_ASSERT(!mDebuggers.Contains(aDebugger)); - mDebuggers.AppendElement(aDebugger); + RefPtr debugger = new WorkerDebugger(aWorkerPrivate); + mDebuggers.AppendElement(debugger); - nsTArray> listeners; - { - MutexAutoLock lock(mMutex); + aWorkerPrivate->SetDebugger(debugger); - listeners.AppendElements(mListeners); - } + if (aNotifyListeners) { + nsTArray> listeners; + { + MutexAutoLock lock(mMutex); + + listeners = mListeners; + } - if (aHasListeners) { for (size_t index = 0; index < listeners.Length(); ++index) { - listeners[index]->OnRegister(aDebugger); + listeners[index]->OnRegister(debugger); } } - aDebugger->Enable(); + aWorkerPrivate->SetIsDebuggerRegistered(true); } void -WorkerDebuggerManager::UnregisterDebuggerOnMainThread(WorkerDebugger* aDebugger) +WorkerDebuggerManager::UnregisterDebuggerMainThread( + WorkerPrivate* aWorkerPrivate) { AssertIsOnMainThread(); - MOZ_ASSERT(mDebuggers.Contains(aDebugger)); - mDebuggers.RemoveElement(aDebugger); + // There is nothing to do here if the debugger was never succesfully + // registered. We need to check this on the main thread because the worker + // does not wait for the registration to complete if there were no listeners + // installed when it started. + if (!aWorkerPrivate->IsDebuggerRegistered()) { + return; + } + + RefPtr debugger = aWorkerPrivate->Debugger(); + mDebuggers.RemoveElement(debugger); + + aWorkerPrivate->SetDebugger(nullptr); nsTArray> listeners; { MutexAutoLock lock(mMutex); - listeners.AppendElements(mListeners); + listeners = mListeners; } for (size_t index = 0; index < listeners.Length(); ++index) { - listeners[index]->OnUnregister(aDebugger); + listeners[index]->OnUnregister(debugger); } - aDebugger->Disable(); + debugger->Close(); + aWorkerPrivate->SetIsDebuggerRegistered(false); } END_WORKERS_NAMESPACE diff --git a/dom/workers/WorkerDebuggerManager.h b/dom/workers/WorkerDebuggerManager.h index a8e5fc9250ce..2b3d1b5646ab 100644 --- a/dom/workers/WorkerDebuggerManager.h +++ b/dom/workers/WorkerDebuggerManager.h @@ -20,23 +20,19 @@ #define WORKERDEBUGGERMANAGER_CONTRACTID \ "@mozilla.org/dom/workers/workerdebuggermanager;1" -class RegisterDebuggerRunnable; - BEGIN_WORKERS_NAMESPACE class WorkerDebugger; class WorkerDebuggerManager final : public nsIWorkerDebuggerManager { - friend class ::RegisterDebuggerRunnable; - - mozilla::Mutex mMutex; + Mutex mMutex; // Protected by mMutex. nsTArray> mListeners; // Only touched on the main thread. - nsTArray mDebuggers; + nsTArray> mDebuggers; public: static WorkerDebuggerManager* @@ -54,17 +50,17 @@ public: void ClearListeners(); - void RegisterDebugger(WorkerDebugger* aDebugger); + void RegisterDebugger(WorkerPrivate* aWorkerPrivate); - void UnregisterDebugger(WorkerDebugger* aDebugger); + void UnregisterDebugger(WorkerPrivate* aWorkerPrivate); + + void RegisterDebuggerMainThread(WorkerPrivate* aWorkerPrivate, + bool aNotifyListeners); + + void UnregisterDebuggerMainThread(WorkerPrivate* aWorkerPrivate); private: virtual ~WorkerDebuggerManager(); - - void RegisterDebuggerOnMainThread(WorkerDebugger* aDebugger, - bool aHasListeners); - - void UnregisterDebuggerOnMainThread(WorkerDebugger* aDebugger); }; inline nsresult @@ -81,7 +77,7 @@ ClearWorkerDebuggerManagerListeners() } inline nsresult -RegisterWorkerDebugger(WorkerDebugger* aDebugger) +RegisterWorkerDebugger(WorkerPrivate* aWorkerPrivate) { RefPtr manager = WorkerDebuggerManager::GetOrCreateService(); @@ -89,12 +85,12 @@ RegisterWorkerDebugger(WorkerDebugger* aDebugger) return NS_ERROR_FAILURE; } - manager->RegisterDebugger(aDebugger); + manager->RegisterDebugger(aWorkerPrivate); return NS_OK; } inline nsresult -UnregisterWorkerDebugger(WorkerDebugger* aDebugger) +UnregisterWorkerDebugger(WorkerPrivate* aWorkerPrivate) { RefPtr manager = WorkerDebuggerManager::GetOrCreateService(); @@ -102,7 +98,7 @@ UnregisterWorkerDebugger(WorkerDebugger* aDebugger) return NS_ERROR_FAILURE; } - manager->UnregisterDebugger(aDebugger); + manager->UnregisterDebugger(aWorkerPrivate); return NS_OK; } diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 77f7a81a8ef8..382b7fc432c8 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -2314,12 +2314,9 @@ WorkerPrivateParent::EnableDebugger() WorkerPrivate* self = ParentAsWorkerPrivate(); - MOZ_ASSERT(!self->mDebugger); - self->mDebugger = new WorkerDebugger(self); - - if (NS_FAILED(RegisterWorkerDebugger(self->mDebugger))) { + if (NS_FAILED(RegisterWorkerDebugger(self))) { NS_WARNING("Failed to register worker debugger!"); - self->mDebugger = nullptr; + return; } } @@ -2331,15 +2328,9 @@ WorkerPrivateParent::DisableDebugger() WorkerPrivate* self = ParentAsWorkerPrivate(); - if (!self->mDebugger) { - return; - } - - if (NS_FAILED(UnregisterWorkerDebugger(self->mDebugger))) { + if (NS_FAILED(UnregisterWorkerDebugger(self))) { NS_WARNING("Failed to unregister worker debugger!"); } - - self->mDebugger = nullptr; } template @@ -3550,17 +3541,43 @@ WorkerPrivateParent::AssertInnerWindowIsCorrect() const #endif -class ReportDebuggerErrorRunnable final : public nsIRunnable +class PostDebuggerMessageRunnable final : public nsRunnable { - RefPtr mDebugger; + WorkerDebugger *mDebugger; + nsString mMessage; + +public: + PostDebuggerMessageRunnable(WorkerDebugger* aDebugger, + const nsAString& aMessage) + : mDebugger(aDebugger), + mMessage(aMessage) + { + } + +private: + ~PostDebuggerMessageRunnable() + { } + + NS_IMETHOD + Run() override + { + mDebugger->PostMessageToDebuggerOnMainThread(mMessage); + + return NS_OK; + } +}; + +class ReportDebuggerErrorRunnable final : public nsRunnable +{ + WorkerDebugger *mDebugger; nsString mFilename; uint32_t mLineno; nsString mMessage; public: ReportDebuggerErrorRunnable(WorkerDebugger* aDebugger, - const nsAString& aFilename, uint32_t aLineno, - const nsAString& aMessage) + const nsAString& aFilename, uint32_t aLineno, + const nsAString& aMessage) : mDebugger(aDebugger), mFilename(aFilename), mLineno(aLineno), @@ -3568,8 +3585,6 @@ public: { } - NS_DECL_THREADSAFE_ISUPPORTS - private: ~ReportDebuggerErrorRunnable() { } @@ -3583,22 +3598,16 @@ private: } }; -NS_IMPL_ISUPPORTS(ReportDebuggerErrorRunnable, nsIRunnable) - WorkerDebugger::WorkerDebugger(WorkerPrivate* aWorkerPrivate) -: mMutex("WorkerDebugger::mMutex"), - mCondVar(mMutex, "WorkerDebugger::mCondVar"), - mWorkerPrivate(aWorkerPrivate), - mIsEnabled(false), +: mWorkerPrivate(aWorkerPrivate), mIsInitialized(false) { - mWorkerPrivate->AssertIsOnParentThread(); + AssertIsOnMainThread(); } WorkerDebugger::~WorkerDebugger() { MOZ_ASSERT(!mWorkerPrivate); - MOZ_ASSERT(!mIsEnabled); if (!NS_IsMainThread()) { nsCOMPtr mainThread; @@ -3623,8 +3632,6 @@ WorkerDebugger::GetIsClosed(bool* aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - *aResult = !mWorkerPrivate; return NS_OK; } @@ -3634,8 +3641,6 @@ WorkerDebugger::GetIsChrome(bool* aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3649,8 +3654,6 @@ WorkerDebugger::GetIsInitialized(bool* aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3664,8 +3667,6 @@ WorkerDebugger::GetParent(nsIWorkerDebugger** aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3688,8 +3689,6 @@ WorkerDebugger::GetType(uint32_t* aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3703,8 +3702,6 @@ WorkerDebugger::GetUrl(nsAString& aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3718,8 +3715,6 @@ WorkerDebugger::GetWindow(nsIDOMWindow** aResult) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3769,8 +3764,6 @@ WorkerDebugger::Initialize(const nsAString& aURL, JSContext* aCx) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate) { return NS_ERROR_UNEXPECTED; } @@ -3793,8 +3786,6 @@ WorkerDebugger::PostMessageMoz(const nsAString& aMessage, JSContext* aCx) { AssertIsOnMainThread(); - MutexAutoLock lock(mMutex); - if (!mWorkerPrivate || !mIsInitialized) { return NS_ERROR_UNEXPECTED; } @@ -3835,57 +3826,15 @@ WorkerDebugger::RemoveListener(nsIWorkerDebuggerListener* aListener) } void -WorkerDebugger::WaitIsEnabled(bool aIsEnabled) +WorkerDebugger::Close() { - MutexAutoLock lock(mMutex); - - while (mIsEnabled != aIsEnabled) { - mCondVar.Wait(); - } -} - -void -WorkerDebugger::NotifyIsEnabled(bool aIsEnabled) -{ - mMutex.AssertCurrentThreadOwns(); - - MOZ_ASSERT(mIsEnabled != aIsEnabled); - mIsEnabled = aIsEnabled; - mCondVar.Notify(); -} - -void -WorkerDebugger::Enable() -{ - AssertIsOnMainThread(); - - MutexAutoLock lock(mMutex); - - MOZ_ASSERT(mWorkerPrivate); - - NotifyIsEnabled(true); -} - -void -WorkerDebugger::Disable() -{ - AssertIsOnMainThread(); - - MutexAutoLock lock(mMutex); - MOZ_ASSERT(mWorkerPrivate); mWorkerPrivate = nullptr; - { - MutexAutoUnlock unlock(mMutex); - - nsTArray> listeners(mListeners); - for (size_t index = 0; index < listeners.Length(); ++index) { + nsTArray> listeners(mListeners); + for (size_t index = 0; index < listeners.Length(); ++index) { listeners[index]->OnClose(); - } } - - NotifyIsEnabled(false); } void @@ -3893,10 +3842,11 @@ WorkerDebugger::PostMessageToDebugger(const nsAString& aMessage) { mWorkerPrivate->AssertIsOnWorkerThread(); - nsCOMPtr runnable = - NS_NewRunnableMethodWithArg(this, - &WorkerDebugger::PostMessageToDebuggerOnMainThread, nsString(aMessage)); - NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL); + RefPtr runnable = + new PostDebuggerMessageRunnable(this, aMessage); + if (NS_FAILED(NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL))) { + NS_WARNING("Failed to post message to debugger on main thread!"); + } } void @@ -3904,14 +3854,7 @@ WorkerDebugger::PostMessageToDebuggerOnMainThread(const nsAString& aMessage) { AssertIsOnMainThread(); - nsTArray> listeners; - - { - MutexAutoLock lock(mMutex); - - listeners.AppendElements(mListeners); - } - + nsTArray> listeners(mListeners); for (size_t index = 0; index < listeners.Length(); ++index) { listeners[index]->OnMessage(aMessage); } @@ -3924,7 +3867,7 @@ WorkerDebugger::ReportErrorToDebugger(const nsAString& aFilename, { mWorkerPrivate->AssertIsOnWorkerThread(); - nsCOMPtr runnable = + RefPtr runnable = new ReportDebuggerErrorRunnable(this, aFilename, aLineno, aMessage); if (NS_FAILED(NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL))) { NS_WARNING("Failed to report error to debugger on main thread!"); @@ -3938,13 +3881,7 @@ WorkerDebugger::ReportErrorToDebuggerOnMainThread(const nsAString& aFilename, { AssertIsOnMainThread(); - nsTArray> listeners; - { - MutexAutoLock lock(mMutex); - - listeners.AppendElements(mListeners); - } - + nsTArray> listeners(mListeners); for (size_t index = 0; index < listeners.Length(); ++index) { listeners[index]->OnError(aFilename, aLineno, aMessage); } @@ -3961,6 +3898,8 @@ WorkerPrivate::WorkerPrivate(JSContext* aCx, : WorkerPrivateParent(aCx, aParent, aScriptURL, aIsChromeWorker, aWorkerType, aWorkerName, aLoadInfo) + , mDebuggerRegistered(false) + , mDebugger(nullptr) , mJSContext(nullptr) , mPRThread(nullptr) , mDebuggerEventLoopLevel(0) diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index e0a28865e66c..e107bd7f9bc0 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -62,6 +62,7 @@ class PrincipalInfo; struct PRThread; class ReportDebuggerErrorRunnable; +class PostDebuggerMessageRunnable; BEGIN_WORKERS_NAMESPACE @@ -803,35 +804,23 @@ public: class WorkerDebugger : public nsIWorkerDebugger { friend class ::ReportDebuggerErrorRunnable; + friend class ::PostDebuggerMessageRunnable; - mozilla::Mutex mMutex; - mozilla::CondVar mCondVar; - - // Protected by mMutex WorkerPrivate* mWorkerPrivate; - bool mIsEnabled; - - // Only touched on the main thread. bool mIsInitialized; nsTArray> mListeners; public: explicit WorkerDebugger(WorkerPrivate* aWorkerPrivate); - NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_ISUPPORTS NS_DECL_NSIWORKERDEBUGGER void AssertIsOnParentThread(); void - WaitIsEnabled(bool aIsEnabled); - - void - Enable(); - - void - Disable(); + Close(); void PostMessageToDebugger(const nsAString& aMessage); @@ -844,9 +833,6 @@ private: virtual ~WorkerDebugger(); - void - NotifyIsEnabled(bool aIsEnabled); - void PostMessageToDebuggerOnMainThread(const nsAString& aMessage); @@ -876,7 +862,8 @@ class WorkerPrivate : public WorkerPrivateParent NoTimer }; - RefPtr mDebugger; + bool mDebuggerRegistered; + WorkerDebugger* mDebugger; Queue mControlQueue; Queue mDebuggerQueue; @@ -981,14 +968,60 @@ public: static void OverrideLoadInfoLoadGroup(WorkerLoadInfo& aLoadInfo); + bool + IsDebuggerRegistered() + { + AssertIsOnMainThread(); + + // No need to lock here since this is only ever modified by the same thread. + return mDebuggerRegistered; + } + + void + SetIsDebuggerRegistered(bool aDebuggerRegistered) + { + AssertIsOnMainThread(); + + MutexAutoLock lock(mMutex); + + MOZ_ASSERT(mDebuggerRegistered != aDebuggerRegistered); + mDebuggerRegistered = aDebuggerRegistered; + + mCondVar.Notify(); + } + + void + WaitForIsDebuggerRegistered(bool aDebuggerRegistered) + { + AssertIsOnParentThread(); + + MOZ_ASSERT(!NS_IsMainThread()); + + MutexAutoLock lock(mMutex); + + while (mDebuggerRegistered != aDebuggerRegistered) { + mCondVar.Wait(); + } + } + WorkerDebugger* Debugger() const { AssertIsOnMainThread(); + MOZ_ASSERT(mDebugger); return mDebugger; } + void + SetDebugger(WorkerDebugger* aDebugger) + { + AssertIsOnMainThread(); + + MOZ_ASSERT(mDebugger != aDebugger); + mDebugger = aDebugger; + } + void DoRunLoop(JSContext* aCx);