From cb4677e8530d1e4f680031ebcbf83c2300f9a64e Mon Sep 17 00:00:00 2001 From: Jeff Gilbert Date: Thu, 16 Jun 2016 10:01:44 -0700 Subject: [PATCH] Bug 1280507 - Simplify context loss handler. - r=jrmuizel Use a self-referential RefPtr instead of manual AddRef/Release. Reuse DisableTimer for when a worker is dead. MozReview-Commit-ID: E1Cv9M7rbe2 --- dom/canvas/WebGLContext.cpp | 9 +- dom/canvas/WebGLContext.h | 8 +- dom/canvas/WebGLContextLossHandler.cpp | 232 +++++-------------------- dom/canvas/WebGLContextLossHandler.h | 34 ++-- 4 files changed, 65 insertions(+), 218 deletions(-) diff --git a/dom/canvas/WebGLContext.cpp b/dom/canvas/WebGLContext.cpp index bd35500b2613..1b4e099678fb 100644 --- a/dom/canvas/WebGLContext.cpp +++ b/dom/canvas/WebGLContext.cpp @@ -113,6 +113,7 @@ WebGLContext::WebGLContext() , mMaxFetchedVertices(0) , mMaxFetchedInstances(0) , mBypassShaderValidation(false) + , mContextLossHandler(this) , mNeedsFakeNoAlpha(false) , mNeedsFakeNoDepth(false) , mNeedsFakeNoStencil(false) @@ -174,7 +175,6 @@ WebGLContext::WebGLContext() mAllowContextRestore = true; mLastLossWasSimulated = false; - mContextLossHandler = new WebGLContextLossHandler(this); mContextStatus = ContextNotLost; mLoseContextOnMemoryPressure = false; mCanLoseContextInForeground = true; @@ -210,9 +210,6 @@ WebGLContext::~WebGLContext() // XXX mtseng: bug 709490, not thread safe WebGLMemoryTracker::RemoveWebGLContext(this); } - - mContextLossHandler->DisableTimer(); - mContextLossHandler = nullptr; } template @@ -1623,7 +1620,7 @@ WebGLContext::TryToRestoreContext() void WebGLContext::RunContextLossTimer() { - mContextLossHandler->RunTimer(); + mContextLossHandler.RunTimer(); } class UpdateContextLossStatusTask : public CancelableRunnable @@ -1765,7 +1762,7 @@ WebGLContext::UpdateContextLossStatus() if (!TryToRestoreContext()) { // Failed to restore. Try again later. - mContextLossHandler->RunTimer(); + mContextLossHandler.RunTimer(); return; } diff --git a/dom/canvas/WebGLContext.h b/dom/canvas/WebGLContext.h index 1eae6397e189..e21662e915c4 100644 --- a/dom/canvas/WebGLContext.h +++ b/dom/canvas/WebGLContext.h @@ -19,7 +19,6 @@ #include "mozilla/gfx/2D.h" #include "mozilla/LinkedList.h" #include "mozilla/UniquePtr.h" -#include "mozilla/WeakPtr.h" #include "nsCycleCollectionNoteChild.h" #include "nsICanvasRenderingContextInternal.h" #include "nsLayoutUtils.h" @@ -34,6 +33,7 @@ #endif // Local +#include "WebGLContextLossHandler.h" #include "WebGLContextUnchecked.h" #include "WebGLFormats.h" #include "WebGLObjectModel.h" @@ -90,7 +90,6 @@ class ScopedCopyTexImageSource; class ScopedResolveTexturesForDraw; class ScopedUnpackReset; class WebGLActiveInfo; -class WebGLContextLossHandler; class WebGLBuffer; class WebGLExtensionBase; class WebGLFramebuffer; @@ -187,7 +186,6 @@ class WebGLContext , public WebGLContextUnchecked , public WebGLRectangleObject , public nsWrapperCache - , public SupportsWeakPtr { friend class WebGL2Context; friend class WebGLContextUserData; @@ -223,8 +221,6 @@ protected: virtual ~WebGLContext(); public: - MOZ_DECLARE_WEAKREFERENCE_TYPENAME(WebGLContext) - NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(WebGLContext, @@ -1491,7 +1487,7 @@ protected: GLsizei mViewportHeight; bool mAlreadyWarnedAboutViewportLargerThanDest; - RefPtr mContextLossHandler; + WebGLContextLossHandler mContextLossHandler; bool mAllowContextRestore; bool mLastLossWasSimulated; ContextStatus mContextStatus; diff --git a/dom/canvas/WebGLContextLossHandler.cpp b/dom/canvas/WebGLContextLossHandler.cpp index 5f1c62e80448..1f50896a5a30 100644 --- a/dom/canvas/WebGLContextLossHandler.cpp +++ b/dom/canvas/WebGLContextLossHandler.cpp @@ -5,237 +5,99 @@ #include "WebGLContextLossHandler.h" +#include "mozilla/DebugOnly.h" +#include "nsISupportsImpl.h" #include "nsITimer.h" #include "nsThreadUtils.h" #include "WebGLContext.h" -#include "mozilla/dom/WorkerPrivate.h" namespace mozilla { -// ------------------------------------------------------------------- -// Begin worker specific code -// ------------------------------------------------------------------- - -// On workers we can only dispatch CancelableRunnables, so we have to wrap the -// timer's EventTarget to use our own cancelable runnable - -class ContextLossWorkerEventTarget final : public nsIEventTarget +class WatchdogTimerEvent final : public nsITimerCallback { + const WeakPtr mHandler; + public: - explicit ContextLossWorkerEventTarget(nsIEventTarget* aEventTarget) - : mEventTarget(aEventTarget) - { - MOZ_ASSERT(aEventTarget); - } + NS_DECL_ISUPPORTS - NS_DECL_NSIEVENTTARGET - NS_DECL_THREADSAFE_ISUPPORTS - -protected: - ~ContextLossWorkerEventTarget() {} + explicit WatchdogTimerEvent(WebGLContextLossHandler* handler) + : mHandler(handler) + { } private: - nsCOMPtr mEventTarget; -}; + virtual ~WatchdogTimerEvent() { } -class ContextLossWorkerRunnable final : public CancelableRunnable -{ -public: - explicit ContextLossWorkerRunnable(nsIRunnable* aRunnable) - : mRunnable(aRunnable) - { + NS_IMETHOD Notify(nsITimer*) override { + if (mHandler) { + mHandler->TimerCallback(); + } + return NS_OK; } - - nsresult Cancel() override; - - NS_FORWARD_NSIRUNNABLE(mRunnable->) - -protected: - ~ContextLossWorkerRunnable() {} - -private: - nsCOMPtr mRunnable; }; -NS_IMPL_ISUPPORTS(ContextLossWorkerEventTarget, nsIEventTarget, - nsISupports) +NS_IMPL_ISUPPORTS(WatchdogTimerEvent, nsITimerCallback, nsISupports) -NS_IMETHODIMP -ContextLossWorkerEventTarget::DispatchFromScript(nsIRunnable* aEvent, uint32_t aFlags) -{ - nsCOMPtr event(aEvent); - return Dispatch(event.forget(), aFlags); -} - -NS_IMETHODIMP -ContextLossWorkerEventTarget::Dispatch(already_AddRefed aEvent, - uint32_t aFlags) -{ - nsCOMPtr eventRef(aEvent); - RefPtr wrappedEvent = - new ContextLossWorkerRunnable(eventRef); - return mEventTarget->Dispatch(wrappedEvent, aFlags); -} - -NS_IMETHODIMP -ContextLossWorkerEventTarget::DelayedDispatch(already_AddRefed, - uint32_t) -{ - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP -ContextLossWorkerEventTarget::IsOnCurrentThread(bool* aResult) -{ - return mEventTarget->IsOnCurrentThread(aResult); -} - -nsresult -ContextLossWorkerRunnable::Cancel() -{ - mRunnable = nullptr; - return NS_OK; -} - -// ------------------------------------------------------------------- -// End worker-specific code -// ------------------------------------------------------------------- +//////////////////////////////////////// WebGLContextLossHandler::WebGLContextLossHandler(WebGLContext* webgl) - : mWeakWebGL(webgl) + : mWebGL(webgl) , mTimer(do_CreateInstance(NS_TIMER_CONTRACTID)) - , mIsTimerRunning(false) + , mTimerPending(false) , mShouldRunTimerAgain(false) - , mIsDisabled(false) - , mWorkerHolderAdded(false) #ifdef DEBUG , mThread(NS_GetCurrentThread()) #endif { + MOZ_ASSERT(mThread); } WebGLContextLossHandler::~WebGLContextLossHandler() { - MOZ_ASSERT(!mIsTimerRunning); + // NS_GetCurrentThread() returns null during shutdown. + const DebugOnly callingThread = NS_GetCurrentThread(); + MOZ_ASSERT(callingThread == mThread || !callingThread); } +//////////////////// + void -WebGLContextLossHandler::StartTimer(unsigned long delayMS) +WebGLContextLossHandler::RunTimer() { - // We can't pass an already_AddRefed through InitWithFuncCallback, so we - // should do the AddRef/Release manually. - this->AddRef(); + MOZ_ASSERT(NS_GetCurrentThread() == mThread); - mTimer->InitWithFuncCallback(StaticTimerCallback, - static_cast(this), - delayMS, - nsITimer::TYPE_ONE_SHOT); + // If the timer was already running, don't restart it here. Instead, + // wait until the previous call is done, then fire it one more time. + // This is also an optimization to prevent unnecessary + // cross-communication between threads. + if (mTimerPending) { + mShouldRunTimerAgain = true; + return; + } + + const RefPtr event = new WatchdogTimerEvent(this); + const uint32_t kDelayMS = 1000; + mTimer->InitWithCallback(event, kDelayMS, nsITimer::TYPE_ONE_SHOT); + + mTimerPending = true; } -/*static*/ void -WebGLContextLossHandler::StaticTimerCallback(nsITimer*, void* voidHandler) -{ - typedef WebGLContextLossHandler T; - T* handler = static_cast(voidHandler); - - handler->TimerCallback(); - - // Release the AddRef from StartTimer. - handler->Release(); -} +//////////////////// void WebGLContextLossHandler::TimerCallback() { MOZ_ASSERT(NS_GetCurrentThread() == mThread); - MOZ_ASSERT(mIsTimerRunning); - mIsTimerRunning = false; - if (mIsDisabled) - return; + mTimerPending = false; - // If we need to run the timer again, restart it immediately. - // Otherwise, the code we call into below might *also* try to - // restart it. - if (mShouldRunTimerAgain) { - RunTimer(); - MOZ_ASSERT(mIsTimerRunning); - } - - if (mWeakWebGL) { - mWeakWebGL->UpdateContextLossStatus(); - } -} - -void -WebGLContextLossHandler::RunTimer() -{ - MOZ_ASSERT(!mIsDisabled); - - // If the timer was already running, don't restart it here. Instead, - // wait until the previous call is done, then fire it one more time. - // This is an optimization to prevent unnecessary - // cross-communication between threads. - if (mIsTimerRunning) { - mShouldRunTimerAgain = true; - return; - } - - if (!NS_IsMainThread()) { - dom::workers::WorkerPrivate* workerPrivate = - dom::workers::GetCurrentThreadWorkerPrivate(); - nsCOMPtr target = workerPrivate->GetEventTarget(); - mTimer->SetTarget(new ContextLossWorkerEventTarget(target)); - if (!mWorkerHolderAdded) { - HoldWorker(workerPrivate); - mWorkerHolderAdded = true; - } - } - - StartTimer(1000); - - mIsTimerRunning = true; + const bool runOnceMore = mShouldRunTimerAgain; mShouldRunTimerAgain = false; -} -void -WebGLContextLossHandler::DisableTimer() -{ - if (mIsDisabled) - return; + mWebGL->UpdateContextLossStatus(); - mIsDisabled = true; - - if (mWorkerHolderAdded) { - dom::workers::WorkerPrivate* workerPrivate = - dom::workers::GetCurrentThreadWorkerPrivate(); - MOZ_RELEASE_ASSERT(workerPrivate, "GFX: No private worker created."); - ReleaseWorker(); - mWorkerHolderAdded = false; + if (runOnceMore && !mTimerPending) { + RunTimer(); } - - // We can't just Cancel() the timer, as sometimes we end up - // receiving a callback after calling Cancel(). This could cause us - // to receive the callback after object destruction. - - // Instead, we let the timer finish, but ignore it. - - if (!mIsTimerRunning) - return; - - mTimer->SetDelay(0); -} - -bool -WebGLContextLossHandler::Notify(dom::workers::Status aStatus) -{ - bool isWorkerRunning = aStatus < dom::workers::Closing; - if (!isWorkerRunning && mIsTimerRunning) { - mIsTimerRunning = false; - this->Release(); - } - - return true; } } // namespace mozilla diff --git a/dom/canvas/WebGLContextLossHandler.h b/dom/canvas/WebGLContextLossHandler.h index b66ac94c667c..096fdc387cd0 100644 --- a/dom/canvas/WebGLContextLossHandler.h +++ b/dom/canvas/WebGLContextLossHandler.h @@ -6,11 +6,8 @@ #ifndef WEBGL_CONTEXT_LOSS_HANDLER_H_ #define WEBGL_CONTEXT_LOSS_HANDLER_H_ -#include "mozilla/DebugOnly.h" #include "mozilla/WeakPtr.h" #include "nsCOMPtr.h" -#include "nsISupportsImpl.h" -#include "WorkerHolder.h" class nsIThread; class nsITimer; @@ -18,35 +15,30 @@ class nsITimer; namespace mozilla { class WebGLContext; -class WebGLContextLossHandler : public dom::workers::WorkerHolder +class WebGLContextLossHandler final : public SupportsWeakPtr { - WeakPtr mWeakWebGL; - nsCOMPtr mTimer; - bool mIsTimerRunning; - bool mShouldRunTimerAgain; - bool mIsDisabled; - bool mWorkerHolderAdded; + WebGLContext* const mWebGL; + const nsCOMPtr mTimer; // If we don't hold a ref to the timer, it will think + bool mTimerPending; // that it's been discarded, and be canceled 'for our + bool mShouldRunTimerAgain; // convenience'. #ifdef DEBUG - nsIThread* mThread; + nsIThread* const mThread; #endif + friend class WatchdogTimerEvent; + public: - NS_INLINE_DECL_REFCOUNTING(WebGLContextLossHandler) + MOZ_DECLARE_WEAKREFERENCE_TYPENAME(WebGLContextLossHandler) explicit WebGLContextLossHandler(WebGLContext* webgl); - - void RunTimer(); - void DisableTimer(); - bool Notify(dom::workers::Status aStatus) override; - -protected: ~WebGLContextLossHandler(); - void StartTimer(unsigned long delayMS); - static void StaticTimerCallback(nsITimer*, void* tempRefForTimer); + void RunTimer(); + +private: void TimerCallback(); }; } // namespace mozilla -#endif +#endif // WEBGL_CONTEXT_LOSS_HANDLER_H_