Fix shutdown crash in AudioOutputAuthorizationHandler.

content/browser/renderer_host/media/audio_output_authorization_handler.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/audio_output_authorization_handler.h"
 
 #include "base/task_runner_util.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 16 matching lines @@
   }
 
   // If hardware parameters are still invalid, use dummy parameters with
   // fake audio path and let the client handle the error.
   return params_copy.IsValid()
              ? params_copy
              : media::AudioParameters::UnavailableDeviceParams();
 }
 
 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)
              ? audio_manager->GetDefaultOutputStreamParameters()
              : audio_manager->GetOutputStreamParameters(unique_id);
 }
 
 }  // namespace
 
 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) {
   DCHECK(media_stream_manager_);
 }
 
 void AudioOutputAuthorizationHandler::OverridePermissionsForTesting(
     bool override_value) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   permission_checker_.reset(new MediaDevicesPermissionChecker(override_value));
@@ skipping 116 matching lines @@
   cb.Run(media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND, false,
          media::AudioParameters::UnavailableDeviceParams(), std::string());
 }
 
 void AudioOutputAuthorizationHandler::GetDeviceParameters(
     AuthorizationCompletedCallback cb,
     const std::string& raw_device_id) const {
   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));
 }
 
 void AudioOutputAuthorizationHandler::DeviceParametersReceived(
     AuthorizationCompletedCallback cb,
     bool should_send_id,
     const std::string& raw_device_id,
     const media::AudioParameters& output_params) const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!raw_device_id.empty());
 
   cb.Run(media::OUTPUT_DEVICE_STATUS_OK, should_send_id,
          output_params.IsValid() ? output_params
                                  : TryToFixAudioParameters(output_params),
          raw_device_id);
 }
 
 }  // namespace content