Bug 1280507 - Simplify context loss handler. - r=jrmuizel
☠☠ backed out by bb8c537dc68e ☠ ☠
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 16 Jun 2016 10:01:44 -0700
changeset 302814 9b419a38b9c94db1fbba9e5f3a6477a2089cff01
parent 302813 f99734930e6899a3bc0b69e752c3b773615488ec
child 302815 269a48e6757917fdad3780f1f48dc99f4b2044ca
push id30376
push usercbook@mozilla.com
push dateTue, 28 Jun 2016 14:09:36 +0000
treeherdermozilla-central@e45890951ce7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1280507
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextLossHandler.cpp
dom/canvas/WebGLContextLossHandler.h
--- a/dom/canvas/WebGLContext.cpp
+++ b/dom/canvas/WebGLContext.cpp
@@ -107,16 +107,17 @@ WebGLContextOptions::WebGLContextOptions
 
 WebGLContext::WebGLContext()
     : WebGLContextUnchecked(nullptr)
     , mBufferFetchingIsVerified(false)
     , mBufferFetchingHasPerVertex(false)
     , mMaxFetchedVertices(0)
     , mMaxFetchedInstances(0)
     , mBypassShaderValidation(false)
+    , mContextLossHandler(this)
     , mNeedsFakeNoAlpha(false)
     , mNeedsFakeNoDepth(false)
     , mNeedsFakeNoStencil(false)
     , mNeedsEmulatedLoneDepthStencil(false)
 {
     mGeneration = 0;
     mInvalidated = false;
     mCapturedFrameInvalidated = false;
@@ -168,17 +169,16 @@ WebGLContext::WebGLContext()
 
     if (NS_IsMainThread()) {
         // XXX mtseng: bug 709490, not thread safe
         WebGLMemoryTracker::AddWebGLContext(this);
     }
 
     mAllowContextRestore = true;
     mLastLossWasSimulated = false;
-    mContextLossHandler = new WebGLContextLossHandler(this);
     mContextStatus = ContextNotLost;
     mLoseContextOnMemoryPressure = false;
     mCanLoseContextInForeground = true;
     mRestoreWhenVisible = false;
 
     mAlreadyGeneratedWarnings = 0;
     mAlreadyWarnedAboutFakeVertexAttrib0 = false;
     mAlreadyWarnedAboutViewportLargerThanDest = false;
@@ -204,19 +204,16 @@ WebGLContext::~WebGLContext()
 {
     RemovePostRefreshObserver();
 
     DestroyResourcesAndContext();
     if (NS_IsMainThread()) {
         // XXX mtseng: bug 709490, not thread safe
         WebGLMemoryTracker::RemoveWebGLContext(this);
     }
-
-    mContextLossHandler->DisableTimer();
-    mContextLossHandler = nullptr;
 }
 
 template<typename T>
 static void
 ClearLinkedList(LinkedList<T>& list)
 {
     while (!list.isEmpty()) {
         list.getLast()->DeleteOnce();
@@ -1616,17 +1613,17 @@ WebGLContext::TryToRestoreContext()
         return false;
 
     return true;
 }
 
 void
 WebGLContext::RunContextLossTimer()
 {
-    mContextLossHandler->RunTimer();
+    mContextLossHandler.RunTimer();
 }
 
 class UpdateContextLossStatusTask : public CancelableRunnable
 {
     RefPtr<WebGLContext> mWebGL;
 
 public:
     explicit UpdateContextLossStatusTask(WebGLContext* webgl)
@@ -1758,17 +1755,17 @@ WebGLContext::UpdateContextLossStatus()
             // We might decide this after thinking we'd be OK restoring
             // the context, so downgrade.
             mContextStatus = ContextLost;
             return;
         }
 
         if (!TryToRestoreContext()) {
             // Failed to restore. Try again later.
-            mContextLossHandler->RunTimer();
+            mContextLossHandler.RunTimer();
             return;
         }
 
         // Revival!
         mContextStatus = ContextNotLost;
 
         if (mCanvasElement) {
             nsContentUtils::DispatchTrustedEvent(
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -14,31 +14,31 @@
 #include "mozilla/CheckedInt.h"
 #include "mozilla/dom/HTMLCanvasElement.h"
 #include "mozilla/dom/TypedArray.h"
 #include "mozilla/EnumeratedArray.h"
 #include "mozilla/ErrorResult.h"
 #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"
 #include "nsTArray.h"
 #include "nsWrapperCache.h"
 #include "SurfaceTypes.h"
 #include "ScopedGLHelpers.h"
 #include "TexUnpackBlob.h"
 
 #ifdef XP_MACOSX
 #include "ForceDiscreteGPUHelperCGL.h"
 #endif
 
 // Local
+#include "WebGLContextLossHandler.h"
 #include "WebGLContextUnchecked.h"
 #include "WebGLFormats.h"
 #include "WebGLObjectModel.h"
 #include "WebGLStrongTypes.h"
 #include "WebGLTexture.h"
 
 // Generated
 #include "nsIDOMEventListener.h"
@@ -85,17 +85,16 @@ class nsIDocShell;
 #define LOCAL_GL_UNPACK_FLIP_Y_WEBGL                         0x9240
 #define LOCAL_GL_UNPACK_PREMULTIPLY_ALPHA_WEBGL              0x9241
 
 namespace mozilla {
 class ScopedCopyTexImageSource;
 class ScopedResolveTexturesForDraw;
 class ScopedUnpackReset;
 class WebGLActiveInfo;
-class WebGLContextLossHandler;
 class WebGLBuffer;
 class WebGLExtensionBase;
 class WebGLFramebuffer;
 class WebGLProgram;
 class WebGLQuery;
 class WebGLRenderbuffer;
 class WebGLSampler;
 class WebGLShader;
@@ -181,17 +180,16 @@ public:
 
 class WebGLContext
     : public nsIDOMWebGLRenderingContext
     , public nsICanvasRenderingContextInternal
     , public nsSupportsWeakReference
     , public WebGLContextUnchecked
     , public WebGLRectangleObject
     , public nsWrapperCache
-    , public SupportsWeakPtr<WebGLContext>
 {
     friend class WebGL2Context;
     friend class WebGLContextUserData;
     friend class WebGLExtensionCompressedTextureATC;
     friend class WebGLExtensionCompressedTextureES3;
     friend class WebGLExtensionCompressedTextureETC1;
     friend class WebGLExtensionCompressedTexturePVRTC;
     friend class WebGLExtensionCompressedTextureS3TC;
@@ -217,18 +215,16 @@ class WebGLContext
 
 public:
     WebGLContext();
 
 protected:
     virtual ~WebGLContext();
 
 public:
-    MOZ_DECLARE_WEAKREFERENCE_TYPENAME(WebGLContext)
-
     NS_DECL_CYCLE_COLLECTING_ISUPPORTS
 
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(WebGLContext,
                                                            nsIDOMWebGLRenderingContext)
 
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override = 0;
 
     NS_DECL_NSIDOMWEBGLRENDERINGCONTEXT
@@ -1484,17 +1480,17 @@ protected:
     GLfloat mDepthClearValue;
 
     GLint mViewportX;
     GLint mViewportY;
     GLsizei mViewportWidth;
     GLsizei mViewportHeight;
     bool mAlreadyWarnedAboutViewportLargerThanDest;
 
-    RefPtr<WebGLContextLossHandler> mContextLossHandler;
+    WebGLContextLossHandler mContextLossHandler;
     bool mAllowContextRestore;
     bool mLastLossWasSimulated;
     ContextStatus mContextStatus;
     bool mContextLostErrorSet;
 
     // Used for some hardware (particularly Tegra 2 and 4) that likes to
     // be Flushed while doing hundreds of draw calls.
     int mDrawCallsSinceLastFlush;
--- a/dom/canvas/WebGLContextLossHandler.cpp
+++ b/dom/canvas/WebGLContextLossHandler.cpp
@@ -1,241 +1,103 @@
 /* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #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
-// -------------------------------------------------------------------
+class WatchdogTimerEvent final : public nsITimerCallback
+{
+    const WeakPtr<WebGLContextLossHandler> mHandler;
 
-// 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
-{
 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<nsIEventTarget> 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<nsIRunnable> mRunnable;
 };
 
-NS_IMPL_ISUPPORTS(ContextLossWorkerEventTarget, nsIEventTarget,
-                  nsISupports)
-
-NS_IMETHODIMP
-ContextLossWorkerEventTarget::DispatchFromScript(nsIRunnable* aEvent, uint32_t aFlags)
-{
-    nsCOMPtr<nsIRunnable> event(aEvent);
-    return Dispatch(event.forget(), aFlags);
-}
-
-NS_IMETHODIMP
-ContextLossWorkerEventTarget::Dispatch(already_AddRefed<nsIRunnable> aEvent,
-                                       uint32_t aFlags)
-{
-    nsCOMPtr<nsIRunnable> eventRef(aEvent);
-    RefPtr<ContextLossWorkerRunnable> wrappedEvent =
-        new ContextLossWorkerRunnable(eventRef);
-    return mEventTarget->Dispatch(wrappedEvent, aFlags);
-}
+NS_IMPL_ISUPPORTS(WatchdogTimerEvent, nsITimerCallback, nsISupports)
 
-NS_IMETHODIMP
-ContextLossWorkerEventTarget::DelayedDispatch(already_AddRefed<nsIRunnable>,
-                                              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<nsIThread*> 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<void*>(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<WatchdogTimerEvent> 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<T*>(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) {
+    const bool runOnceMore = mShouldRunTimerAgain;
+    mShouldRunTimerAgain = false;
+
+    mWebGL->UpdateContextLossStatus();
+
+    if (runOnceMore && !mTimerPending) {
         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<nsIEventTarget> target = workerPrivate->GetEventTarget();
-        mTimer->SetTarget(new ContextLossWorkerEventTarget(target));
-        if (!mWorkerHolderAdded) {
-            HoldWorker(workerPrivate);
-            mWorkerHolderAdded = true;
-        }
-    }
-
-    StartTimer(1000);
-
-    mIsTimerRunning = true;
-    mShouldRunTimerAgain = false;
-}
-
-void
-WebGLContextLossHandler::DisableTimer()
-{
-    if (mIsDisabled)
-        return;
-
-    mIsDisabled = true;
-
-    if (mWorkerHolderAdded) {
-        dom::workers::WorkerPrivate* workerPrivate =
-            dom::workers::GetCurrentThreadWorkerPrivate();
-        MOZ_RELEASE_ASSERT(workerPrivate, "GFX: No private worker created.");
-        ReleaseWorker();
-        mWorkerHolderAdded = false;
-    }
-
-    // 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
--- a/dom/canvas/WebGLContextLossHandler.h
+++ b/dom/canvas/WebGLContextLossHandler.h
@@ -1,52 +1,44 @@
 /* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #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;
 
 namespace mozilla {
 class WebGLContext;
 
-class WebGLContextLossHandler : public dom::workers::WorkerHolder
+class WebGLContextLossHandler final : public SupportsWeakPtr<WebGLContextLossHandler>
 {
-    WeakPtr<WebGLContext> mWeakWebGL;
-    nsCOMPtr<nsITimer> mTimer;
-    bool mIsTimerRunning;
-    bool mShouldRunTimerAgain;
-    bool mIsDisabled;
-    bool mWorkerHolderAdded;
+    WebGLContext* const mWebGL;
+    const nsCOMPtr<nsITimer> 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);
+    ~WebGLContextLossHandler();
 
     void RunTimer();
-    void DisableTimer();
-    bool Notify(dom::workers::Status aStatus) override;
 
-protected:
-    ~WebGLContextLossHandler();
-
-    void StartTimer(unsigned long delayMS);
-    static void StaticTimerCallback(nsITimer*, void* tempRefForTimer);
+private:
     void TimerCallback();
 };
 
 } // namespace mozilla
 
-#endif
+#endif // WEBGL_CONTEXT_LOSS_HANDLER_H_