Bug 1358372: Part 2 - Run Windows AudioSessionControl operations on main thread r=jimm a=ritu
☠☠ backed out by aec659045f35 ☠ ☠
authorDavid Parks <dparks@mozilla.com>
Wed, 10 Jan 2018 14:57:19 -0800
changeset 454837 444056446ee9c8c6c3c9a3471e4d68432881aa98
parent 454836 e9139fb87c850c1006ca03b5cb41443c0ed06a8c
child 454838 f8d212c4a686626116e66a38c9775890064966fd
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, ritu
bugs1358372
milestone59.0
Bug 1358372: Part 2 - Run Windows AudioSessionControl operations on main thread r=jimm a=ritu This plays better with sndvol.exe. It reduces the impact of a bug that shows multiple volume sliders for the content processes.
accessible/ipc/win/HandlerProvider.h
widget/windows/AudioSession.cpp
--- a/accessible/ipc/win/HandlerProvider.h
+++ b/accessible/ipc/win/HandlerProvider.h
@@ -10,16 +10,17 @@
 #include "mozilla/a11y/AccessibleHandler.h"
 #include "mozilla/AlreadyAddRefed.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/mscom/IHandlerProvider.h"
 #include "mozilla/mscom/Ptr.h"
 #include "mozilla/mscom/StructStream.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/UniquePtr.h"
+#include "HandlerData.h"
 
 struct NEWEST_IA2_INTERFACE;
 
 namespace mozilla {
 
 namespace mscom {
 
 class StructToStream;
--- a/widget/windows/AudioSession.cpp
+++ b/widget/windows/AudioSession.cpp
@@ -15,16 +15,17 @@
 
 //#include "AudioSession.h"
 #include "nsCOMPtr.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/Mutex.h"
 
 #include <objbase.h>
 
 namespace mozilla {
 namespace widget {
 
 /*
  * To take advantage of what Vista+ have to offer with respect to audio,
@@ -49,16 +50,18 @@ public:
                                       DWORD aChangedChannel,
                                       LPCGUID aContext);
   STDMETHODIMP OnDisplayNameChanged(LPCWSTR aDisplayName, LPCGUID aContext);
   STDMETHODIMP OnGroupingParamChanged(LPCGUID aGroupingParam, LPCGUID aContext);
   STDMETHODIMP OnIconPathChanged(LPCWSTR aIconPath, LPCGUID aContext);
   STDMETHODIMP OnSessionDisconnected(AudioSessionDisconnectReason aReason);
 private:
   nsresult OnSessionDisconnectedInternal();
+  nsresult CommitAudioSessionData();
+
 public:
   STDMETHODIMP OnSimpleVolumeChanged(float aVolume,
                                      BOOL aMute,
                                      LPCGUID aContext);
   STDMETHODIMP OnStateChanged(AudioSessionState aState);
 
   nsresult Start();
   nsresult Stop();
@@ -81,16 +84,18 @@ public:
     AUDIO_SESSION_DISCONNECTED // Audio session disconnected
   };
 protected:
   RefPtr<IAudioSessionControl> mAudioSessionControl;
   nsString mDisplayName;
   nsString mIconPath;
   nsID mSessionGroupingParameter;
   SessionState mState;
+  // Guards the IAudioSessionControl
+  mozilla::Mutex mMutex;
 
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD
 
   static AudioSession* sService;
 };
 
 nsresult
@@ -122,17 +127,18 @@ RecvAudioSessionData(const nsID& aID,
 {
   return AudioSession::GetSingleton()->SetSessionData(aID,
                                                       aSessionName,
                                                       aIconPath);
 }
 
 AudioSession* AudioSession::sService = nullptr;
 
-AudioSession::AudioSession()
+AudioSession::AudioSession() :
+  mMutex("AudioSessionControl")
 {
   mState = UNINITIALIZED;
 }
 
 AudioSession::~AudioSession()
 {
 
 }
@@ -249,58 +255,47 @@ AudioSession::Start()
   hr = device->Activate(IID_IAudioSessionManager,
                         CLSCTX_ALL,
                         nullptr,
                         getter_AddRefs(manager));
   if (FAILED(hr)) {
     return NS_ERROR_FAILURE;
   }
 
+  MutexAutoLock lock(mMutex);
   hr = manager->GetAudioSessionControl(&GUID_NULL,
                                        0,
                                        getter_AddRefs(mAudioSessionControl));
 
   if (FAILED(hr)) {
     return NS_ERROR_FAILURE;
   }
 
-  hr = mAudioSessionControl->SetGroupingParam((LPGUID)&mSessionGroupingParameter,
-                                              nullptr);
-  if (FAILED(hr)) {
-    StopInternal();
-    return NS_ERROR_FAILURE;
-  }
-
-  hr = mAudioSessionControl->SetDisplayName(mDisplayName.get(), nullptr);
-  if (FAILED(hr)) {
-    StopInternal();
-    return NS_ERROR_FAILURE;
-  }
-
-  hr = mAudioSessionControl->SetIconPath(mIconPath.get(), nullptr);
-  if (FAILED(hr)) {
-    StopInternal();
-    return NS_ERROR_FAILURE;
-  }
-
   // Increments refcount of 'this'.
   hr = mAudioSessionControl->RegisterAudioSessionNotification(this);
   if (FAILED(hr)) {
     StopInternal();
     return NS_ERROR_FAILURE;
   }
 
+  nsCOMPtr<nsIRunnable> runnable =
+    NewRunnableMethod("AudioSession::CommitAudioSessionData",
+                      this, &AudioSession::CommitAudioSessionData);
+  NS_DispatchToMainThread(runnable);
+
   mState = STARTED;
 
   return NS_OK;
 }
 
 void
 AudioSession::StopInternal()
 {
+  mMutex.AssertCurrentThreadOwns();
+
   if (mAudioSessionControl &&
       (mState == STARTED || mState == STOPPED)) {
     // Decrement refcount of 'this'
     mAudioSessionControl->UnregisterAudioSessionNotification(this);
   }
   mAudioSessionControl = nullptr;
 }
 
@@ -313,16 +308,17 @@ AudioSession::Stop()
              "State invariants violated");
   SessionState state = mState;
   mState = STOPPED;
 
   {
     RefPtr<AudioSession> kungFuDeathGrip;
     kungFuDeathGrip.swap(sService);
 
+    MutexAutoLock lock(mMutex);
     StopInternal();
   }
 
   if (state != UNINITIALIZED) {
     ::CoUninitialize();
   }
   return NS_OK;
 }
@@ -369,16 +365,49 @@ AudioSession::SetSessionData(const nsID&
   mState = CLONED;
 
   CopynsID(mSessionGroupingParameter, aID);
   mDisplayName = aSessionName;
   mIconPath = aIconPath;
   return NS_OK;
 }
 
+nsresult
+AudioSession::CommitAudioSessionData()
+{
+  MutexAutoLock lock(mMutex);
+
+  if (!mAudioSessionControl) {
+    // Stop() was called before we had a chance to do this.
+    return NS_OK;
+  }
+
+  HRESULT hr =
+    mAudioSessionControl->SetGroupingParam((LPGUID)&mSessionGroupingParameter,
+                                           nullptr);
+  if (FAILED(hr)) {
+    StopInternal();
+    return NS_ERROR_FAILURE;
+  }
+
+  hr = mAudioSessionControl->SetDisplayName(mDisplayName.get(), nullptr);
+  if (FAILED(hr)) {
+    StopInternal();
+    return NS_ERROR_FAILURE;
+  }
+
+  hr = mAudioSessionControl->SetIconPath(mIconPath.get(), nullptr);
+  if (FAILED(hr)) {
+    StopInternal();
+    return NS_ERROR_FAILURE;
+  }
+
+  return NS_OK;
+}
+
 STDMETHODIMP
 AudioSession::OnChannelVolumeChanged(DWORD aChannelCount,
                                      float aChannelVolumeArray[],
                                      DWORD aChangedChannel,
                                      LPCGUID aContext)
 {
   return S_OK; // NOOP
 }
@@ -414,25 +443,30 @@ AudioSession::OnSessionDisconnected(Audi
                       this, &AudioSession::OnSessionDisconnectedInternal);
   NS_DispatchToMainThread(runnable);
   return S_OK;
 }
 
 nsresult
 AudioSession::OnSessionDisconnectedInternal()
 {
-  if (!mAudioSessionControl)
-    return NS_OK;
+  {
+    // We need to release the mutex before we call Start().
+    MutexAutoLock lock(mMutex);
+
+    if (!mAudioSessionControl)
+      return NS_OK;
 
-  // When successful, UnregisterAudioSessionNotification will decrement the
-  // refcount of 'this'.  Start will re-increment it.  In the interim,
-  // we'll need to reference ourselves.
-  RefPtr<AudioSession> kungFuDeathGrip(this);
-  mAudioSessionControl->UnregisterAudioSessionNotification(this);
-  mAudioSessionControl = nullptr;
+    // When successful, UnregisterAudioSessionNotification will decrement the
+    // refcount of 'this'.  Start will re-increment it.  In the interim,
+    // we'll need to reference ourselves.
+    RefPtr<AudioSession> kungFuDeathGrip(this);
+    mAudioSessionControl->UnregisterAudioSessionNotification(this);
+    mAudioSessionControl = nullptr;
+  }
 
   mState = AUDIO_SESSION_DISCONNECTED;
   CoUninitialize();
   Start(); // If it fails there's not much we can do.
   return NS_OK;
 }
 
 STDMETHODIMP