Bug 805451: Remove lock (mostly) and ensure other singleton refs are MainThread r=derf
authorRandell Jesup <rjesup@jesup.org>
Thu, 25 Oct 2012 20:14:47 -0400
changeset 111490 1e1c57c06adb32de93095d2f6b6b52e9317de25a
parent 111489 4c15027e1f75ce7287f0f56391355ed71af5c908
child 111491 b36eeabe85b53fcca595a20615e2cb57dfc65526
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersderf
bugs805451
milestone19.0a1
Bug 805451: Remove lock (mostly) and ensure other singleton refs are MainThread r=derf
dom/media/MediaManager.cpp
dom/media/MediaManager.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -57,23 +57,22 @@ public:
   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);
 
-    {
-      MutexAutoLock lock(MediaManager::Get()->GetMutex());
-      WindowTable* activeWindows = MediaManager::Get()->GetActiveWindows();
-      if (activeWindows->Get(mWindowID)) {
-        error->OnError(mErrorMsg);
-      }
+    if (!(MediaManager::Get()->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;
   }
 
 private:
   already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
   already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
   const nsString mErrorMsg;
   uint64_t mWindowID;
@@ -101,24 +100,22 @@ public:
   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);
 
-    {
-      MutexAutoLock lock(MediaManager::Get()->GetMutex());
-      WindowTable* activeWindows = MediaManager::Get()->GetActiveWindows();
-      if (activeWindows->Get(mWindowID)) {
-        // XPConnect is a magical unicorn.
-        success->OnSuccess(mFile);
-      }
+    if (!(MediaManager::Get()->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<nsIDOMFile> mFile;
   uint64_t mWindowID;
@@ -247,29 +244,30 @@ public:
     nsRefPtr<nsDOMLocalMediaStream> stream;
     uint32_t hints = (mAudioSource ? nsDOMMediaStream::HINT_CONTENTS_AUDIO : 0);
     hints |= (mVideoSource ? nsDOMMediaStream::HINT_CONTENTS_VIDEO : 0);
 
     stream = nsDOMLocalMediaStream::CreateInputStream(hints);
 
     nsPIDOMWindow *window = static_cast<nsPIDOMWindow*>
       (nsGlobalWindow::GetInnerWindowWithId(mWindowID));
-    WindowTable* activeWindows = MediaManager::Get()->GetActiveWindows();
     {
-      MutexAutoLock lock(MediaManager::Get()->GetMutex());
-
       if (!stream) {
-        if (activeWindows->Get(mWindowID)) {
-          nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
-          LOG(("Returning error for getUserMedia() - no stream"));
-          error->OnError(NS_LITERAL_STRING("NO_STREAM"));
+        if (!(MediaManager::Get()->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)
+        nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
+        LOG(("Returning error for getUserMedia() - no stream"));
+        error->OnError(NS_LITERAL_STRING("NO_STREAM"));
         return NS_OK;
       }
     }
+
     if (window && window->GetExtantDoc()) {
       stream->CombineWithPrincipal(window->GetExtantDoc()->NodePrincipal());
     }
 
     // Ensure there's a thread for gum to proxy to off main thread
     nsIThread *mediaThread = MediaManager::GetThread();
 
     // Add our listener. We'll call Start() on the source when get a callback
@@ -290,23 +288,23 @@ public:
       new MediaOperationRunnable(MEDIA_START, stream,
                                  mAudioSource, mVideoSource));
     mediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
 
     // We're in the main thread, so no worries here either.
     nsCOMPtr<nsIDOMGetUserMediaSuccessCallback> success(mSuccess);
     nsCOMPtr<nsIDOMGetUserMediaErrorCallback> error(mError);
 
-    {
-      MutexAutoLock lock(MediaManager::Get()->GetMutex());
-      if (activeWindows->Get(mWindowID)) {
-        LOG(("Returning success for getUserMedia()"));
-        success->OnSuccess(static_cast<nsIDOMLocalMediaStream*>(stream));
-      }
+    if (!(MediaManager::Get()->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)
+    LOG(("Returning success for getUserMedia()"));
+    success->OnSuccess(static_cast<nsIDOMLocalMediaStream*>(stream));
 
     return NS_OK;
   }
 
 private:
   already_AddRefed<nsIDOMGetUserMediaSuccessCallback> mSuccess;
   already_AddRefed<nsIDOMGetUserMediaErrorCallback> mError;
   nsRefPtr<MediaEngineSource> mAudioSource;
@@ -430,19 +428,22 @@ public:
                         mVideo ? mVideoDevice->GetSource() : nullptr);
     return NS_OK;
   }
 
   nsresult
   Denied()
   {
     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(NS_LITERAL_STRING("PERMISSION_DENIED"));
     } else {
+      // This will re-check the window being alive on main-thread
       NS_DispatchToMainThread(new ErrorCallbackRunnable(
         mSuccess, mError, NS_LITERAL_STRING("PERMISSION_DENIED"), mWindowID
       ));
     }
 
     return NS_OK;
   }
 
@@ -784,72 +785,69 @@ MediaManager::GetUserMedia(bool aPrivile
     // Hack: should init singleton earlier unless it's expensive (mem or CPU)
     (void) MediaManager::Get();
   }
 
   // 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;
-  {
-    MutexAutoLock lock(mMutex);
-    StreamListeners* listeners = mActiveWindows.Get(windowID);
-    if (!listeners) {
-      listeners = new StreamListeners;
-      mActiveWindows.Put(windowID, listeners);
-    }
-
-    // Developer preference for turning off permission check.
-    if (Preferences::GetBool("media.navigator.permission.disabled", false)) {
-      aPrivileged = true;
-    }
+  // 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);
+  }
 
-    /**
-     * Pass runnables along to GetUserMediaRunnable so it can add the
-     * MediaStreamListener to the runnable list. The last argument can
-     * optionally be a MediaDevice object, which should provided if one was
-     * selected by the user via the UI, or was provided by privileged code
-     * via the device: attribute via nsIMediaStreamOptions.
-     *
-     * If a fake stream was requested, we force the use of the default backend.
-     */
-    if (fake) {
-      // Fake stream from default backend.
-      gUMRunnable = new GetUserMediaRunnable(
-        audio, video, onSuccess.forget(), onError.forget(), listeners,
-        windowID, new MediaEngineDefault()
-                                             );
-    } else if (audiodevice || videodevice) {
-      // Stream from provided device.
-      gUMRunnable = new GetUserMediaRunnable(
-        audio, video, picture, onSuccess.forget(), onError.forget(), listeners,
-        windowID,
-        static_cast<MediaDevice*>(audiodevice.get()),
-        static_cast<MediaDevice*>(videodevice.get())
-                                             );
-    } else {
-      // Stream from default device from WebRTC backend.
-      gUMRunnable = new GetUserMediaRunnable(
-        audio, video, picture, onSuccess.forget(), onError.forget(), listeners,
-        windowID
-                                             );
-    }
+  // Developer preference for turning off permission check.
+  if (Preferences::GetBool("media.navigator.permission.disabled", false)) {
+    aPrivileged = true;
   }
 
+  /**
+   * Pass runnables along to GetUserMediaRunnable so it can add the
+   * MediaStreamListener to the runnable list. The last argument can
+   * optionally be a MediaDevice object, which should provided if one was
+   * selected by the user via the UI, or was provided by privileged code
+   * via the device: attribute via nsIMediaStreamOptions.
+   *
+   * If a fake stream was requested, we force the use of the default backend.
+   */
+  if (fake) {
+    // Fake stream from default backend.
+    gUMRunnable = new GetUserMediaRunnable(
+      audio, video, onSuccess.forget(), onError.forget(), listeners,
+      windowID, new MediaEngineDefault()
+                                           );
+  } else if (audiodevice || videodevice) {
+    // Stream from provided device.
+    gUMRunnable = new GetUserMediaRunnable(
+      audio, video, picture, onSuccess.forget(), onError.forget(), listeners,
+      windowID,
+      static_cast<MediaDevice*>(audiodevice.get()),
+      static_cast<MediaDevice*>(videodevice.get())
+                                           );
+  } else {
+    // Stream from default device from WebRTC backend.
+    gUMRunnable = new GetUserMediaRunnable(
+      audio, video, picture, onSuccess.forget(), onError.forget(), listeners,
+      windowID
+                                           );
+  }
 
 #ifdef ANDROID
   if (picture) {
     // ShowFilePickerForMimeType() must run on the Main Thread! (on Android)
     NS_DispatchToMainThread(gUMRunnable);
   }
   // XXX No support for Audio or Video in Android yet
 #else
   // XXX No full support for picture in Desktop yet (needs proper UI)
   if (aPrivileged || fake) {
-    (void) MediaManager::GetThread();
     mMediaThread->Dispatch(gUMRunnable, 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);
@@ -860,20 +858,17 @@ MediaManager::GetUserMedia(bool aPrivile
     rv = uuidgen->GenerateUUIDInPlace(&id);
     NS_ENSURE_SUCCESS(rv, rv);
 
     char buffer[NSID_LENGTH];
     id.ToProvidedString(buffer);
     NS_ConvertUTF8toUTF16 callID(buffer);
 
     // Store the current callback.
-    {
-      MutexAutoLock lock(mMutex);
-      mActiveCallbacks.Put(callID, gUMRunnable);
-    }
+    mActiveCallbacks.Put(callID, gUMRunnable);
 
     // Construct JSON structure with both the windowID and the callID.
     nsAutoString data;
     data.Append(NS_LITERAL_STRING("{\"windowID\":"));
 
     // Convert window ID to string.
     char windowBuffer[32];
     PR_snprintf(windowBuffer, 32, "%llu", aWindow->GetOuterWindow()->WindowID());
@@ -917,56 +912,52 @@ MediaManager::GetUserMediaDevices(nsPIDO
   return NS_OK;
 }
 
 MediaEngine*
 MediaManager::GetBackend()
 {
   // Plugin backends as appropriate. The default engine also currently
   // includes picture support for Android.
+  // This IS called off main-thread.
+  MutexAutoLock lock(mMutex);
   if (!mBackend) {
 #if defined(MOZ_WEBRTC)
     mBackend = new MediaEngineWebRTC();
 #else
     mBackend = new MediaEngineDefault();
 #endif
   }
-
   return mBackend;
 }
 
-WindowTable*
-MediaManager::GetActiveWindows()
-{
-  return &mActiveWindows;
-}
-
 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.
-  {
-    MutexAutoLock lock(mMutex);
-    StreamListeners* listeners = mActiveWindows.Get(aWindowID);
-    if (!listeners) {
-      return;
-    }
+
+  // This is safe since we're on main-thread, and the windowlist can only
+  // be added to from the main-thread (see OnNavigation)
+  StreamListeners* listeners = GetActiveWindows()->Get(aWindowID);
+  if (!listeners) {
+    return;
+  }
 
-    uint32_t length = listeners->Length();
-    for (uint32_t i = 0; i < length; i++) {
-      nsRefPtr<GetUserMediaCallbackMediaStreamListener> listener =
-        listeners->ElementAt(i);
-      listener->Invalidate();
-      listener = nullptr;
-    }
-    listeners->Clear();
+  uint32_t length = listeners->Length();
+  for (uint32_t i = 0; i < length; i++) {
+    nsRefPtr<GetUserMediaCallbackMediaStreamListener> listener =
+      listeners->ElementAt(i);
+    listener->Invalidate();
+  }
+  listeners->Clear();
 
-    mActiveWindows.Remove(aWindowID);
-  }
+  GetActiveWindows()->Remove(aWindowID);
 }
 
 nsresult
 MediaManager::Observe(nsISupports* aSubject, const char* aTopic,
   const PRUnichar* aData)
 {
   NS_ASSERTION(NS_IsMainThread(), "Observer invoked off the main thread");
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
@@ -974,36 +965,31 @@ MediaManager::Observe(nsISupports* aSubj
   if (!strcmp(aTopic, "xpcom-shutdown")) {
     obs->RemoveObserver(this, "xpcom-shutdown");
     obs->RemoveObserver(this, "getUserMedia:response:allow");
     obs->RemoveObserver(this, "getUserMedia:response:deny");
 
     // Close off any remaining active windows.
     {
       MutexAutoLock lock(mMutex);
-      mActiveWindows.Clear();
+      GetActiveWindows()->Clear();
       mActiveCallbacks.Clear();
       sSingleton = nullptr;
     }
 
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "getUserMedia:response:allow")) {
     nsString key(aData);
     nsRefPtr<nsRunnable> runnable;
-    {
-      MutexAutoLock lock(mMutex);
-      if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
-        return NS_OK;
-      }
+    if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
+      return NS_OK;
     }
-
-    // Reuse the same thread to save memory.
-    (void) MediaManager::GetThread();
+    mActiveCallbacks.Remove(key);
 
     if (aSubject) {
       // A particular device was chosen by the user.
       // NOTE: does not allow setting a device to null; assumes nullptr
       nsCOMPtr<nsIMediaDevice> device = do_QueryInterface(aSubject);
       if (device) {
         GetUserMediaRunnable* gUMRunnable =
           static_cast<GetUserMediaRunnable*>(runnable.get());
@@ -1014,35 +1000,31 @@ MediaManager::Observe(nsISupports* aSubj
         } else if (type.EqualsLiteral("audio")) {
           gUMRunnable->SetAudioDevice(static_cast<MediaDevice*>(device.get()));
         } else {
           NS_WARNING("Unknown device type in getUserMedia");
         }
       }
     }
 
+    // Reuse the same thread to save memory.
     mMediaThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
-    {
-      MutexAutoLock lock(mMutex);
-      mActiveCallbacks.Remove(key);
-    }
     return NS_OK;
   }
 
   if (!strcmp(aTopic, "getUserMedia:response:deny")) {
     nsString key(aData);
     nsRefPtr<nsRunnable> runnable;
-    {
-      MutexAutoLock lock(mMutex);
-      if (mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
-        GetUserMediaRunnable* gUMRunnable =
-          static_cast<GetUserMediaRunnable*>(runnable.get());
-        gUMRunnable->Denied();
-        mActiveCallbacks.Remove(key);
-      }
+    if (!mActiveCallbacks.Get(key, getter_AddRefs(runnable))) {
+      return NS_OK;
     }
+    mActiveCallbacks.Remove(key);
+
+    GetUserMediaRunnable* gUMRunnable =
+      static_cast<GetUserMediaRunnable*>(runnable.get());
+    gUMRunnable->Denied();
     return NS_OK;
   }
 
   return NS_OK;
 }
 
 } // namespace mozilla
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -270,68 +270,75 @@ private:
 
 class MediaManager MOZ_FINAL : public nsIObserver
 {
 public:
   static MediaManager* Get() {
     if (!sSingleton) {
       sSingleton = new MediaManager();
 
+      NS_NewThread(getter_AddRefs(sSingleton->mMediaThread));
+      MM_LOG(("New Media thread for gum"));
+
       NS_ASSERTION(NS_IsMainThread(), "Only create MediaManager on main thread");
       nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
       obs->AddObserver(sSingleton, "xpcom-shutdown", false);
       obs->AddObserver(sSingleton, "getUserMedia:response:allow", false);
       obs->AddObserver(sSingleton, "getUserMedia:response:deny", false);
     }
     return sSingleton;
   }
-  static Mutex& GetMutex() {
-    return Get()->mMutex;
-  }
   static nsIThread* GetThread() {
-    MutexAutoLock lock(Get()->mMutex); // only need to call Get() once
-    if (!sSingleton->mMediaThread) {
-      NS_NewThread(getter_AddRefs(sSingleton->mMediaThread));
-      MM_LOG(("New Media thread for gum"));
-    }
-    return sSingleton->mMediaThread;
+    return Get()->mMediaThread;
   }
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   MediaEngine* GetBackend();
-  WindowTable* GetActiveWindows();
+  bool IsWindowStillActive(uint64_t aWindowId) {
+    NS_ASSERTION(NS_IsMainThread(), "Only access windowlist on main thread");
+
+    return !!mActiveWindows.Get(aWindowId);
+  }
 
   nsresult GetUserMedia(bool aPrivileged, nsPIDOMWindow* aWindow,
     nsIMediaStreamOptions* aParams,
     nsIDOMGetUserMediaSuccessCallback* onSuccess,
     nsIDOMGetUserMediaErrorCallback* onError);
   nsresult GetUserMediaDevices(nsPIDOMWindow* aWindow,
     nsIGetUserMediaDevicesSuccessCallback* onSuccess,
     nsIDOMGetUserMediaErrorCallback* onError);
   void OnNavigation(uint64_t aWindowID);
 
 private:
+  WindowTable *GetActiveWindows() {
+    NS_ASSERTION(NS_IsMainThread(), "Only access windowlist on main thread");
+    return &mActiveWindows;
+  };
+
   // Make private because we want only one instance of this class
   MediaManager()
-  : mMutex("mozilla::MediaManager")
-  , mBackend(nullptr)
-  , mMediaThread(nullptr) {
+  : mMediaThread(nullptr)
+  , mMutex("mozilla::MediaManager")
+  , mBackend(nullptr) {
     mActiveWindows.Init();
     mActiveCallbacks.Init();
   };
 
   ~MediaManager() {
     delete mBackend;
   };
 
+  // ONLY access from MainThread so we don't need to lock
+  WindowTable mActiveWindows;
+  nsRefPtrHashtable<nsStringHashKey, nsRunnable> mActiveCallbacks;
+  // Always exists
+  nsCOMPtr<nsIThread> mMediaThread;
+
   Mutex mMutex;
   // protected with mMutex:
   MediaEngine* mBackend;
-  nsCOMPtr<nsIThread> mMediaThread;
-  WindowTable mActiveWindows;
-  nsRefPtrHashtable<nsStringHashKey, nsRunnable> mActiveCallbacks;
 
   static nsRefPtr<MediaManager> sSingleton;
 };
 
 } // namespace mozilla