Bug 919244 - Leakproof gUMRunnable + OnNavigation cleanup of MediaManager::mActiveCallbacks. r=jesup
authorJan-Ivar Bruaroey <jib@mozilla.com>
Wed, 08 Jan 2014 16:51:33 -0500
changeset 178660 cad9ce8315a498999ed05ccc635e050b7dd8821b
parent 178659 8342bfb3ef5296de6a11e37299aefa4447a4e70b
child 178661 010f5faed9cf9da0703cc69258aceec07a865b89
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs919244
milestone29.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 919244 - Leakproof gUMRunnable + OnNavigation cleanup of MediaManager::mActiveCallbacks. r=jesup
dom/media/MediaManager.cpp
dom/media/MediaManager.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -749,25 +749,37 @@ 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(aSuccess)
-    , mError(aError)
+    , mSuccess(nullptr)
+    , mError(nullptr)
+    , mSuccessHolder(aSuccess)
+    , mErrorHolder(aError)
     , mWindowID(aWindowID)
     , mListener(aListener)
     , mPrefs(aPrefs)
     , mDeviceChosen(false)
     , mBackendChosen(false)
     , mManager(MediaManager::GetInstance())
   {}
 
@@ -778,37 +790,53 @@ 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(aSuccess)
-    , mError(aError)
+    , mSuccess(nullptr)
+    , mError(nullptr)
+    , mSuccessHolder(aSuccess)
+    , mErrorHolder(aError)
     , mWindowID(aWindowID)
     , mListener(aListener)
     , mPrefs(aPrefs)
     , mDeviceChosen(false)
     , mBackendChosen(true)
     , mBackend(aBackend)
     , mManager(MediaManager::GetInstance())
   {}
 
   ~GetUserMediaRunnable() {
     if (mBackendChosen) {
       delete mBackend;
     }
   }
 
+  /**
+   * 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();
+  }
+
   NS_IMETHOD
   Run()
   {
     NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread");
+    MOZ_ASSERT(mSuccess.mRawPtr);
+    MOZ_ASSERT(mError.mRawPtr);
 
     // Was a backend provided?
     if (!mBackendChosen) {
       mBackend = mManager->GetBackend(mWindowID);
     }
 
     // Was a device provided?
     if (!mDeviceChosen) {
@@ -837,18 +865,21 @@ public:
                         ((mConstraints.mVideo && mVideoDevice) ?
                          mVideoDevice->GetSource() : nullptr));
     return NS_OK;
   }
 
   nsresult
   Denied(const nsAString& aErrorMsg)
   {
-      // We add a disabled listener to the StreamListeners array until accepted
-      // If this was the only active MediaStream, remove the window from the list.
+    MOZ_ASSERT(mSuccess.mRawPtr);
+    MOZ_ASSERT(mError.mRawPtr);
+
+    // 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<nsIDOMGetUserMediaErrorCallback> error(mError);
       error->OnError(aErrorMsg);
 
       // Should happen *after* error runs for consistency, but may not matter
       nsRefPtr<MediaManager> manager(MediaManager::GetInstance());
@@ -881,16 +912,18 @@ public:
     mVideoDevice = aVideoDevice;
     mDeviceChosen = true;
     return NS_OK;
   }
 
   nsresult
   SelectDevice()
   {
+    MOZ_ASSERT(mSuccess.mRawPtr);
+    MOZ_ASSERT(mError.mRawPtr);
     if (mConstraints.mPicture || mConstraints.mVideo) {
       ScopedDeletePtr<SourceSet> sources (GetSources(mBackend,
           mConstraints.mVideom, &MediaEngine::EnumerateVideoDevices));
 
       if (!sources->Length()) {
         NS_DispatchToMainThread(new ErrorCallbackRunnable(
           mSuccess, mError, NS_LITERAL_STRING("NO_DEVICES_FOUND"), mWindowID));
         return NS_ERROR_FAILURE;
@@ -919,16 +952,18 @@ 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);
     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
                                                           ));
@@ -957,16 +992,18 @@ public:
 
   /**
    * 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);
     nsresult rv = aSource->Allocate(mPrefs);
     if (NS_FAILED(rv)) {
       NS_DispatchToMainThread(new ErrorCallbackRunnable(
         mSuccess, mError, NS_LITERAL_STRING("HARDWARE_UNAVAILABLE"), mWindowID
       ));
       return;
     }
 
@@ -983,16 +1020,18 @@ public:
     return;
   }
 
 private:
   MediaStreamConstraintsInternal mConstraints;
 
   already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
   already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
+  nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> mSuccessHolder;
+  nsCOMPtr<nsIDOMGetUserMediaErrorCallback> mErrorHolder;
   uint64_t mWindowID;
   nsRefPtr<GetUserMediaCallbackMediaStreamListener> mListener;
   nsRefPtr<MediaDevice> mAudioDevice;
   nsRefPtr<MediaDevice> mVideoDevice;
   MediaEnginePrefs mPrefs;
 
   bool mDeviceChosen;
   bool mBackendChosen;
@@ -1291,17 +1330,16 @@ MediaManager::GetUserMedia(JSContext* aC
     // Initialize MediaPermissionManager before send out any permission request.
     (void) MediaPermissionManager::GetInstance();
 #endif //MOZ_WIDGET_GONK
   }
 
   // Store the WindowID in a hash table and mark as active. The entry is removed
   // when this window is closed or navigated away from.
   uint64_t windowID = aWindow->WindowID();
-  nsRefPtr<GetUserMediaRunnable> gUMRunnable;
   // This is safe since we're on main-thread, and the windowlist can only
   // be invalidated from the main-thread (see OnNavigation)
   StreamListeners* listeners = GetActiveWindows()->Get(windowID);
   if (!listeners) {
     listeners = new StreamListeners;
     GetActiveWindows()->Put(windowID, listeners);
   }
 
@@ -1335,46 +1373,46 @@ MediaManager::GetUserMedia(JSContext* aC
   // Developer preference for turning off permission check.
   if (Preferences::GetBool("media.navigator.permission.disabled", false)) {
     aPrivileged = true;
   }
   if (!Preferences::GetBool("media.navigator.video.enabled", true)) {
     c.mVideo = false;
   }
 
-  /**
-   * Pass runnables along to GetUserMediaRunnable so it can add the
-   * MediaStreamListener to the runnable list.
-   */
+  // Pass callbacks and MediaStreamListener along to GetUserMediaRunnable.
+  nsRefPtr<GetUserMediaRunnable> runnable;
   if (c.mFake) {
     // Fake stream from default backend.
-    gUMRunnable = new GetUserMediaRunnable(c, onSuccess.forget(),
+    runnable = new GetUserMediaRunnable(c, onSuccess.forget(),
       onError.forget(), windowID, listener, mPrefs, new MediaEngineDefault());
   } else {
     // Stream from default device from WebRTC backend.
-    gUMRunnable = new GetUserMediaRunnable(c, onSuccess.forget(),
+    runnable = new GetUserMediaRunnable(c, onSuccess.forget(),
       onError.forget(), windowID, listener, mPrefs);
   }
 
 #ifdef MOZ_B2G_CAMERA
   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)
-    NS_DispatchToMainThread(gUMRunnable);
+    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) {
-    mMediaThread->Dispatch(gUMRunnable, NS_DISPATCH_NORMAL);
+    runnable->Arm();
+    mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
   } else {
     // Ask for user permission, and dispatch runnable (or not) when a response
     // is received via an observer notification. Each call is paired with its
     // runnable by a GUID.
     nsresult rv;
     nsCOMPtr<nsIUUIDGenerator> uuidgen =
       do_GetService("@mozilla.org/uuid-generator;1", &rv);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1383,19 +1421,28 @@ MediaManager::GetUserMedia(JSContext* aC
     nsID id;
     rv = uuidgen->GenerateUUIDInPlace(&id);
     NS_ENSURE_SUCCESS(rv, rv);
 
     char buffer[NSID_LENGTH];
     id.ToProvidedString(buffer);
     NS_ConvertUTF8toUTF16 callID(buffer);
 
-    // Store the current callback.
-    mActiveCallbacks.Put(callID, gUMRunnable);
+    // Store the current unarmed runnable w/callbacks.
+    mActiveCallbacks.Put(callID, runnable);
 
+    // Add a WindowID cross-reference so OnNavigation can tear things down
+    nsTArray<nsString>* array;
+    if (!mCallIds.Get(windowID, &array)) {
+      array = new nsTArray<nsString>();
+      array->AppendElement(callID);
+      mCallIds.Put(windowID, array);
+    } else {
+      array->AppendElement(callID);
+    }
     nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
     nsRefPtr<GetUserMediaRequest> req = new GetUserMediaRequest(aWindow,
                                                                 callID, c);
     obs->NotifyObservers(req, "getUserMedia:request", nullptr);
   }
 
   return NS_OK;
 }
@@ -1466,16 +1513,24 @@ MediaManager::GetBackend(uint64_t aWindo
 void
 MediaManager::OnNavigation(uint64_t aWindowID)
 {
   NS_ASSERTION(NS_IsMainThread(), "OnNavigation called off main thread");
 
   // Invalidate this window. The runnables check this value before making
   // a call to content.
 
+  nsTArray<nsString>* callIds;
+  if (mCallIds.Get(aWindowID, &callIds)) {
+    for (int i = 0, len = callIds->Length(); i < len; ++i) {
+      mActiveCallbacks.Remove((*callIds)[i]);
+    }
+    mCallIds.Remove(aWindowID);
+  }
+
   // This is safe since we're on main-thread, and the windowlist can only
   // be added to from the main-thread
   StreamListeners* listeners = GetWindowListeners(aWindowID);
   if (!listeners) {
     return;
   }
 
   uint32_t length = listeners->Length();
@@ -1601,57 +1656,57 @@ MediaManager::Observe(nsISupports* aSubj
       prefs->RemoveObserver("media.navigator.load_adapt", this);
     }
 
     // Close off any remaining active windows.
     {
       MutexAutoLock lock(mMutex);
       GetActiveWindows()->Clear();
       mActiveCallbacks.Clear();
+      mCallIds.Clear();
       LOG(("Releasing MediaManager singleton and thread"));
       sSingleton = nullptr;
     }
 
     return NS_OK;
 
   } else if (!strcmp(aTopic, "getUserMedia:response:allow")) {
     nsString key(aData);
-    nsRefPtr<nsRunnable> runnable;
+    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
-      GetUserMediaRunnable* gUMRunnable =
-        static_cast<GetUserMediaRunnable*>(runnable.get());
-
       nsCOMPtr<nsISupportsArray> array(do_QueryInterface(aSubject));
       MOZ_ASSERT(array);
       uint32_t len = 0;
       array->Count(&len);
       MOZ_ASSERT(len);
       if (!len) {
-        gUMRunnable->Denied(NS_LITERAL_STRING("PERMISSION_DENIED")); // neither audio nor video were selected
+        // neither audio nor video were selected
+        runnable->Denied(NS_LITERAL_STRING("PERMISSION_DENIED"));
         return NS_OK;
       }
       for (uint32_t i = 0; i < len; i++) {
         nsCOMPtr<nsISupports> supports;
         array->GetElementAt(i,getter_AddRefs(supports));
         nsCOMPtr<nsIMediaDevice> device(do_QueryInterface(supports));
         MOZ_ASSERT(device); // shouldn't be returning anything else...
         if (device) {
           nsString type;
           device->GetType(type);
           if (type.EqualsLiteral("video")) {
-            gUMRunnable->SetVideoDevice(static_cast<MediaDevice*>(device.get()));
+            runnable->SetVideoDevice(static_cast<MediaDevice*>(device.get()));
           } else if (type.EqualsLiteral("audio")) {
-            gUMRunnable->SetAudioDevice(static_cast<MediaDevice*>(device.get()));
+            runnable->SetAudioDevice(static_cast<MediaDevice*>(device.get()));
           } else {
             NS_WARNING("Unknown device type in getUserMedia");
           }
         }
       }
     }
 
     // Reuse the same thread to save memory.
@@ -1665,25 +1720,23 @@ MediaManager::Observe(nsISupports* aSubj
       nsCOMPtr<nsISupportsString> msg(do_QueryInterface(aSubject));
       MOZ_ASSERT(msg);
       msg->GetData(errorMessage);
       if (errorMessage.IsEmpty())
         errorMessage.Assign(NS_LITERAL_STRING("UNKNOWN_ERROR"));
     }
 
     nsString key(aData);
-    nsRefPtr<nsRunnable> runnable;
+    nsRefPtr<GetUserMediaRunnable> runnable;
     if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
       return NS_OK;
     }
     mActiveCallbacks.Remove(key);
-
-    GetUserMediaRunnable* gUMRunnable =
-      static_cast<GetUserMediaRunnable*>(runnable.get());
-    gUMRunnable->Denied(errorMessage);
+    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)) {
       LOG(("Revoking MediaCapture access for window %llu",windowID));
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -236,16 +236,17 @@ class GetUserMediaNotificationEvent: pub
 };
 
 typedef enum {
   MEDIA_START,
   MEDIA_STOP
 } MediaOperation;
 
 class MediaManager;
+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
 {
@@ -519,17 +520,18 @@ private:
 
   nsresult MediaCaptureWindowStateInternal(nsIDOMWindow* aWindow, bool* aVideo,
                                            bool* aAudio);
 
   void StopMediaStreams();
 
   // ONLY access from MainThread so we don't need to lock
   WindowTable mActiveWindows;
-  nsRefPtrHashtable<nsStringHashKey, nsRunnable> mActiveCallbacks;
+  nsRefPtrHashtable<nsStringHashKey, GetUserMediaRunnable> mActiveCallbacks;
+  nsClassHashtable<nsUint64HashKey, nsTArray<nsString>> mCallIds;
   // Always exists
   nsCOMPtr<nsIThread> mMediaThread;
 
   Mutex mMutex;
   // protected with mMutex:
   MediaEngine* mBackend;
 
   static StaticRefPtr<MediaManager> sSingleton;