Fix shutdown crash in AudioOutputAuthorizationHandler.

content/browser/renderer_host/media/audio_output_authorization_handler.cc

Index: content/browser/renderer_host/media/audio_output_authorization_handler.cc
diff --git a/content/browser/renderer_host/media/audio_output_authorization_handler.cc b/content/browser/renderer_host/media/audio_output_authorization_handler.cc
index 3bc765fdc86ec5611fa73f2a2e0c39320abae5fb..9c8a1f469fe02de2221829dc8f8eb4056915c584 100644
--- a/content/browser/renderer_host/media/audio_output_authorization_handler.cc
+++ b/content/browser/renderer_host/media/audio_output_authorization_handler.cc
@@ -34,8 +34,8 @@ media::AudioParameters TryToFixAudioParameters(
 }
 
 media::AudioParameters GetDeviceParametersOnDeviceThread(
+    media::AudioManager* audio_manager,
     const std::string& unique_id) {
-  media::AudioManager* audio_manager = media::AudioManager::Get();
   DCHECK(audio_manager->GetTaskRunner()->BelongsToCurrentThread());
 
   return media::AudioDeviceDescription::IsDefaultDevice(unique_id)
@@ -48,11 +48,13 @@ media::AudioParameters GetDeviceParametersOnDeviceThread(
 namespace content {
 
 AudioOutputAuthorizationHandler::AudioOutputAuthorizationHandler(
+    media::AudioManager* audio_manager,
     MediaStreamManager* media_stream_manager,
     int render_process_id,
     const std::string& salt)
-    : media_stream_manager_(media_stream_manager),
-      permission_checker_(new MediaDevicesPermissionChecker()),
+    : audio_manager_(audio_manager),
+      media_stream_manager_(media_stream_manager),
+      permission_checker_(base::MakeUnique<MediaDevicesPermissionChecker>()),

[o1ka, 2016/11/24 14:34:01]

Why?

[Max Morin, 2016/11/24 14:49:03]

This was also a drive-by fix :). Reading on chromium-dev, I see many are quite
hostile to bare new, see also comment at
https://cs.chromium.org/chromium/src/base/memory/ptr_util.h?q=makeunique&sq=p....
In this particular case, it wasn't clear in the old code that
permission_checker_ wasn't a raw pointer, so you'd have to check the h file to
see if the code was correct.

[o1ka, 2016/11/24 15:11:54]

Ok. Though I have not really seen a problem with the code.

       render_process_id_(render_process_id),
       salt_(salt),
       weak_factory_(this) {
@@ -189,8 +191,17 @@ void AudioOutputAuthorizationHandler::GetDeviceParameters(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!raw_device_id.empty());
   base::PostTaskAndReplyWithResult(
-      media::AudioManager::Get()->GetTaskRunner(), FROM_HERE,
-      base::Bind(&GetDeviceParametersOnDeviceThread, raw_device_id),
+      // Note: In the case of a shutdown, the task to delete |audio_manager_| is
+      // posted to the audio thread after the IO thread is stopped, so the task
+      // to delete the audio manager hasn't been posted yet. This means that

[o1ka, 2016/11/24 14:34:01]

task to delete -> destructor?

[Max Morin, 2016/11/24 14:49:03]

Not really? The task will delete the pointer (if you follow the template
nonsense in code search, you end up at
https://cs.chromium.org/chromium/src/base/sequenced_task_runner_helpers.h?sq=...).
This will also run the destructor, but that's not all the task does.

[o1ka, 2016/11/24 15:11:54]

:) Acknowledged.

+      // unretained is safe here.
+      // Mac is a special case. Since the audio manager lives on the UI thread
+      // on Mac, this task is posted to the UI thread, but tasks posted to the
+      // UI task runner will be ignored when the shutdown has progressed to
+      // deleting the audio manager, so this is still safe.
+      audio_manager_->GetTaskRunner(), FROM_HERE,
+      base::Bind(&GetDeviceParametersOnDeviceThread,
+                 base::Unretained(audio_manager_), raw_device_id),
       base::Bind(&AudioOutputAuthorizationHandler::DeviceParametersReceived,
                  weak_factory_.GetWeakPtr(), std::move(cb), false,
                  raw_device_id));