Bug 967364: Replace crazy already_AddRefed handling with something reasonable. r=jesup
authorKyle Huey <khuey@kylehuey.com>
Sat, 15 Mar 2014 12:00:15 -0700
changeset 173740 4d78ec7d6f0d6ecbde1d85f6e81e619ae7286971
parent 173739 32f48d6d3389ea5db45cfc6e452ec52595c11a43
child 173741 49dfebffbf8e300cf0c63d3040905add42576274
push id41086
push userkhuey@mozilla.com
push dateSat, 15 Mar 2014 19:01:56 +0000
treeherdermozilla-inbound@89e4000c2f40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs967364
milestone30.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 967364: Replace crazy already_AddRefed handling with something reasonable. r=jesup
dom/media/MediaManager.cpp
dom/media/MediaManager.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -139,34 +139,40 @@ static nsresult ValidateTrackConstraints
                                       aNormalized.mMandatory,
                                       aOutUnknownConstraint);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
 ErrorCallbackRunnable::ErrorCallbackRunnable(
-  already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
-  already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback>& aSuccess,
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback>& aError,
   const nsAString& aErrorMsg, uint64_t aWindowID)
-  : mSuccess(aSuccess)
-  , mError(aError)
-  , mErrorMsg(aErrorMsg)
+  : mErrorMsg(aErrorMsg)
   , mWindowID(aWindowID)
-  , mManager(MediaManager::GetInstance()) {
-  }
+  , mManager(MediaManager::GetInstance())
+{
+  mSuccess.swap(aSuccess);
+  mError.swap(aError);
+}
+
+ErrorCallbackRunnable::~ErrorCallbackRunnable()
+{
+  MOZ_ASSERT(!mSuccess && !mError);
+}
 
 NS_IMETHODIMP
 ErrorCallbackRunnable::Run()
 {
   // Only run if the window is still active.
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
-  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success(mSuccess);
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success = mSuccess.forget();
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error = mError.forget();
 
   if (!(mManager->IsWindowStillActive(mWindowID))) {
     return NS_OK;
   }
   // This is safe since we're on main-thread, and the windowlist can only
   // be invalidated from the main-thread (see OnNavigation)
   error->OnError(mErrorMsg);
   return NS_OK;
@@ -177,46 +183,48 @@ ErrorCallbackRunnable::Run()
  * DOMBlob in the case of {picture:true}, and a MediaStream in the case of
  * {audio:true} or {video:true}. There is a constructor available for each
  * form. Do this only on the main thread.
  */
 class SuccessCallbackRunnable : public nsRunnable
 {
 public:
   SuccessCallbackRunnable(
-    already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
-    already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback>& aSuccess,
+    nsCOMPtr<nsIDOMGetUserMediaErrorCallback>& aError,
     nsIDOMFile* aFile, uint64_t aWindowID)
-    : mSuccess(aSuccess)
-    , mError(aError)
-    , mFile(aFile)
+    : mFile(aFile)
     , mWindowID(aWindowID)
-    , mManager(MediaManager::GetInstance()) {}
+    , mManager(MediaManager::GetInstance())
+  {
+    mSuccess.swap(aSuccess);
+    mError.swap(aError);
+  }
 
   NS_IMETHOD
   Run()
   {
     // Only run if the window is still active.
     NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
-    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success(mSuccess);
-    nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success = mSuccess.forget();
+    nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error = mError.forget();
 
     if (!(mManager->IsWindowStillActive(mWindowID))) {
       return NS_OK;
     }
     // This is safe since we're on main-thread, and the windowlist can only
     // be invalidated from the main-thread (see OnNavigation)
     success->OnSuccess(mFile);
     return NS_OK;
   }
 
 private:
-  already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
-  already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mError;
   nsCOMPtr<nsIDOMFile> mFile;
   uint64_t mWindowID;
   nsRefPtr<MediaManager> mManager; // get ref to this when creating the runnable
 };
 
 /**
  * Invoke the GetUserMediaDevices success callback. Wrapped in a runnable
  * so that it may be called on the main thread. The error callback is also
@@ -465,29 +473,31 @@ public:
  * though that would complicate the constructors some.  Currently the
  * GetUserMedia spec does not allow for more than 2 streams to be obtained in
  * one call, to simplify handling of constraints.
  */
 class GetUserMediaStreamRunnable : public nsRunnable
 {
 public:
   GetUserMediaStreamRunnable(
-    already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
-    already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback>& aSuccess,
+    nsCOMPtr<nsIDOMGetUserMediaErrorCallback>& aError,
     uint64_t aWindowID,
     GetUserMediaCallbackMediaStreamListener* aListener,
     MediaEngineSource* aAudioSource,
     MediaEngineSource* aVideoSource)
-    : mSuccess(aSuccess)
-    , mError(aError)
-    , mAudioSource(aAudioSource)
+    : mAudioSource(aAudioSource)
     , mVideoSource(aVideoSource)
     , mWindowID(aWindowID)
     , mListener(aListener)
-    , mManager(MediaManager::GetInstance()) {}
+    , mManager(MediaManager::GetInstance())
+  {
+    mSuccess.swap(aSuccess);
+    mError.swap(aError);
+  }
 
   ~GetUserMediaStreamRunnable() {}
 
   class TracksAvailableCallback : public DOMMediaStream::OnTracksAvailableCallback
   {
   public:
     TracksAvailableCallback(MediaManager* aManager,
                             nsIDOMGetUserMediaSuccessCallback* aSuccess,
@@ -507,17 +517,17 @@ public:
       aStream->SetLogicalStreamStartTime(aStream->GetStream()->GetCurrentTime());
 
       // This is safe since we're on main-thread, and the windowlist can only
       // be invalidated from the main-thread (see OnNavigation)
       LOG(("Returning success for getUserMedia()"));
       mSuccess->OnSuccess(aStream);
     }
     uint64_t mWindowID;
-    nsRefPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
     nsRefPtr<MediaManager> mManager;
     // Keep the DOMMediaStream alive until the NotifyTracksAvailable callback
     // has fired, otherwise we might immediately destroy the DOMMediaStream and
     // shut down the underlying MediaStream prematurely.
     // This creates a cycle which is broken when NotifyTracksAvailable
     // is fired (which will happen unless the browser shuts down,
     // since we only add this callback when we've successfully appended
     // the desired tracks in the MediaStreamGraph) or when
@@ -622,18 +632,18 @@ public:
 #endif
 
     // We won't need mError now.
     mError = nullptr;
     return NS_OK;
   }
 
 private:
-  nsRefPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
-  nsRefPtr<nsIDOMGetUserMediaErrorCallback> mError;
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mError;
   nsRefPtr<MediaEngineSource> mAudioSource;
   nsRefPtr<MediaEngineSource> mVideoSource;
   uint64_t mWindowID;
   nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener;
   nsRefPtr<MediaManager> mManager; // get ref to this when creating the runnable
 };
 
 /**
@@ -760,37 +770,25 @@ static SourceSet *
  * are sent back to the DOM.
  *
  * Do not run this on the main thread. The success and error callbacks *MUST*
  * be dispatched on the main thread!
  */
 class GetUserMediaRunnable : public nsRunnable
 {
 public:
-  /**
-   * GetUserMediaRunnable is meant for dispatch off main thread, where it would
-   * be illegal to free JS callbacks. Therefore, it uses a rather risky strategy
-   * of holding "already_AddRefed" references to the JS callbacks that must be
-   * transfered and released in consequent dispatches arriving on main thread.
-   *
-   * A special Arm() method must be called before dispatch to enable this
-   * behavior, because GetUserMediaRunnables are held in other circumstances.
-   */
-
   GetUserMediaRunnable(
     const MediaStreamConstraintsInternal& aConstraints,
     already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
     already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
     uint64_t aWindowID, GetUserMediaCallbackMediaStreamListener *aListener,
     MediaEnginePrefs &aPrefs)
     : mConstraints(aConstraints)
-    , mSuccess(nullptr)
-    , mError(nullptr)
-    , mSuccessHolder(aSuccess)
-    , mErrorHolder(aError)
+    , mSuccess(aSuccess)
+    , mError(aError)
     , mWindowID(aWindowID)
     , mListener(aListener)
     , mPrefs(aPrefs)
     , mDeviceChosen(false)
     , mBackend(nullptr)
     , mManager(MediaManager::GetInstance())
   {}
 
@@ -801,49 +799,46 @@ public:
   GetUserMediaRunnable(
     const MediaStreamConstraintsInternal& aConstraints,
     already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
     already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
     uint64_t aWindowID, GetUserMediaCallbackMediaStreamListener *aListener,
     MediaEnginePrefs &aPrefs,
     MediaEngine* aBackend)
     : mConstraints(aConstraints)
-    , mSuccess(nullptr)
-    , mError(nullptr)
-    , mSuccessHolder(aSuccess)
-    , mErrorHolder(aError)
+    , mSuccess(aSuccess)
+    , mError(aError)
     , mWindowID(aWindowID)
     , mListener(aListener)
     , mPrefs(aPrefs)
     , mDeviceChosen(false)
     , mBackend(aBackend)
     , mManager(MediaManager::GetInstance())
   {}
 
   ~GetUserMediaRunnable() {
   }
 
-  /**
-   * Once "armed", GetUserMediaRunnable will leak its JS callbacks if destroyed.
-   * Arm() must be called before the runnable can be dispatched or used, and the
-   * runnable must be dispatched or used once armed. Callbacks get released on
-   * the main thread at the runnable's completion.
-   */
   void
-  Arm() {
-    mSuccess = mSuccessHolder.forget();
-    mError = mErrorHolder.forget();
+  Fail(const nsAString& aMessage) {
+    nsRefPtr<ErrorCallbackRunnable> runnable =
+      new ErrorCallbackRunnable(mSuccess, mError, aMessage, mWindowID);
+    // These should be empty now
+    MOZ_ASSERT(!mSuccess);
+    MOZ_ASSERT(!mError);
+
+    NS_DispatchToMainThread(runnable);
   }
 
   NS_IMETHOD
   Run()
   {
     NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
-    MOZ_ASSERT(mSuccess.mRawPtr);
-    MOZ_ASSERT(mError.mRawPtr);
+    MOZ_ASSERT(mSuccess);
+    MOZ_ASSERT(mError);
 
     MediaEngine* backend = mBackend;
     // Was a backend provided?
     if (!backend) {
       backend = mManager->GetBackend(mWindowID);
     }
 
     // Was a device provided?
@@ -851,19 +846,17 @@ public:
       nsresult rv = SelectDevice(backend);
       if (rv != NS_OK) {
         return rv;
       }
     }
 
     // It is an error if audio or video are requested along with picture.
     if (mConstraints.mPicture && (mConstraints.mAudio || mConstraints.mVideo)) {
-      NS_DispatchToMainThread(new ErrorCallbackRunnable(
-        mSuccess, mError, NS_LITERAL_STRING("NOT_SUPPORTED_ERR"), mWindowID
-      ));
+      Fail(NS_LITERAL_STRING("NOT_SUPPORTED_ERR"));
       return NS_OK;
     }
 
     if (mConstraints.mPicture) {
       ProcessGetUserMediaSnapshot(mVideoDevice->GetSource(), 0);
       return NS_OK;
     }
 
@@ -873,42 +866,43 @@ public:
                         ((mConstraints.mVideo && mVideoDevice) ?
                          mVideoDevice->GetSource() : nullptr));
     return NS_OK;
   }
 
   nsresult
   Denied(const nsAString& aErrorMsg)
   {
-    MOZ_ASSERT(mSuccess.mRawPtr);
-    MOZ_ASSERT(mError.mRawPtr);
+    MOZ_ASSERT(mSuccess);
+    MOZ_ASSERT(mError);
 
     // We add a disabled listener to the StreamListeners array until accepted
     // If this was the only active MediaStream, remove the window from the list.
     if (NS_IsMainThread()) {
       // This is safe since we're on main-thread, and the window can only
       // be invalidated from the main-thread (see OnNavigation)
-      nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success(mSuccess);
-      nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
+      nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success = mSuccess.forget();
+      nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error = mError.forget();
       error->OnError(aErrorMsg);
 
       // Should happen *after* error runs for consistency, but may not matter
       nsRefPtr<MediaManager> manager(MediaManager::GetInstance());
       manager->RemoveFromWindowList(mWindowID, mListener);
     } else {
       // This will re-check the window being alive on main-thread
       // Note: we must remove the listener on MainThread as well
-      NS_DispatchToMainThread(new ErrorCallbackRunnable(
-        mSuccess, mError, aErrorMsg, mWindowID
-      ));
+      Fail(aErrorMsg);
 
       // MUST happen after ErrorCallbackRunnable Run()s, as it checks the active window list
       NS_DispatchToMainThread(new GetUserMediaListenerRemove(mWindowID, mListener));
     }
 
+    MOZ_ASSERT(!mSuccess);
+    MOZ_ASSERT(!mError);
+
     return NS_OK;
   }
 
   nsresult
   SetContraints(const MediaStreamConstraintsInternal& aConstraints)
   {
     mConstraints = aConstraints;
     return NS_OK;
@@ -928,39 +922,37 @@ public:
     mVideoDevice = aVideoDevice;
     mDeviceChosen = true;
     return NS_OK;
   }
 
   nsresult
   SelectDevice(MediaEngine* backend)
   {
-    MOZ_ASSERT(mSuccess.mRawPtr);
-    MOZ_ASSERT(mError.mRawPtr);
+    MOZ_ASSERT(mSuccess);
+    MOZ_ASSERT(mError);
     if (mConstraints.mPicture || mConstraints.mVideo) {
       ScopedDeletePtr<SourceSet> sources (GetSources(backend,
           mConstraints.mVideom, &MediaEngine::EnumerateVideoDevices));
 
       if (!sources->Length()) {
-        NS_DispatchToMainThread(new ErrorCallbackRunnable(
-          mSuccess, mError, NS_LITERAL_STRING("NO_DEVICES_FOUND"), mWindowID));
+        Fail(NS_LITERAL_STRING("NO_DEVICES_FOUND"));
         return NS_ERROR_FAILURE;
       }
       // Pick the first available device.
       mVideoDevice = do_QueryObject((*sources)[0]);
       LOG(("Selected video device"));
     }
 
     if (mConstraints.mAudio) {
       ScopedDeletePtr<SourceSet> sources (GetSources(backend,
           mConstraints.mAudiom, &MediaEngine::EnumerateAudioDevices));
 
       if (!sources->Length()) {
-        NS_DispatchToMainThread(new ErrorCallbackRunnable(
-          mSuccess, mError, NS_LITERAL_STRING("NO_DEVICES_FOUND"), mWindowID));
+        Fail(NS_LITERAL_STRING("NO_DEVICES_FOUND"));
         return NS_ERROR_FAILURE;
       }
       // Pick the first available device.
       mAudioDevice = do_QueryObject((*sources)[0]);
       LOG(("Selected audio device"));
     }
 
     return NS_OK;
@@ -968,86 +960,86 @@ public:
 
   /**
    * Allocates a video or audio device and returns a MediaStream via
    * a GetUserMediaStreamRunnable. Runs off the main thread.
    */
   void
   ProcessGetUserMedia(MediaEngineSource* aAudioSource, MediaEngineSource* aVideoSource)
   {
-    MOZ_ASSERT(mSuccess.mRawPtr);
-    MOZ_ASSERT(mError.mRawPtr);
+    MOZ_ASSERT(mSuccess);
+    MOZ_ASSERT(mError);
     nsresult rv;
     if (aAudioSource) {
       rv = aAudioSource->Allocate(mPrefs);
       if (NS_FAILED(rv)) {
         LOG(("Failed to allocate audiosource %d",rv));
-        NS_DispatchToMainThread(new ErrorCallbackRunnable(
-                                  mSuccess, mError, NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"), mWindowID
-                                                          ));
+        Fail(NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"));
         return;
       }
     }
     if (aVideoSource) {
       rv = aVideoSource->Allocate(mPrefs);
       if (NS_FAILED(rv)) {
         LOG(("Failed to allocate videosource %d\n",rv));
         if (aAudioSource) {
           aAudioSource->Deallocate();
         }
-        NS_DispatchToMainThread(new ErrorCallbackRunnable(
-          mSuccess, mError, NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"), mWindowID
-                                                          ));
+        Fail(NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"));
         return;
       }
     }
 
     NS_DispatchToMainThread(new GetUserMediaStreamRunnable(
       mSuccess, mError, mWindowID, mListener, aAudioSource, aVideoSource
     ));
+
+    MOZ_ASSERT(!mSuccess);
+    MOZ_ASSERT(!mError);
+
     return;
   }
 
   /**
    * Allocates a video device, takes a snapshot and returns a DOMFile via
    * a SuccessRunnable or an error via the ErrorRunnable. Off the main thread.
    */
   void
   ProcessGetUserMediaSnapshot(MediaEngineSource* aSource, int aDuration)
   {
-    MOZ_ASSERT(mSuccess.mRawPtr);
-    MOZ_ASSERT(mError.mRawPtr);
+    MOZ_ASSERT(mSuccess);
+    MOZ_ASSERT(mError);
     nsresult rv = aSource->Allocate(mPrefs);
     if (NS_FAILED(rv)) {
-      NS_DispatchToMainThread(new ErrorCallbackRunnable(
-        mSuccess, mError, NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"), mWindowID
-      ));
+      Fail(NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"));
       return;
     }
 
     /**
      * Display picture capture UI here before calling Snapshot() - Bug 748835.
      */
     nsCOMPtr<nsIDOMFile> file;
     aSource->Snapshot(aDuration, getter_AddRefs(file));
     aSource->Deallocate();
 
     NS_DispatchToMainThread(new SuccessCallbackRunnable(
       mSuccess, mError, file, mWindowID
     ));
+
+    MOZ_ASSERT(!mSuccess);
+    MOZ_ASSERT(!mError);
+
     return;
   }
 
 private:
   MediaStreamConstraintsInternal mConstraints;
 
-  already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
-  already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
-  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccessHolder;
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mErrorHolder;
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mError;
   uint64_t mWindowID;
   nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener;
   nsRefPtr<MediaDevice> mAudioDevice;
   nsRefPtr<MediaDevice> mVideoDevice;
   MediaEnginePrefs mPrefs;
 
   bool mDeviceChosen;
 
@@ -1415,25 +1407,23 @@ MediaManager::GetUserMedia(JSContext* aC
   if (mCameraManager == nullptr) {
     mCameraManager = nsDOMCameraManager::CreateInstance(aWindow);
   }
 #endif
 
 #if defined(ANDROID) && !defined(MOZ_WIDGET_GONK)
   if (c.mPicture) {
     // ShowFilePickerForMimeType() must run on the Main Thread! (on Android)
-    runnable->Arm();
     NS_DispatchToMainThread(runnable);
     return NS_OK;
   }
 #endif
   // XXX No full support for picture in Desktop yet (needs proper UI)
   if (aPrivileged ||
       (c.mFake && !Preferences::GetBool("media.navigator.permission.fake"))) {
-    runnable->Arm();
     mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
   } else {
     // Check if this site has persistent permissions.
     nsresult rv;
     nsCOMPtr<nsIPermissionManager> permManager =
       do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1463,17 +1453,16 @@ MediaManager::GetUserMedia(JSContext* aC
         c.mAudio = false;
         runnable->SetContraints(c);
       }
       if (c.mVideo && videoPerm == nsIPermissionManager::DENY_ACTION) {
         c.mVideo = false;
         runnable->SetContraints(c);
       }
 
-      runnable->Arm();
       if (!c.mAudio && !c.mVideo) {
         return runnable->Denied(NS_LITERAL_STRING("PERMISSION_DENIED"));
       }
 
       return mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
     }
 
     // Ask for user permission, and dispatch runnable (or not) when a response
@@ -1731,17 +1720,16 @@ MediaManager::Observe(nsISupports* aSubj
 
   } else if (!strcmp(aTopic, "getUserMedia:response:allow")) {
     nsString key(aData);
     nsRefPtr<GetUserMediaRunnable> runnable;
     if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
       return NS_OK;
     }
     mActiveCallbacks.Remove(key);
-    runnable->Arm();
 
     if (aSubject) {
       // A particular device or devices were chosen by the user.
       // NOTE: does not allow setting a device to null; assumes nullptr
       nsCOMPtr<nsISupportsArray> array(do_QueryInterface(aSubject));
       MOZ_ASSERT(array);
       uint32_t len = 0;
       array->Count(&len);
@@ -1786,17 +1774,16 @@ MediaManager::Observe(nsISupports* aSubj
     }
 
     nsString key(aData);
     nsRefPtr<GetUserMediaRunnable> runnable;
     if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
       return NS_OK;
     }
     mActiveCallbacks.Remove(key);
-    runnable->Arm();
     runnable->Denied(errorMessage);
     return NS_OK;
 
   } else if (!strcmp(aTopic, "getUserMedia:revoke")) {
     nsresult rv;
     uint64_t windowID = nsString(aData).ToInteger64(&rv);
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     if (NS_SUCCEEDED(rv)) {
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -252,23 +252,25 @@ class GetUserMediaRunnable;
  * Send an error back to content. The error is the form a string.
  * Do this only on the main thread. The success callback is also passed here
  * so it can be released correctly.
  */
 class ErrorCallbackRunnable : public nsRunnable
 {
 public:
   ErrorCallbackRunnable(
-    already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
-    already_AddRefed<nsIDOMGetUserMediaErrorCallback> aError,
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback>& aSuccess,
+    nsCOMPtr<nsIDOMGetUserMediaErrorCallback>& aError,
     const nsAString& aErrorMsg, uint64_t aWindowID);
   NS_IMETHOD Run();
 private:
-  already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
-  already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
+  ~ErrorCallbackRunnable();
+
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccess;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mError;
   const nsString mErrorMsg;
   uint64_t mWindowID;
   nsRefPtr<MediaManager> mManager; // get ref to this when creating the runnable
 };
 
 class ReleaseMediaOperationResource : public nsRunnable
 {
 public:
@@ -317,17 +319,18 @@ public:
   nsresult returnAndCallbackError(nsresult rv, const char* errorLog)
   {
     MM_LOG(("%s , rv=%d", errorLog, rv));
     NS_DispatchToMainThread(new ReleaseMediaOperationResource(mStream.forget(),
           mOnTracksAvailableCallback.forget()));
     nsString log;
 
     log.AssignASCII(errorLog, strlen(errorLog));
-    NS_DispatchToMainThread(new ErrorCallbackRunnable(nullptr, mError.forget(),
+    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success;
+    NS_DispatchToMainThread(new ErrorCallbackRunnable(success, mError,
       log, mWindowID));
     return NS_OK;
   }
 
   NS_IMETHOD
   Run() MOZ_OVERRIDE
   {
     SourceMediaStream *source = mListener->GetSourceStream();
@@ -419,17 +422,17 @@ private:
   MediaOperation mType;
   nsRefPtr<DOMMediaStream> mStream;
   nsAutoPtr<DOMMediaStream::OnTracksAvailableCallback> mOnTracksAvailableCallback;
   nsRefPtr<MediaEngineSource> mAudioSource; // threadsafe
   nsRefPtr<MediaEngineSource> mVideoSource; // threadsafe
   nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener; // threadsafe
   bool mFinish;
   uint64_t mWindowID;
-  nsRefPtr<nsIDOMGetUserMediaErrorCallback> mError;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mError;
 };
 
 typedef nsTArray<nsRefPtr<GetUserMediaCallbackMediaStreamListener> > StreamListeners;
 typedef nsClassHashtable<nsUint64HashKey, StreamListeners> WindowTable;
 
 class MediaDevice : public nsIMediaDevice
 {
 public: