Use WeakPtrs with AudioDeviceListenerWin

There is a potential race condition on shutdown if there is a device
change as AudioManagerWin is destroyed.

This CL fixes the issue by using WeakPtrs. The use of WeakPtrs is nuanced and intentional here, to accommodate for different threading
situations:
- When the audio service is run out of process, there is no issue,
as there is only one thread, and WeakPtrs can be used without
afterthought.
- When the audio service is run in process, there is a main thread on
which the AudioManagerWin is created & destroyed, and the Audio thread
on which the AudioManagerWin operates. We make sure to allocate and invalidate WeakPtrs on the audio thread, before the WeakPtrFactory
is destroyed on the main thread, to avoid threading issues.

crbug.com/1458623 will aim to remove kAudioServiceOutOfProcess and
simplify this by only running audio out of process.

Bug: 1449929
Change-Id: I1d73e47cd4819dea4e154d8b5d52d59afd270171
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4635284
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1163850}

media/audio/win/audio_manager_win.cc

@@ -115,6 +115,12 @@ AudioManagerWin::AudioManagerWin(std::unique_ptr<AudioThread> audio_thread,
 AudioManagerWin::~AudioManagerWin() = default;
 
 void AudioManagerWin::ShutdownOnAudioThread() {
+  // Prevent pending callbacks from `output_device_listener_` from being run.
+  // TODO(crbug.com/1458623): Remove this call when kAudioServiceOutOfProcess is
+  // removed on Windows; `weak_factory_on_audio_thread_` will be guaranteed to
+  // be destroyed/invalidated on the right thread then.
+  weak_factory_on_audio_thread_.InvalidateWeakPtrs();
+
   AudioManagerBase::ShutdownOnAudioThread();
 
   // Destroy AudioDeviceListenerWin instance on the audio thread because it
@@ -139,11 +145,22 @@ bool AudioManagerWin::HasAudioInputDevices() {
 void AudioManagerWin::InitializeOnAudioThread() {
   DCHECK(GetTaskRunner()->BelongsToCurrentThread());
 
+  // Initialize should only be called once.
+  CHECK(!output_device_listener_);
+
+  // Create a WeakPtr bound to the Audio thread, which will be invalidated
+  // in ShutdownOnAudioThread().
+  weak_this_on_audio_thread_ = weak_factory_on_audio_thread_.GetWeakPtr();
+
   // AudioDeviceListenerWin must be initialized on a COM thread.
+  // Despite `this` owning `output_device_listener_`, we need to bind the
+  // callback to a WeakPtr: NotifyAllOutputDeviceChangeListeners() will be
+  // posted to the audio thread instead of being run synchronously, since we use
+  // BindPostTaskToCurrentDefault().
   output_device_listener_ = std::make_unique<AudioDeviceListenerWin>(
       base::BindPostTaskToCurrentDefault(base::BindRepeating(
           &AudioManagerWin::NotifyAllOutputDeviceChangeListeners,
-          base::Unretained(this))));
+          weak_this_on_audio_thread_)));
 }
 
 void AudioManagerWin::GetAudioDeviceNamesImpl(bool input,

media/audio/win/audio_manager_win.h

@@ -86,6 +86,12 @@ class MEDIA_EXPORT AudioManagerWin : public AudioManagerBase {
 
   // Listen for output device changes.
   std::unique_ptr<AudioDeviceListenerWin> output_device_listener_;
+
+  // Used to invalidate pending `output_device_listener_` callbacks on shutdow.
+  // `audio_weak_factory_` must be invalidated on the audio thread as part of
+  // shutdown, before it is destroyed on whichever thread owns `this`.
+  base::WeakPtr<AudioManagerWin> weak_this_on_audio_thread_;
+  base::WeakPtrFactory<AudioManagerWin> weak_factory_on_audio_thread_{this};
 };
 
 }  // namespace media