Backed out changeset e1b946a41694 (bug 1617283) for audio related crashes and hangs (bug 1755700, bug 1755717). a=backout

build/win32/orderfile.txt

@@ -6180,6 +6180,7 @@ XPCOMService_GetSocketTransportService
 ?BroadcastStringBundle@ContentParent@dom@mozilla@@SAXABVStringBundleDescriptor@23@@Z
 ?Get@SharedStringMap@ipc@dom@mozilla@@QAE_NABV?$nsTString@D@@AAV?$nsTSubstring@_S@@@Z
 ?ReinitForContent@ImageBridgeChild@layers@mozilla@@SA_N$$QAV?$Endpoint@VPImageBridgeChild@layers@mozilla@@@ipc@3@I@Z
+?RecvAudioSessionData@widget@mozilla@@YA?AW4nsresult@@ABUnsID@@ABV?$nsTString@_S@@1@Z
 ?Release@WakeLock@dom@mozilla@@UAGKXZ
 ?GetInstance@PowerManagerService@power@dom@mozilla@@SA?AU?$already_AddRefed@VPowerManagerService@power@dom@mozilla@@@@XZ
 ?RegisterWakeLockObserver@hal@mozilla@@YAXPAV?$Observer@VWakeLockInformation@hal@mozilla@@@2@@Z

build/win64/orderfile.txt

@@ -6464,6 +6464,7 @@ ZN11encoding_rs3mem17utf16_valid_up_to17h89dc8b8f218f8fa1E
 ?BroadcastStringBundle@ContentParent@dom@mozilla@@SAXAEBVStringBundleDescriptor@23@@Z
 ?Get@SharedStringMap@ipc@dom@mozilla@@QEAA_NAEBV?$nsTString@D@@AEAV?$nsTSubstring@_S@@@Z
 ?GetAndResetReleaseFence@RenderCompositor@wr@mozilla@@UEAA?AVFileDescriptor@ipc@3@XZ
+?RecvAudioSessionData@widget@mozilla@@YA?AW4nsresult@@AEBUnsID@@AEBV?$nsTString@_S@@1@Z
 ?Release@WakeLock@dom@mozilla@@UEAAKXZ
 ?GetInstance@PowerManagerService@power@dom@mozilla@@SA?AU?$already_AddRefed@VPowerManagerService@power@dom@mozilla@@@@XZ
 ?RegisterWakeLockObserver@hal@mozilla@@YAXPEAV?$Observer@VWakeLockInformation@hal@mozilla@@@2@@Z

dom/ipc/ContentChild.cpp

@@ -228,6 +228,7 @@
 #  include <process.h>
 #  define getpid _getpid
 #  include "mozilla/WinDllServices.h"
+#  include "mozilla/widget/AudioSession.h"
 #  include "mozilla/widget/WinContentSystemParameters.h"
 #endif
 
@@ -3059,6 +3060,10 @@ void ContentChild::ShutdownInternal() {
     os->NotifyObservers(ToSupports(this), "content-child-shutdown", nullptr);
   }
 
+#if defined(XP_WIN)
+  mozilla::widget::StopAudioSession();
+#endif
+
   GetIPCChannel()->SetAbortOnError(false);
 
   if (mProfilerController) {

toolkit/xre/nsAppRunner.cpp

@@ -5903,10 +5903,6 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) {
   // run!
   rv = XRE_mainRun();
 
-#if defined(XP_WIN)
-  mozilla::widget::StopAudioSession();
-#endif
-
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP
   mozilla::ShutdownEventTracing();
 #endif
@@ -5924,6 +5920,10 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) {
 
   mScopedXPCOM = nullptr;
 
+#if defined(XP_WIN)
+  mozilla::widget::StopAudioSession();
+#endif
+
   // unlock the profile after ScopedXPCOMStartup object (xpcom)
   // has gone out of scope.  see bug #386739 for more details
   mProfileLock->Unlock();

tools/lint/mscom-init.yml

@@ -37,6 +37,7 @@ forbid-mscom-init:
         - toolkit/crashreporter/google-breakpad/src/common/windows/pdb_source_line_writer.cc
         - toolkit/mozapps/defaultagent/main.cpp
         - uriloader/exthandler/win/nsOSHelperAppService.cpp
+        - widget/windows/AudioSession.cpp
         - widget/windows/InkCollector.cpp
         - widget/windows/TaskbarPreview.cpp
         - widget/windows/WinTaskbar.cpp

widget/windows/AudioSession.cpp

@@ -11,6 +11,7 @@
 #include "mozilla/RefPtr.h"
 #include "nsIStringBundle.h"
 
+//#include "AudioSession.h"
 #include "nsCOMPtr.h"
 #include "nsID.h"
 #include "nsServiceManagerUtils.h"
@@ -18,10 +19,11 @@
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "mozilla/Attributes.h"
-#include "mozilla/mscom/Utils.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/WindowsVersion.h"
 
+#include <objbase.h>
+
 namespace mozilla {
 namespace widget {
 
@@ -31,6 +33,10 @@ namespace widget {
  * and implements IAudioSessionEvents (for callbacks from Windows)
  */
 class AudioSession final : public IAudioSessionEvents {
+ private:
+  AudioSession();
+  ~AudioSession();
+
  public:
   static AudioSession* GetSingleton();
 
@@ -47,17 +53,23 @@ class AudioSession final : public IAudioSessionEvents {
   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);
 
-  void Start();
-  void Stop();
+  nsresult Start();
+  nsresult Stop();
   void StopInternal();
-  void InitializeAudioSession();
 
   nsresult GetSessionData(nsID& aID, nsString& aSessionName,
                           nsString& aIconPath);
+
   nsresult SetSessionData(const nsID& aID, const nsString& aSessionName,
                           const nsString& aIconPath);
 
@@ -70,44 +82,39 @@ class AudioSession final : public IAudioSessionEvents {
     AUDIO_SESSION_DISCONNECTED  // Audio session disconnected
   };
 
-  SessionState mState;
-
- private:
-  AudioSession();
-  ~AudioSession();
-  nsresult CommitAudioSessionData();
-
  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;
 };
 
-static AudioSession* sService = nullptr;
+nsresult StartAudioSession() { return AudioSession::GetSingleton()->Start(); }
 
-void StartAudioSession() {
-  AudioSession::GetSingleton()->InitializeAudioSession();
-  NS_DispatchBackgroundTask(NS_NewRunnableFunction(
-      "StartAudioSession",
-      []() -> void { AudioSession::GetSingleton()->Start(); }));
+nsresult StopAudioSession() { return AudioSession::GetSingleton()->Stop(); }
+
+nsresult GetAudioSessionData(nsID& aID, nsString& aSessionName,
+                             nsString& aIconPath) {
+  return AudioSession::GetSingleton()->GetSessionData(aID, aSessionName,
+                                                      aIconPath);
 }
 
-void StopAudioSession() {
-  RefPtr<AudioSession> audioSession;
-  audioSession.swap(sService);
-  if (audioSession) {
-    NS_DispatchBackgroundTask(NS_NewRunnableFunction(
-        "StopAudioSession",
-        [audioSession]() -> void { audioSession->Stop(); }));
-  }
+nsresult RecvAudioSessionData(const nsID& aID, const nsString& aSessionName,
+                              const nsString& aIconPath) {
+  return AudioSession::GetSingleton()->SetSessionData(aID, aSessionName,
+                                                      aIconPath);
 }
 
+AudioSession* AudioSession::sService = nullptr;
+
 AudioSession::AudioSession() : mMutex("AudioSessionControl") {
   mState = UNINITIALIZED;
 }
@@ -115,14 +122,14 @@ AudioSession::AudioSession() : mMutex("AudioSessionControl") {
 AudioSession::~AudioSession() {}
 
 AudioSession* AudioSession::GetSingleton() {
-  if (!sService) {
+  if (!(AudioSession::sService)) {
     RefPtr<AudioSession> service = new AudioSession();
-    service.forget(&sService);
+    service.forget(&AudioSession::sService);
   }
 
   // We don't refcount AudioSession on the Gecko side, we hold one single ref
   // as long as the appshell is running.
-  return sService;
+  return AudioSession::sService;
 }
 
 // It appears Windows will use us on a background thread ...
@@ -141,40 +148,10 @@ AudioSession::QueryInterface(REFIID iid, void** ppv) {
   return E_NOINTERFACE;
 }
 
-void AudioSession::InitializeAudioSession() {
-  // This func must be run on the main thread as
-  // nsStringBundle is not thread safe otherwise
-  MOZ_ASSERT(NS_IsMainThread());
-
-  MOZ_ASSERT(XRE_IsParentProcess(),
-             "Should only get here in a chrome process!");
-
-  if (mState != UNINITIALIZED) return;
-
-  nsCOMPtr<nsIStringBundleService> bundleService =
-      do_GetService(NS_STRINGBUNDLE_CONTRACTID);
-  MOZ_ASSERT(bundleService);
-
-  nsCOMPtr<nsIStringBundle> bundle;
-  bundleService->CreateBundle("chrome://branding/locale/brand.properties",
-                              getter_AddRefs(bundle));
-  MOZ_ASSERT(bundle);
-  bundle->GetStringFromName("brandFullName", mDisplayName);
-
-  wchar_t* buffer;
-  mIconPath.GetMutableData(&buffer, MAX_PATH);
-  ::GetModuleFileNameW(nullptr, buffer, MAX_PATH);
-
-  [[maybe_unused]] nsresult rv =
-      nsID::GenerateUUIDInPlace(mSessionGroupingParameter);
-  MOZ_ASSERT(rv == NS_OK);
-}
-
 // Once we are started Windows will hold a reference to us through our
 // IAudioSessionEvents interface that will keep us alive until the appshell
 // calls Stop.
-void AudioSession::Start() {
-  MOZ_ASSERT(mscom::IsCurrentThreadMTA());
+nsresult AudioSession::Start() {
   MOZ_ASSERT(mState == UNINITIALIZED || mState == CLONED ||
                  mState == AUDIO_SESSION_DISCONNECTED,
              "State invariants violated");
@@ -185,6 +162,41 @@ void AudioSession::Start() {
 
   HRESULT hr;
 
+  // There's a matching CoUninit in Stop() for this tied to a state of
+  // UNINITIALIZED.
+  hr = CoInitialize(nullptr);
+  MOZ_ASSERT(SUCCEEDED(hr),
+             "CoInitialize failure in audio session control, unexpected");
+
+  if (mState == UNINITIALIZED) {
+    mState = FAILED;
+
+    // Content processes should be CLONED
+    if (XRE_IsContentProcess()) {
+      return NS_ERROR_FAILURE;
+    }
+
+    MOZ_ASSERT(XRE_IsParentProcess(),
+               "Should only get here in a chrome process!");
+
+    nsCOMPtr<nsIStringBundleService> bundleService =
+        do_GetService(NS_STRINGBUNDLE_CONTRACTID);
+    NS_ENSURE_TRUE(bundleService, NS_ERROR_FAILURE);
+    nsCOMPtr<nsIStringBundle> bundle;
+    bundleService->CreateBundle("chrome://branding/locale/brand.properties",
+                                getter_AddRefs(bundle));
+    NS_ENSURE_TRUE(bundle, NS_ERROR_FAILURE);
+
+    bundle->GetStringFromName("brandFullName", mDisplayName);
+
+    wchar_t* buffer;
+    mIconPath.GetMutableData(&buffer, MAX_PATH);
+    ::GetModuleFileNameW(nullptr, buffer, MAX_PATH);
+
+    nsresult rv = nsID::GenerateUUIDInPlace(mSessionGroupingParameter);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   mState = FAILED;
 
   MOZ_ASSERT(!mDisplayName.IsEmpty() || !mIconPath.IsEmpty(),
@@ -193,21 +205,21 @@ void AudioSession::Start() {
   RefPtr<IMMDeviceEnumerator> enumerator;
   hr = ::CoCreateInstance(CLSID_MMDeviceEnumerator, nullptr, CLSCTX_ALL,
                           IID_IMMDeviceEnumerator, getter_AddRefs(enumerator));
-  if (FAILED(hr)) return;
+  if (FAILED(hr)) return NS_ERROR_NOT_AVAILABLE;
 
   RefPtr<IMMDevice> device;
   hr = enumerator->GetDefaultAudioEndpoint(
       EDataFlow::eRender, ERole::eMultimedia, getter_AddRefs(device));
   if (FAILED(hr)) {
-    if (hr == E_NOTFOUND) return;
-    return;
+    if (hr == E_NOTFOUND) return NS_ERROR_NOT_AVAILABLE;
+    return NS_ERROR_FAILURE;
   }
 
   RefPtr<IAudioSessionManager> manager;
   hr = device->Activate(IID_IAudioSessionManager, CLSCTX_ALL, nullptr,
                         getter_AddRefs(manager));
   if (FAILED(hr)) {
-    return;
+    return NS_ERROR_FAILURE;
   }
 
   MutexAutoLock lock(mMutex);
@@ -215,18 +227,44 @@ void AudioSession::Start() {
                                        getter_AddRefs(mAudioSessionControl));
 
   if (FAILED(hr)) {
-    return;
+    return NS_ERROR_FAILURE;
   }
 
   // Increments refcount of 'this'.
   hr = mAudioSessionControl->RegisterAudioSessionNotification(this);
   if (FAILED(hr)) {
     StopInternal();
-    return;
+    return NS_ERROR_FAILURE;
   }
 
-  CommitAudioSessionData();
+  nsCOMPtr<nsIRunnable> runnable =
+      NewRunnableMethod("AudioSession::CommitAudioSessionData", this,
+                        &AudioSession::CommitAudioSessionData);
+  NS_DispatchToMainThread(runnable);
+
   mState = STARTED;
+
+  return NS_OK;
+}
+
+void SpawnASCReleaseThread(RefPtr<IAudioSessionControl>&& aASC) {
+  // Fake moving to the other thread by circumventing the ref count.
+  // (RefPtrs don't play well with C++11 lambdas and we don't want to use
+  // XPCOM here.)
+  IAudioSessionControl* rawPtr = nullptr;
+  aASC.forget(&rawPtr);
+  MOZ_ASSERT(rawPtr);
+  PRThread* thread = PR_CreateThread(
+      PR_USER_THREAD,
+      [](void* aRawPtr) {
+        NS_SetCurrentThreadName("AudioASCReleaser");
+        static_cast<IAudioSessionControl*>(aRawPtr)->Release();
+      },
+      rawPtr, PR_PRIORITY_NORMAL, PR_LOCAL_THREAD, PR_UNJOINABLE_THREAD, 0);
+  if (!thread) {
+    // We can't make a thread so just destroy the IAudioSessionControl here.
+    rawPtr->Release();
+  }
 }
 
 void AudioSession::StopInternal() {
@@ -238,25 +276,32 @@ void AudioSession::StopInternal() {
   }
 
   if (mAudioSessionControl) {
-    // Release cannot be called on mAudioSessionControl directly because
-    // RefPtr<T>::operator-> is marked MOZ_NO_ADDREF_RELEASE_ON_RETURN
-    // See Bug 1156084
-    IAudioSessionControl* rawPtr = nullptr;
-    mAudioSessionControl.forget(&rawPtr);
-    MOZ_ASSERT(rawPtr);
-    rawPtr->Release();
+    // Avoid hanging when destroying AudioSessionControl.  We do that by
+    // moving the AudioSessionControl to a worker thread (that we never
+    // 'join') for destruction.
+    SpawnASCReleaseThread(std::move(mAudioSessionControl));
   }
 }
 
-void AudioSession::Stop() {
+nsresult AudioSession::Stop() {
   MOZ_ASSERT(mState == STARTED || mState == UNINITIALIZED ||  // XXXremove this
                  mState == FAILED,
              "State invariants violated");
-  MOZ_ASSERT(mscom::IsCurrentThreadMTA());
-
-  MutexAutoLock lock(mMutex);
+  SessionState state = mState;
   mState = STOPPED;
-  StopInternal();
+
+  {
+    RefPtr<AudioSession> kungFuDeathGrip;
+    kungFuDeathGrip.swap(sService);
+
+    MutexAutoLock lock(mMutex);
+    StopInternal();
+  }
+
+  if (state != UNINITIALIZED) {
+    ::CoUninitialize();
+  }
+  return NS_OK;
 }
 
 void CopynsID(nsID& lhs, const nsID& rhs) {
@@ -297,7 +342,7 @@ nsresult AudioSession::SetSessionData(const nsID& aID,
 }
 
 nsresult AudioSession::CommitAudioSessionData() {
-  mMutex.AssertCurrentThreadOwns();
+  MutexAutoLock lock(mMutex);
 
   if (!mAudioSessionControl) {
     // Stop() was called before we had a chance to do this.
@@ -305,7 +350,7 @@ nsresult AudioSession::CommitAudioSessionData() {
   }
 
   HRESULT hr = mAudioSessionControl->SetGroupingParam(
-      (LPGUID) & (mSessionGroupingParameter), nullptr);
+      (LPGUID)&mSessionGroupingParameter, nullptr);
   if (FAILED(hr)) {
     StopInternal();
     return NS_ERROR_FAILURE;
@@ -350,21 +395,35 @@ AudioSession::OnIconPathChanged(LPCWSTR aIconPath, LPCGUID aContext) {
 
 STDMETHODIMP
 AudioSession::OnSessionDisconnected(AudioSessionDisconnectReason aReason) {
+  // Run our code asynchronously.  Per MSDN we can't do anything interesting
+  // in this callback.
+  nsCOMPtr<nsIRunnable> runnable =
+      NewRunnableMethod("widget::AudioSession::OnSessionDisconnectedInternal",
+                        this, &AudioSession::OnSessionDisconnectedInternal);
+  NS_DispatchToMainThread(runnable);
+  return S_OK;
+}
+
+nsresult AudioSession::OnSessionDisconnectedInternal() {
   // 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);
 
-  // We need to release the mutex before we call Start().
   {
+    // We need to release the mutex before we call Start().
     MutexAutoLock lock(mMutex);
-    if (!mAudioSessionControl) return S_OK;
+
+    if (!mAudioSessionControl) return NS_OK;
+
     mAudioSessionControl->UnregisterAudioSessionNotification(this);
     mAudioSessionControl = nullptr;
-    mState = AUDIO_SESSION_DISCONNECTED;
   }
-  Start();  // If it fails there's not much we can do.
 
-  return S_OK;
+  mState = AUDIO_SESSION_DISCONNECTED;
+  CoUninitialize();
+  Start();  // If it fails there's not much we can do.
+  return NS_OK;
 }
 
 STDMETHODIMP

widget/windows/AudioSession.h

@@ -10,10 +10,19 @@ namespace mozilla {
 namespace widget {
 
 // Start the audio session in the current process
-void StartAudioSession();
+nsresult StartAudioSession();
+
+// Pass the information necessary to start an audio session in another process
+nsresult GetAudioSessionData(nsID& aID, nsString& aSessionName,
+                             nsString& aIconPath);
+
+// Receive the information necessary to start an audio session in a non-chrome
+// process
+nsresult RecvAudioSessionData(const nsID& aID, const nsString& aSessionName,
+                              const nsString& aIconPath);
 
 // Stop the audio session in the current process
-void StopAudioSession();
+nsresult StopAudioSession();
 
 }  // namespace widget
 }  // namespace mozilla

widget/windows/nsAppShell.cpp

@@ -585,6 +585,11 @@ nsresult nsAppShell::Init() {
 
 NS_IMETHODIMP
 nsAppShell::Run(void) {
+  // Content processes initialize audio later through PContent using audio
+  // tray id information pulled from the browser process AudioSession. This
+  // way the two share a single volume control.
+  // Note StopAudioSession() is called from nsAppRunner.cpp after xpcom is torn
+  // down to insure the browser shuts down after child processes.
   if (XRE_IsParentProcess()) {
     mozilla::widget::StartAudioSession();
   }