diff --git a/xpcom/threads/StateWatching.h b/xpcom/threads/StateWatching.h index 351f59915b58e..4e04857660470 100644 --- a/xpcom/threads/StateWatching.h +++ b/xpcom/threads/StateWatching.h @@ -59,7 +59,7 @@ namespace mozilla { extern LazyLogModule gStateWatchingLog; -#define WATCH_LOG(x, ...) \ +#define WATCH_LOG(x, ...) \ MOZ_LOG(gStateWatchingLog, LogLevel::Debug, (x, ##__VA_ARGS__)) /* @@ -69,7 +69,10 @@ class AbstractWatcher { public: NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AbstractWatcher) - AbstractWatcher() : mDestroyed(false) {} + AbstractWatcher() + : mDestroyed(false) + { + } bool IsDestroyed() { return mDestroyed; } virtual void Notify() = 0; @@ -89,7 +92,10 @@ class AbstractWatcher class WatchTarget { public: - explicit WatchTarget(const char* aName) : mName(aName) {} + explicit WatchTarget(const char* aName) + : mName(aName) + { + } void AddWatcher(AbstractWatcher* aWatcher) { @@ -141,7 +147,10 @@ class Watchable : public WatchTarget { public: Watchable(const T& aInitialValue, const char* aName) - : WatchTarget(aName), mValue(aInitialValue) {} + : WatchTarget(aName) + , mValue(aInitialValue) + { + } const T& Ref() const { return mValue; } operator const T&() const { return Ref(); } @@ -156,8 +165,8 @@ class Watchable : public WatchTarget } private: - Watchable(const Watchable& aOther); // Not implemented - Watchable& operator=(const Watchable& aOther); // Not implemented + Watchable(const Watchable& aOther) = delete; + Watchable& operator=(const Watchable& aOther) = delete; T mValue; }; @@ -167,25 +176,28 @@ class Watchable : public WatchTarget // // Internally, WatchManager maintains one AbstractWatcher per callback method. // Consumers invoke Watch/Unwatch on a particular (WatchTarget, Callback) tuple. -// This causes an AbstractWatcher for |Callback| to be instantiated if it doesn't -// already exist, and registers it with |WatchTarget|. +// This causes an AbstractWatcher for |Callback| to be instantiated if it +// doesn't already exist, and registers it with |WatchTarget|. // // Using Direct Tasks on the TailDispatcher, WatchManager ensures that we fire -// watch callbacks no more than once per task, once all other operations for that -// task have been completed. +// watch callbacks no more than once per task, once all other operations for +// that task have been completed. // // WatchManager is intended to be declared as a member of |OwnerType| -// objects. Given that, it and its owned objects can't hold permanent strong refs to -// the owner, since that would keep the owner alive indefinitely. Instead, it -// _only_ holds strong refs while waiting for Direct Tasks to fire. This ensures -// that everything is kept alive just long enough. -template +// objects. Given that, it and its owned objects can't hold permanent strong +// refs to the owner, since that would keep the owner alive indefinitely. +// Instead, it _only_ holds strong refs while waiting for Direct Tasks to fire. +// This ensures that everything is kept alive just long enough. +template class WatchManager { public: - typedef void(OwnerType::*CallbackMethod)(); + typedef void (OwnerType::*CallbackMethod)(); explicit WatchManager(OwnerType* aOwner, AbstractThread* aOwnerThread) - : mOwner(aOwner), mOwnerThread(aOwnerThread) {} + : mOwner(aOwner) + , mOwnerThread(aOwnerThread) + { + } ~WatchManager() { @@ -201,8 +213,8 @@ class WatchManager void Shutdown() { MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn()); - for (size_t i = 0; i < mWatchers.Length(); ++i) { - mWatchers[i]->Destroy(); + for (auto& watcher : mWatchers) { + watcher->Destroy(); } mWatchers.Clear(); mOwner = nullptr; @@ -234,8 +246,14 @@ class WatchManager class PerCallbackWatcher : public AbstractWatcher { public: - PerCallbackWatcher(OwnerType* aOwner, AbstractThread* aOwnerThread, CallbackMethod aMethod) - : mOwner(aOwner), mOwnerThread(aOwnerThread), mCallbackMethod(aMethod) {} + PerCallbackWatcher(OwnerType* aOwner, + AbstractThread* aOwnerThread, + CallbackMethod aMethod) + : mOwner(aOwner) + , mOwnerThread(aOwnerThread) + , mCallbackMethod(aMethod) + { + } void Destroy() { @@ -247,19 +265,26 @@ class WatchManager void Notify() override { MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn()); - MOZ_DIAGNOSTIC_ASSERT(mOwner, "mOwner is only null after destruction, " - "at which point we shouldn't be notified"); - if (mStrongRef) { + MOZ_DIAGNOSTIC_ASSERT(mOwner, + "mOwner is only null after destruction, " + "at which point we shouldn't be notified"); + if (mNotificationPending) { // We've already got a notification job in the pipe. return; } - mStrongRef = mOwner; // Hold the owner alive while notifying. + mNotificationPending = true; // Queue up our notification jobs to run in a stable state. mOwnerThread->TailDispatcher().AddDirectTask( - NewRunnableMethod("WatchManager::PerCallbackWatcher::DoNotify", - this, - &PerCallbackWatcher::DoNotify)); + NS_NewRunnableFunction("WatchManager::PerCallbackWatcher::Notify", [ + self = RefPtr(this), + owner = RefPtr(mOwner) + ]() { + if (!self->mDestroyed) { + ((*owner).*(self->mCallbackMethod))(); + } + self->mNotificationPending = false; + })); } bool CallbackMethodIs(CallbackMethod aMethod) const @@ -268,20 +293,10 @@ class WatchManager } private: - ~PerCallbackWatcher() {} - - void DoNotify() - { - MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn()); - MOZ_ASSERT(mStrongRef); - RefPtr ref = mStrongRef.forget(); - if (!mDestroyed) { - ((*ref).*mCallbackMethod)(); - } - } + ~PerCallbackWatcher() = default; OwnerType* mOwner; // Never null. - RefPtr mStrongRef; // Only non-null when notifying. + bool mNotificationPending = false; RefPtr mOwnerThread; CallbackMethod mCallbackMethod; }; @@ -289,9 +304,9 @@ class WatchManager PerCallbackWatcher* GetWatcher(CallbackMethod aMethod) { MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn()); - for (size_t i = 0; i < mWatchers.Length(); ++i) { - if (mWatchers[i]->CallbackMethodIs(aMethod)) { - return mWatchers[i]; + for (auto& watcher : mWatchers) { + if (watcher->CallbackMethodIs(aMethod)) { + return watcher; } } return nullptr; @@ -304,7 +319,10 @@ class WatchManager if (watcher) { return *watcher; } - watcher = mWatchers.AppendElement(new PerCallbackWatcher(mOwner, mOwnerThread, aMethod))->get(); + watcher = mWatchers + .AppendElement(MakeAndAddRef( + mOwner, mOwnerThread, aMethod)) + ->get(); return *watcher; }