 Chromium Code Reviews
 Chromium Code Reviews Issue 2784433002:
  Ensures that audio tasks cannot run after AudioManager is deleted.  (Closed)
    
  
    Issue 2784433002:
  Ensures that audio tasks cannot run after AudioManager is deleted.  (Closed) 
  | Index: content/browser/browser_main_loop.cc | 
| diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc | 
| index 5e96a960c6ce476aa5682fbf51b0b8bcb8069f30..68acd42072e90767de07e2a8c217590a08274107 100644 | 
| --- a/content/browser/browser_main_loop.cc | 
| +++ b/content/browser/browser_main_loop.cc | 
| @@ -93,6 +93,7 @@ | 
| #include "device/battery/battery_status_service.h" | 
| #include "device/gamepad/gamepad_service.h" | 
| #include "gpu/vulkan/features.h" | 
| +#include "media/audio/audio_manager.h" | 
| #include "media/audio/audio_system_impl.h" | 
| #include "media/base/media.h" | 
| #include "media/base/user_input_monitor.h" | 
| @@ -1393,6 +1394,10 @@ void BrowserMainLoop::ShutdownThreadsAndCleanUp() { | 
| TRACE_EVENT0("shutdown", "BrowserMainLoop::Subsystem:DeleteDataSources"); | 
| URLDataManager::DeleteDataSources(); | 
| } | 
| + { | 
| + TRACE_EVENT0("shutdown", "BrowserMainLoop::Subsystem:AudioMan"); | 
| + DestroyAudioManager(); | 
| + } | 
| if (parts_) { | 
| TRACE_EVENT0("shutdown", "BrowserMainLoop::Subsystem:PostDestroyThreads"); | 
| @@ -1781,10 +1786,11 @@ void BrowserMainLoop::CreateAudioManager() { | 
| DCHECK(!audio_thread_); | 
| DCHECK(!audio_manager_); | 
| + audio_thread_ = base::MakeUnique<AudioManagerThread>(); | 
| audio_manager_ = GetContentClient()->browser()->CreateAudioManager( | 
| + audio_thread_->task_runner(), audio_thread_->worker_task_runner(), | 
| MediaInternals::GetInstance()); | 
| if (!audio_manager_) { | 
| - audio_thread_ = base::MakeUnique<AudioManagerThread>(); | 
| audio_manager_ = media::AudioManager::Create( | 
| audio_thread_->task_runner(), audio_thread_->worker_task_runner(), | 
| BrowserThread::GetTaskRunnerForThread(BrowserThread::FILE), | 
| @@ -1796,4 +1802,25 @@ void BrowserMainLoop::CreateAudioManager() { | 
| CHECK(audio_system_); | 
| } | 
| +void BrowserMainLoop::DestroyAudioManager() { | 
| + DCHECK(audio_manager_); | 
| + DCHECK(audio_thread_); | 
| + | 
| + // |audio_manager_| must be shut down on |audio_thread_| before it is deleted. | 
| + if (audio_manager_->GetTaskRunner()->BelongsToCurrentThread()) { | 
| + audio_manager_->Shutdown(); | 
| + } else { | 
| + audio_manager_->GetTaskRunner()->PostTask( | 
| + FROM_HERE, base::Bind(&media::AudioManager::Shutdown, | 
| + base::Unretained(audio_manager_.get()))); | 
| + } | 
| + | 
| + // Stop |audio_thread_| before deleting |audio_manager_| so that all tasks | 
| + // currently in the |audio_thread_| queue run and no more can be posted. | 
| + audio_thread_.reset(); | 
| 
o1ka
2017/03/28 18:32:23
Is there a guarantee that all the queued tasks wil
 
o1ka
2017/03/29 09:28:32
Never mind - I checked it now, base::Thread will d
 | 
| + | 
| + // Safe to delete the |audio_manager_|. | 
| + audio_manager_.reset(); | 
| 
o1ka
2017/03/28 18:32:23
Looks like we need to reset AudioSystem before thi
 | 
| +} | 
| + | 
| } // namespace content |