Refactor the AudioInputDeviceManager by removing the device thread.

content/browser/renderer_host/media/audio_input_device_manager.cc

Index: content/browser/renderer_host/media/audio_input_device_manager.cc
===================================================================
--- content/browser/renderer_host/media/audio_input_device_manager.cc	(revision 111331)
+++ content/browser/renderer_host/media/audio_input_device_manager.cc	(working copy)
@@ -21,37 +21,12 @@
 // Starting id for the first capture session.
 const int kFirstSessionId = AudioInputDeviceManager::kFakeOpenSessionId + 1;
 
-// Helper function.
-static bool IsValidAudioInputDevice(const media::AudioDeviceName& device) {
-  AudioManager* audio_manager = AudioManager::GetAudioManager();
-  if (!audio_manager)
-    return false;
-
-  // Get the up-to-date list of devices and verify the device is in the list.
-  media::AudioDeviceNames device_names;
-  audio_manager->GetAudioInputDeviceNames(&device_names);
-  if (!device_names.empty()) {
-    for (media::AudioDeviceNames::iterator iter = device_names.begin();
-         iter != device_names.end();
-         ++iter) {
-      if (iter->device_name == device.device_name &&
-          iter->unique_id == device.unique_id)
-        return true;
-    }
-  }
-  // The device wasn't found.
-  return false;
-}
-
 AudioInputDeviceManager::AudioInputDeviceManager()
-    : audio_input_device_thread_("AudioInputDeviceManagerThread"),
-      listener_(NULL),
+    : listener_(NULL),
       next_capture_session_id_(kFirstSessionId) {
-  audio_input_device_thread_.Start();
 }
 
 AudioInputDeviceManager::~AudioInputDeviceManager() {
-  audio_input_device_thread_.Stop();
 }
 
 void AudioInputDeviceManager::Register(MediaStreamProviderListener* listener) {
@@ -70,79 +45,6 @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(listener_);
 
-  audio_input_device_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::EnumerateOnDeviceThread,
-                 base::Unretained(this)));
-}
-
-int AudioInputDeviceManager::Open(const StreamDeviceInfo& device) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  // Generate a new id for this device.
-  int audio_input_session_id = next_capture_session_id_++;
-
-  audio_input_device_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::OpenOnDeviceThread,
-                 base::Unretained(this), audio_input_session_id, device));
-
-  return audio_input_session_id;
-}
-
-void AudioInputDeviceManager::Close(int session_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(listener_);
-
-  audio_input_device_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::CloseOnDeviceThread,
-                 base::Unretained(this), session_id));
-}
-
-void AudioInputDeviceManager::Start(
-    int session_id, AudioInputDeviceManagerEventHandler* event_handler) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(event_handler);
-
-  // Solution for not using MediaStreamManager. This is needed when Start() is
-  // called without using Open(), we post default device for test purpose.
-  // And we do not store the info for the kFakeOpenSessionId but return
-  // the callback immediately.
-  if (session_id == kFakeOpenSessionId) {
-    event_handler->OnDeviceStarted(session_id,
-                                   AudioManagerBase::kDefaultDeviceId);
-    return;
-  }
-
-  // If session has been started, post a callback with an error.
-  if (event_handlers_.find(session_id) != event_handlers_.end()) {
-    // Session has been started, post a callback with error.
-    event_handler->OnDeviceStarted(session_id, kInvalidDeviceId);
-    return;
-  }
-
-  // Add the event handler to the session.
-  event_handlers_.insert(std::make_pair(session_id, event_handler));
-
-  audio_input_device_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::StartOnDeviceThread,
-                 base::Unretained(this), session_id));
-}
-
-void AudioInputDeviceManager::Stop(int session_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  audio_input_device_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioInputDeviceManager::StopOnDeviceThread,
-                 base::Unretained(this), session_id));
-}
-
-void AudioInputDeviceManager::EnumerateOnDeviceThread() {
-  DCHECK(IsOnCaptureDeviceThread());
-
   // Get the device list from system.
   media::AudioDeviceNames device_names;
   AudioManager::GetAudioManager()->GetAudioInputDeviceNames(&device_names);
@@ -155,43 +57,58 @@
                                        it->unique_id, false));
   }
 
+  // Returns the device list through the listener by posting a task on
+  // IO thread since MediaStreamManager handles the callback asynchronously.
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&AudioInputDeviceManager::DevicesEnumeratedOnIOThread,
-                 base::Unretained(this), devices));
+                 base::Unretained(this),
+                 devices));

[tommi (sloooow) - chröme, 2011/11/23 20:16:31]

won't |devices| go out of scope and thus be garbage when the task runs?  I guess
I must be missing something... will there be a copy made?

[no longer working on chromium, 2011/11/24 14:47:34]

I did not dig into the details on how the bind works regarding to this question,
but I found quite a lot of sample code doing the similar things. So please allow
me to assume it is a copy made.

[henrika (OOO until Aug 14), 2011/11/24 15:22:26]

Can probably be done without a copy.

[no longer working on chromium, 2011/11/24 16:00:42]

Use a pointer as Tommi suggests.

 }
 
-void AudioInputDeviceManager::OpenOnDeviceThread(
-    int session_id, const StreamDeviceInfo& device) {
-  DCHECK(IsOnCaptureDeviceThread());
+int AudioInputDeviceManager::Open(const StreamDeviceInfo& device) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Generate a new id for this device.
+  int session_id = next_capture_session_id_++;
   DCHECK(devices_.find(session_id) == devices_.end());
 
   media::AudioDeviceName audio_input_device_name;
   audio_input_device_name.device_name = device.name;
   audio_input_device_name.unique_id = device.device_id;
 
-  // Check if the device is valid
-  if (!IsValidAudioInputDevice(audio_input_device_name)) {
-    SignalError(session_id, kDeviceNotAvailable);
-    return;
-  }
-
   // Add the session_id and device to the list.
   devices_[session_id] = audio_input_device_name;
+
+  // Returns the |session_id| through the listener by posting a task on
+  // IO thread since MediaStreamManager handles the callback asynchronously.
   BrowserThread::PostTask(BrowserThread::IO,
                           FROM_HERE,
                           base::Bind(
                               &AudioInputDeviceManager::OpenedOnIOThread,

[henrika (OOO until Aug 14), 2011/11/24 15:22:26]

Odd indentation.

[no longer working on chromium, 2011/11/24 16:00:42]

Done.

                               base::Unretained(this),
                               session_id));
+
+  return session_id;
 }
 
-void AudioInputDeviceManager::CloseOnDeviceThread(int session_id) {
-  DCHECK(IsOnCaptureDeviceThread());
+void AudioInputDeviceManager::Close(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(listener_);
   DCHECK(devices_.find(session_id) != devices_.end());
 
   devices_.erase(session_id);
+
+  // Checks if the device has been stopped, if not, sends stop signal.
+  EventHandlerMap::iterator it = event_handlers_.find(session_id);
+  if (it != event_handlers_.end()) {
+    it->second->OnDeviceStopped(session_id);
+    event_handlers_.erase(session_id);
+  }
+
+  // Posts a callback through the listener on IO thread since
+  // MediaStreamManager handles the callback asynchronously.
   BrowserThread::PostTask(BrowserThread::IO,
                           FROM_HERE,
                           base::Bind(
@@ -200,49 +117,42 @@
                               session_id));
 }
 
-void AudioInputDeviceManager::StartOnDeviceThread(const int session_id) {
-  DCHECK(IsOnCaptureDeviceThread());
+void AudioInputDeviceManager::Start(
+    int session_id, AudioInputDeviceManagerEventHandler* event_handler) {

[tommi (sloooow) - chröme, 2011/11/23 20:16:31]

move int session_id to the previous line

[no longer working on chromium, 2011/11/24 14:47:34]

"AudioInputDeviceManagerEventHandler* event_handler" is too long to be aligned
to "int session_id" if I move it to the previous line.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(event_handler);
 
-  // Get the up-to-date device enumeration list from the OS and find out
-  // the unique id of the device.
-  std::string device_id = kInvalidDeviceId;
-  AudioInputDeviceMap::const_iterator it = devices_.find(session_id);
-  if (it != devices_.end()) {
-    media::AudioDeviceNames device_names;
-    AudioManager::GetAudioManager()->GetAudioInputDeviceNames(&device_names);
-    if (!device_names.empty()) {
-      int index = 0;
-      for (media::AudioDeviceNames::iterator iter = device_names.begin();
-           iter != device_names.end();
-           ++iter, ++index) {
-        if (iter->device_name == it->second.device_name &&
-            iter->unique_id == it->second.unique_id) {
-          // Found the device.
-          device_id = iter->unique_id;
-          break;
-        }
-      }
-    }
+  // Solution for not using MediaStreamManager. This is needed when Start() is
+  // called without using Open(), we post default device for test purpose.
+  // And we do not store the info for the kFakeOpenSessionId but return
+  // the callback immediately.
+  if (session_id == kFakeOpenSessionId) {
+    event_handler->OnDeviceStarted(session_id,
+                                   AudioManagerBase::kDefaultDeviceId);
+    return;
   }
 
-  // Posts the index to AudioInputRenderHost through the event handler.
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(
-                              &AudioInputDeviceManager::StartedOnIOThread,
-                              base::Unretained(this),
-                              session_id,
-                              device_id));
+  // Checks if the device has been opened or not.
+  std::string device_id = (devices_.find(session_id) == devices_.end()) ?
+      kInvalidDeviceId : devices_[session_id].unique_id;
+
+  // Adds the event handler to the session if the session has not been started,
+  // otherwise post a |kInvalidDeviceId| to indicate that Start() fails.
+  if (event_handlers_.find(session_id) == event_handlers_.end())
+    event_handlers_.insert(std::make_pair(session_id, event_handler));
+  else
+    device_id = kInvalidDeviceId;
+
+  // Post a callback through the AudioInputRendererHost to notify the renderer
+  // device has been started.
+  event_handler->OnDeviceStarted(session_id, device_id);
 }
 
-void AudioInputDeviceManager::StopOnDeviceThread(int session_id) {
-  DCHECK(IsOnCaptureDeviceThread());
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(
-                              &AudioInputDeviceManager::StoppedOnIOThread,
-                              base::Unretained(this),
-                              session_id));
+void AudioInputDeviceManager::Stop(int session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  // Erase the event handler referenced by the session_id.
+  event_handlers_.erase(session_id);
 }
 
 void AudioInputDeviceManager::DevicesEnumeratedOnIOThread(
@@ -260,69 +170,8 @@
 
 void AudioInputDeviceManager::ClosedOnIOThread(int session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  EventHandlerMap::iterator it = event_handlers_.find(session_id);
-  if (it != event_handlers_.end()) {
-    // The device hasn't been stopped, send stop signal.
-    it->second->OnDeviceStopped(session_id);
-    event_handlers_.erase(session_id);
-  }
-  listener_->Closed(kAudioCapture, session_id);
-}
-
-
-void AudioInputDeviceManager::ErrorOnIOThread(int session_id,
-                                              MediaStreamProviderError error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (listener_)
-    listener_->Error(kAudioCapture, session_id, error);
+    listener_->Closed(kAudioCapture, session_id);
 }
 
-void AudioInputDeviceManager::StartedOnIOThread(
-    int session_id, const std::string& device_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  EventHandlerMap::iterator it = event_handlers_.find(session_id);
-  if (it == event_handlers_.end())
-    return;
-
-  // Post a callback through the AudioInputRendererHost to notify the renderer
-  // device has been started.
-  it->second->OnDeviceStarted(session_id, device_id);
-}
-
-void AudioInputDeviceManager::StoppedOnIOThread(int session_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // Erase the event handler referenced by the session_id.
-  event_handlers_.erase(session_id);
-}
-
-void AudioInputDeviceManager::SignalError(int session_id,
-                                          MediaStreamProviderError error) {
-  BrowserThread::PostTask(BrowserThread::IO,
-                          FROM_HERE,
-                          base::Bind(
-                              &AudioInputDeviceManager::ErrorOnIOThread,
-                              base::Unretained(this),
-                              session_id,
-                              error));
-}
-
-bool AudioInputDeviceManager::IsOnCaptureDeviceThread() const {
-  return MessageLoop::current() == audio_input_device_thread_.message_loop();
-}
-
-void AudioInputDeviceManager::UnregisterEventHandler(int session_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  event_handlers_.erase(session_id);
-}
-
-bool AudioInputDeviceManager::HasEventHandler(int session_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  return event_handlers_.find(session_id) != event_handlers_.end();
-}
-
-MessageLoop* AudioInputDeviceManager::message_loop() {
-  return audio_input_device_thread_.message_loop();
-}
-
 }  // namespace media_stream