Bug 1299515 - Remove special handling for releasing callbacks on main thread. r?jib draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Wed, 15 Nov 2017 18:40:35 +0100
changeset 748935 b2114cc1d2d3681515ddcbd46c7e3bca8eec5dbe
parent 748934 1b7c4ae3c7629fbeefc50340e1f43f36fcfb40ad
child 748936 786b9ce7d8df4d6b996feb8f1be6ae6fa7475be7
push id97281
push userbmo:apehrson@mozilla.com
push dateTue, 30 Jan 2018 18:29:03 +0000
reviewersjib
bugs1299515
milestone60.0a1
Bug 1299515 - Remove special handling for releasing callbacks on main thread. r?jib MozReview-Commit-ID: 1JtVdbkL1DK
dom/media/MediaManager.cpp
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -630,66 +630,54 @@ private:
   // The task will reset this to false. MainThread only.
   bool mChromeNotificationTaskPosted;
 
   nsTArray<RefPtr<SourceListener>> mInactiveListeners;
   nsTArray<RefPtr<SourceListener>> mActiveListeners;
 };
 
 /**
- * Send an error back to content.
- * Do this only on the main thread. The onSuccess callback is also passed here
- * so it can be released correctly.
+ * Send an error back to content. Do this only on the main thread.
  */
-template<class SuccessCallbackType>
 class ErrorCallbackRunnable : public Runnable
 {
 public:
-  ErrorCallbackRunnable(nsCOMPtr<SuccessCallbackType>&& aOnSuccess,
-                        nsCOMPtr<nsIDOMGetUserMediaErrorCallback>&& aOnFailure,
+  ErrorCallbackRunnable(const nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback>& aOnFailure,
                         MediaMgrError& aError,
                         uint64_t aWindowID)
     : Runnable("ErrorCallbackRunnable")
+    , mOnFailure(aOnFailure)
     , mError(&aError)
     , mWindowID(aWindowID)
     , mManager(MediaManager::GetInstance())
   {
-    mOnSuccess.swap(aOnSuccess);
-    mOnFailure.swap(aOnFailure);
   }
 
   NS_IMETHOD
   Run() override
   {
     MOZ_ASSERT(NS_IsMainThread());
 
-    nsCOMPtr<SuccessCallbackType> onSuccess = mOnSuccess.forget();
-    nsCOMPtr<nsIDOMGetUserMediaErrorCallback> onFailure = mOnFailure.forget();
-
     // Only run if the window is still active.
     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)
     if (auto* window = nsGlobalWindowInner::GetInnerWindowWithId(mWindowID)) {
       RefPtr<MediaStreamError> error =
         new MediaStreamError(window->AsInner(), *mError);
-      onFailure->OnError(error);
+      mOnFailure->OnError(error);
     }
     return NS_OK;
   }
 private:
-  ~ErrorCallbackRunnable()
-  {
-    MOZ_ASSERT(!mOnSuccess && !mOnFailure);
-  }
-
-  nsCOMPtr<SuccessCallbackType> mOnSuccess;
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mOnFailure;
+  ~ErrorCallbackRunnable() override = default;
+
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback> mOnFailure;
   RefPtr<MediaMgrError> mError;
   uint64_t mWindowID;
   RefPtr<MediaManager> mManager; // get ref to this when creating the runnable
 };
 
 /**
  * nsIMediaDevice implementation.
  */
@@ -968,48 +956,48 @@ NS_IMPL_CYCLE_COLLECTION_INHERITED(FakeT
  * 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 Runnable
 {
 public:
   GetUserMediaStreamRunnable(
-    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback>& aOnSuccess,
-    nsCOMPtr<nsIDOMGetUserMediaErrorCallback>& aOnFailure,
+    const nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback>& aOnSuccess,
+    const nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback>& aOnFailure,
     uint64_t aWindowID,
     GetUserMediaWindowListener* aWindowListener,
     SourceListener* aSourceListener,
     const ipc::PrincipalInfo& aPrincipalInfo,
     const MediaStreamConstraints& aConstraints,
     AudioDevice* aAudioDevice,
     VideoDevice* aVideoDevice,
     PeerIdentity* aPeerIdentity)
     : Runnable("GetUserMediaStreamRunnable")
+    , mOnSuccess(aOnSuccess)
+    , mOnFailure(aOnFailure)
     , mConstraints(aConstraints)
     , mAudioDevice(aAudioDevice)
     , mVideoDevice(aVideoDevice)
     , mWindowID(aWindowID)
     , mWindowListener(aWindowListener)
     , mSourceListener(aSourceListener)
     , mPrincipalInfo(aPrincipalInfo)
     , mPeerIdentity(aPeerIdentity)
     , mManager(MediaManager::GetInstance())
   {
-    mOnSuccess.swap(aOnSuccess);
-    mOnFailure.swap(aOnFailure);
   }
 
   ~GetUserMediaStreamRunnable() {}
 
   class TracksAvailableCallback : public OnTracksAvailableCallback
   {
   public:
     TracksAvailableCallback(MediaManager* aManager,
-                            already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aSuccess,
+                            const nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback>& aSuccess,
                             uint64_t aWindowID,
                             DOMMediaStream* aStream)
       : mWindowID(aWindowID), mOnSuccess(aSuccess), mManager(aManager),
         mStream(aStream) {}
     void NotifyTracksAvailable(DOMMediaStream* aStream) override
     {
       // We're in the main thread, so no worries here.
       if (!(mManager->IsWindowStillActive(mWindowID))) {
@@ -1021,17 +1009,17 @@ public:
       aStream->SetLogicalStreamStartTime(aStream->GetPlaybackStream()->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()"));
       mOnSuccess->OnSuccess(aStream);
     }
     uint64_t mWindowID;
-    nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
+    nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
     RefPtr<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
@@ -1201,42 +1189,41 @@ public:
         RefPtr<MediaStreamTrack> track =
           domStream->CreateDOMTrack(kVideoTrack, MediaSegment::VIDEO, videoSource,
                                     GetInvariant(mConstraints.mVideo));
         domStream->AddTrackInternal(track);
       }
     }
 
     if (!domStream || !stream || sHasShutdown) {
-      nsCOMPtr<nsIDOMGetUserMediaErrorCallback> onFailure = mOnFailure.forget();
       LOG(("Returning error for getUserMedia() - no stream"));
 
       if (auto* window = nsGlobalWindowInner::GetInnerWindowWithId(mWindowID)) {
         RefPtr<MediaStreamError> error = new MediaStreamError(window->AsInner(),
             NS_LITERAL_STRING("InternalError"),
             sHasShutdown ? NS_LITERAL_STRING("In shutdown") :
                           NS_LITERAL_STRING("No stream."));
-        onFailure->OnError(error);
+        mOnFailure->OnError(error);
       }
       return NS_OK;
     }
 
     // Activate our source listener. We'll call Start() on the source when we
     // get a callback that the MediaStream has started consuming. The listener
     // is freed when the page is invalidated (on navigation or close).
     mWindowListener->Activate(mSourceListener, stream, mAudioDevice, mVideoDevice);
 
     // Note: includes JS callbacks; must be released on MainThread
     typedef Refcountable<UniquePtr<TracksAvailableCallback>> Callback;
     nsMainThreadPtrHandle<Callback> callback(
       new nsMainThreadPtrHolder<Callback>(
         "GetUserMediaStreamRunnable::TracksAvailableCallbackMainThreadHolder",
         MakeAndAddRef<Callback>(
           new TracksAvailableCallback(mManager,
-                                      mOnSuccess.forget(),
+                                      mOnSuccess,
                                       mWindowID,
                                       domStream))));
 
     // Dispatch to the media thread to ask it to start the sources,
     // because that can take a while.
     // Pass ownership of domStream through the lambda to the nested chrome
     // notification lambda to ensure it's kept alive until that lambda runs or is discarded.
     RefPtr<GetUserMediaStreamRunnable> self = this;
@@ -1269,23 +1256,18 @@ public:
           nsString log;
           log.AssignASCII("Starting video failed");
           error = new MediaMgrError(NS_LITERAL_STRING("InternalError"), log);
         }
       }
 
       if (error) {
         // Dispatch the error callback on main thread.
-        nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> onSuccess;
-        NS_DispatchToMainThread(do_AddRef(
-          new ErrorCallbackRunnable<nsIDOMGetUserMediaSuccessCallback>(
-            Move(onSuccess), Move(self->mOnFailure), *error, self->mWindowID)));
-
-        // This should be empty now
-        MOZ_ASSERT(!self->mOnFailure);
+        NS_DispatchToMainThread(MakeAndAddRef<ErrorCallbackRunnable>(
+          self->mOnFailure, *error, self->mWindowID));
         return NS_OK;
       }
 
       // Start() queued the tracks to be added synchronously to avoid races
       source->FinishAddTracks();
 
       source->SetPullEnabled(true);
       source->AdvanceKnownTracksTime(STREAM_TIME_MAX);
@@ -1326,18 +1308,18 @@ public:
       // deviceIds to persistent, in case they're not already. Fire'n'forget.
       RefPtr<Pledge<nsCString>> p =
         media::GetPrincipalKey(mPrincipalInfo, true);
     }
     return NS_OK;
   }
 
 private:
-  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mOnFailure;
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback> mOnFailure;
   MediaStreamConstraints mConstraints;
   RefPtr<AudioDevice> mAudioDevice;
   RefPtr<VideoDevice> mVideoDevice;
   uint64_t mWindowID;
   RefPtr<GetUserMediaWindowListener> mWindowListener;
   RefPtr<SourceListener> mSourceListener;
   ipc::PrincipalInfo mPrincipalInfo;
   RefPtr<PeerIdentity> mPeerIdentity;
@@ -1466,18 +1448,18 @@ MediaManager::SelectSettings(
  * Do not run this on the main thread. The success and error callbacks *MUST*
  * be dispatched on the main thread!
  */
 class GetUserMediaTask : public Runnable
 {
 public:
   GetUserMediaTask(
     const MediaStreamConstraints& aConstraints,
-    already_AddRefed<nsIDOMGetUserMediaSuccessCallback> aOnSuccess,
-    already_AddRefed<nsIDOMGetUserMediaErrorCallback> aOnFailure,
+    const nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback>& aOnSuccess,
+    const nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback>& aOnFailure,
     uint64_t aWindowID,
     GetUserMediaWindowListener* aWindowListener,
     SourceListener* aSourceListener,
     MediaEnginePrefs& aPrefs,
     const ipc::PrincipalInfo& aPrincipalInfo,
     bool aIsChrome,
     MediaManager::SourceSet* aSourceSet)
     : Runnable("GetUserMediaTask")
@@ -1498,21 +1480,18 @@ public:
   ~GetUserMediaTask() {
   }
 
   void
   Fail(const nsAString& aName,
        const nsAString& aMessage = EmptyString(),
        const nsAString& aConstraint = EmptyString()) {
     RefPtr<MediaMgrError> error = new MediaMgrError(aName, aMessage, aConstraint);
-    auto errorRunnable = MakeRefPtr<ErrorCallbackRunnable<nsIDOMGetUserMediaSuccessCallback>>(
-        Move(mOnSuccess), Move(mOnFailure), *error, mWindowID);
-    // These should be empty now
-    MOZ_ASSERT(!mOnSuccess);
-    MOZ_ASSERT(!mOnFailure);
+    auto errorRunnable = MakeRefPtr<ErrorCallbackRunnable>(
+        mOnFailure, *error, mWindowID);
 
     NS_DispatchToMainThread(errorRunnable.forget());
     // Do after ErrorCallbackRunnable Run()s, as it checks active window list
     NS_DispatchToMainThread(NewRunnableMethod<RefPtr<SourceListener>>(
       "GetUserMediaWindowListener::Remove",
       mWindowListener,
       &GetUserMediaWindowListener::Remove,
       mSourceListener));
@@ -1590,52 +1569,41 @@ public:
     }
 
     NS_DispatchToMainThread(do_AddRef(
         new GetUserMediaStreamRunnable(mOnSuccess, mOnFailure, mWindowID,
                                        mWindowListener, mSourceListener,
                                        mPrincipalInfo, mConstraints,
                                        mAudioDevice, mVideoDevice,
                                        peerIdentity)));
-    MOZ_ASSERT(!mOnSuccess);
-    MOZ_ASSERT(!mOnFailure);
     return NS_OK;
   }
 
   nsresult
   Denied(const nsAString& aName,
          const nsAString& aMessage = EmptyString())
   {
     MOZ_ASSERT(mOnSuccess);
     MOZ_ASSERT(mOnFailure);
 
     // 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> onSuccess = mOnSuccess.forget();
-      nsCOMPtr<nsIDOMGetUserMediaErrorCallback> onFailure = mOnFailure.forget();
-
       if (auto* window = nsGlobalWindowInner::GetInnerWindowWithId(mWindowID)) {
         RefPtr<MediaStreamError> error = new MediaStreamError(window->AsInner(),
                                                               aName, aMessage);
-        onFailure->OnError(error);
+        mOnFailure->OnError(error);
       }
       // Should happen *after* error runs for consistency, but may not matter
       mWindowListener->Remove(mSourceListener);
     } else {
       // This will re-check the window being alive on main-thread
-      // and remove the listener on MainThread as well
       Fail(aName, aMessage);
     }
 
-    MOZ_ASSERT(!mOnSuccess);
-    MOZ_ASSERT(!mOnFailure);
-
     return NS_OK;
   }
 
   nsresult
   SetContraints(const MediaStreamConstraints& aConstraints)
   {
     mConstraints = aConstraints;
     return NS_OK;
@@ -1667,18 +1635,18 @@ public:
   GetWindowID()
   {
     return mWindowID;
   }
 
 private:
   MediaStreamConstraints mConstraints;
 
-  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mOnFailure;
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback> mOnSuccess;
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback> mOnFailure;
   uint64_t mWindowID;
   RefPtr<GetUserMediaWindowListener> mWindowListener;
   RefPtr<SourceListener> mSourceListener;
   RefPtr<AudioDevice> mAudioDevice;
   RefPtr<VideoDevice> mVideoDevice;
   MediaEnginePrefs mPrefs;
   ipc::PrincipalInfo mPrincipalInfo;
   bool mIsChrome;
@@ -2225,18 +2193,22 @@ MediaManager::GetUserMedia(nsPIDOMWindow
                            nsIDOMGetUserMediaSuccessCallback* aOnSuccess,
                            nsIDOMGetUserMediaErrorCallback* aOnFailure,
                            dom::CallerType aCallerType)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aWindow);
   MOZ_ASSERT(aOnFailure);
   MOZ_ASSERT(aOnSuccess);
-  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> onSuccess(aOnSuccess);
-  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> onFailure(aOnFailure);
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaSuccessCallback> onSuccess(
+      new nsMainThreadPtrHolder<nsIDOMGetUserMediaSuccessCallback>(
+          "GetUserMedia::SuccessCallback", aOnSuccess));
+  nsMainThreadPtrHandle<nsIDOMGetUserMediaErrorCallback> onFailure(
+      new nsMainThreadPtrHolder<nsIDOMGetUserMediaErrorCallback>(
+          "GetUserMedia::FailureCallback", aOnFailure));
   uint64_t windowID = aWindow->WindowID();
 
   MediaStreamConstraints c(aConstraintsPassedIn); // use a modifiable copy
 
   // Do all the validation we can while we're sync (to return an
   // already-rejected promise on failure).
 
   if (!IsOn(c.mVideo) && !IsOn(c.mAudio)) {
@@ -2625,18 +2597,18 @@ MediaManager::GetUserMedia(nsPIDOMWindow
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return;
           }
         }
       }
 
       // Pass callbacks and listeners along to GetUserMediaTask.
       RefPtr<GetUserMediaTask> task (new GetUserMediaTask(c,
-                                                          onSuccess.forget(),
-                                                          onFailure.forget(),
+                                                          onSuccess,
+                                                          onFailure,
                                                           windowID,
                                                           windowListener,
                                                           sourceListener,
                                                           prefs,
                                                           principalInfo,
                                                           isChrome,
                                                           devices->release()));
       // Store the task w/callbacks.