refactor EnumerateDevices to make it a persistent request.

content/browser/renderer_host/media/media_stream_manager.cc

 // Copyright (c) 2012 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/media_stream_manager.h"
 
 #include <list>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "base/win/scoped_com_initializer.h"
+#include "content/browser/browser_main_loop.h"
+#include "content/browser/devices_monitor/devices_monitor.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/media_stream_device_settings.h"
 #include "content/browser/renderer_host/media/media_stream_requester.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/common/media/media_stream_options.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/media_observer.h"
 #include "googleurl/src/gurl.h"
 
@@ skipping 16 matching lines @@
   for (size_t i = 0; i < label.size(); ++i) {
     int random_char = base::RandGenerator(sizeof(kAlphabet) - 1);
     label[i] = kAlphabet[random_char];
   }
   return label;
 }
 
 // Helper to verify if a media stream type is part of options or not.
 static bool Requested(const StreamOptions& options,
                       MediaStreamType stream_type) {
   if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE &&

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

nit:

return (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE &&
        options.video) ||
       (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE &&
        options.audio);

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

       options.video) {
     return true;
   } else if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE &&
              options.audio) {
     return true;
   }
   return false;
 }
 
 DeviceThread::DeviceThread(const char* name)
@@ skipping 63 matching lines @@
   std::vector<RequestState> state;
   RequestType type;
   int render_process_id;
   int render_view_id;
   GURL security_origin;
   std::string requested_device_id;
   StreamDeviceInfoArray audio_devices;
   StreamDeviceInfoArray video_devices;
 };
 
+MediaStreamManager::EnumerationCache::EnumerationCache()
+    : valid(false) {
+}
+
 MediaStreamManager::MediaStreamManager(
     AudioInputDeviceManager* audio_input_device_manager,
     VideoCaptureManager* video_capture_manager)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
           device_settings_(new MediaStreamDeviceSettings(this))),
       audio_input_device_manager_(audio_input_device_manager),
       video_capture_manager_(video_capture_manager),
+      monitoring_started_(false),
       enumeration_in_progress_(content::NUM_MEDIA_STREAM_DEVICE_TYPES, false),

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

This array seems to be used in conjunction with the cache.  How about adding a
bool to the struct, enumeration_in_progress_ and reduce the bookkeeping?

[wjia(left Chromium), 2012/08/01 01:28:59]

This |enumeration_in_progress_| also include other type of requests, such as
GenerationStream, while cache is used for EnumerateDevices only.

       io_loop_(NULL) {
 }
 
 MediaStreamManager::~MediaStreamManager() {
   DCHECK(requests_.empty());
   DCHECK(!device_thread_.get());
   DCHECK(!io_loop_);
 }
 
 VideoCaptureManager* MediaStreamManager::video_capture_manager() {
@@ skipping 17 matching lines @@
                                         const GURL& security_origin,
                                         std::string* label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Create a new request based on options.
   DeviceRequest new_request(requester, options,
                             render_process_id,
                             render_view_id,
                             security_origin);
   StartEnumeration(&new_request, label);
+
+  // Get user confirmation to use capture devices.
+  // Need to make an asynchronous call to make sure the |requester| gets the
+  // |label| before it would receive any event.
+  BrowserThread::PostTask(BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&MediaStreamDeviceSettings::RequestCaptureDeviceUsage,
+                 base::Unretained(device_settings_.get()),
+                 *label, render_process_id,
+                 render_view_id, options,
+                 security_origin));
 }
 
 void MediaStreamManager::CancelRequests(MediaStreamRequester* requester) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DeviceRequests::iterator it = requests_.begin();
   while (it != requests_.end()) {
     if (it->second.requester == requester && !RequestDone(it->second)) {
       // The request isn't complete, but there might be some devices already
       // opened -> close them.
       DeviceRequest* request = &(it->second);
@@ skipping 50 matching lines @@
     }
     device_settings_->RemovePendingCaptureRequest(label);
   }
 }
 
 void MediaStreamManager::StopGeneratedStream(const std::string& label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Find the request and close all open devices for the request.
   DeviceRequests::iterator it = requests_.find(label);
   if (it != requests_.end()) {
+    if (it->second.type == DeviceRequest::kEnumerateDevices) {

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

kEnumerateDevices -> ENUMERATE_DEVICES

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

+      StopEnumerateDevices(label);
+      return;
+    }
     for (StreamDeviceInfoArray::iterator audio_it =
          it->second.audio_devices.begin();
          audio_it != it->second.audio_devices.end(); ++audio_it) {
       audio_input_device_manager()->Close(audio_it->session_id);
     }
     for (StreamDeviceInfoArray::iterator video_it =
          it->second.video_devices.begin();
          video_it != it->second.video_devices.end(); ++video_it) {
       video_capture_manager()->Close(video_it->session_id);
     }
     if (it->second.type == DeviceRequest::kGenerateStream) {
       NotifyObserverDevicesClosed(&(it->second));
     }
     requests_.erase(it);
-    return;
   }
 }
 
 void MediaStreamManager::EnumerateDevices(
     MediaStreamRequester* requester,
     int render_process_id,
     int render_view_id,
     MediaStreamType type,
     const GURL& security_origin,
     std::string* label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK(type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE ||
+         type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE);
 
   // Create a new request.
   StreamOptions options;
-  if (type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE)
+  EnumerationCache* cache = NULL;
+  if (type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE) {
     options.audio = true;
-  else
+    cache = &audio_enumeration_cache_;
+  } else {
     options.video = true;
+    cache = &video_enumeration_cache_;
+  }
 
   DeviceRequest new_request(requester, options,
                             render_process_id,
                             render_view_id,
                             security_origin);
   new_request.type = DeviceRequest::kEnumerateDevices;
 
-  StartEnumeration(&new_request, label);
+  if (cache->valid) {
+    // Cached device list of this type exists. Just send it out.
+    new_request.state[type] = DeviceRequest::kRequested;

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

kRequested -> REQUESTED

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

+    AddRequest(&new_request, label);
+    // Need to post a task since the requester won't have label till
+    // this function returns.
+    BrowserThread::PostTask(BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&MediaStreamManager::SendCachedDeviceList,
+                   base::Unretained(this), cache, *label));
+  } else {
+    StartEnumeration(&new_request, label);
+    StartMonitoring();
+  }
+}
+
+void MediaStreamManager::StopEnumerateDevices(const std::string& label) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  DeviceRequests::iterator it = requests_.find(label);
+  if (it != requests_.end()) {
+    DCHECK_EQ(it->second.type, DeviceRequest::kEnumerateDevices);
+    requests_.erase(it);
+    if (!HasEnumerationRequest()) {
+      StopMonitoring();
+    }
+  }
 }
 
 void MediaStreamManager::OpenDevice(
     MediaStreamRequester* requester,
     int render_process_id,
     int render_view_id,
     const std::string& device_id,
     MediaStreamType type,
     const GURL& security_origin,
     std::string* label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Create a new request.
   StreamOptions options;
   if (type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE)
     options.audio = true;
   else
     options.video = true;
 
   DeviceRequest new_request(requester, options,
                             render_process_id,
                             render_view_id,
                             security_origin);
   new_request.type = DeviceRequest::kOpenDevice;
   new_request.requested_device_id = device_id;
 
   StartEnumeration(&new_request, label);
 }
 
+void MediaStreamManager::SendCachedDeviceList(
+    EnumerationCache* cache,
+    const std::string& label) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (cache->valid && requests_.find(label) != requests_.end()) {
+    requests_[label].requester->DevicesEnumerated(label, cache->devices);

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

nit: you've already looked up the label so you could use an iterator instead of
looking it up again via the [] operator.

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

+  }
+}
+
+void MediaStreamManager::StartMonitoring() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (!monitoring_started_) {
+    monitoring_started_ = true;
+    content::BrowserMainLoop::GetDevicesMonitor()->Start(this);
+    base::SystemMonitor::Get()->AddDevicesChangedObserver(this);

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

What do you think about this:

* Remove:
  - StartMonitoring
  - StopMonitoring
  - monitoring_started_ 
  - the call to BrowserMainLoop::GetDevicesMonitor()->Start(this).
  I don't think we need a dependency on BrowserLoop here since
  all we need is provided by the SystemMonitor.

* Move the creation of the MediaStreamManager from BML::MainMessageLoopStart to
BML::BrowserThreadsStarted.

* In the MediaStreamManager ctor invoke AddDevicesChangedObserver:

BrowserThread::PostTask(BrowserThread::IO,
    FROM_HERE,
    base::Bind(&base::SystemMonitor::AddDevicesChangedObserver,
        base::SystemMonitor::Get(), this));


I think this will simplify things quite a bit without reduced functionality.

[wjia(left Chromium), 2012/08/01 01:28:59]

The start/stop of DeviceMonitor has been removed because DeviceMonitor has been
started ever since browser is started.

It makes sense for MSM to listen to SystemMonitor only when there is such
request. 

[tommi (sloooow) - chröme, 2012/08/01 10:21:37]

Great.  Thanks.

+  }
+}
+
+void MediaStreamManager::StopMonitoring() {
+  DCHECK_EQ(MessageLoop::current(), io_loop_);
+  if (monitoring_started_ && !HasEnumerationRequest()) {
+    content::BrowserMainLoop::GetDevicesMonitor()->Stop(this);
+    base::SystemMonitor::Get()->RemoveDevicesChangedObserver(this);
+    monitoring_started_ = false;
+    ClearEnumerationCache(&audio_enumeration_cache_);
+    ClearEnumerationCache(&video_enumeration_cache_);
+  }
+}
+
+void MediaStreamManager::ClearEnumerationCache(EnumerationCache* cache) {

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

could an empty cache mean valid == false?
if so, we could just use the array directly instead of a struct and clear() it
here.

[wjia(left Chromium), 2012/08/01 01:28:59]

Unfortunately, empty cache doesn't mean valid or invalid since it's possible
enumeration returns an empty list of device when there is no such devices
attached to the computer.

[tommi (sloooow) - chröme, 2012/08/01 10:21:37]

sgtm.

+  DCHECK_EQ(MessageLoop::current(), io_loop_);
+  if (cache->valid) {

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

no need to check it first.  just set valid to false regardless.

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

+    cache->valid = false;
+  }
+}
+
 void MediaStreamManager::StartEnumeration(
     DeviceRequest* new_request,
     std::string* label) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   MediaStreamType stream_type = content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE;
   if (Requested(new_request->options, stream_type)) {
     new_request->state[stream_type] = DeviceRequest::kRequested;
     if (!enumeration_in_progress_[stream_type]) {
       enumeration_in_progress_[stream_type] = true;
       GetDeviceManager(stream_type)->EnumerateDevices();
     }
   }
   stream_type = content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE;
   if (Requested(new_request->options, stream_type)) {
     new_request->state[stream_type] = DeviceRequest::kRequested;
     if (!enumeration_in_progress_[stream_type]) {
       enumeration_in_progress_[stream_type] = true;
       GetDeviceManager(stream_type)->EnumerateDevices();
     }
   }
 
+  AddRequest(new_request, label);
+}
+
+void MediaStreamManager::AddRequest(
+    DeviceRequest* new_request,
+    std::string* label) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
   // Create a label for this request and verify it is unique.
   std::string request_label;
   do {
     request_label = RandomLabel();
   } while (requests_.find(request_label) != requests_.end());
 
   requests_.insert(std::make_pair(request_label, *new_request));
 
-  // Get user confirmation to use capture devices.
-  // Need to make an asynchronous call to make sure the |requester| gets the
-  // |label| before it would receive any event.
-  if (new_request->type == DeviceRequest::kGenerateStream) {
-    BrowserThread::PostTask(BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&MediaStreamDeviceSettings::RequestCaptureDeviceUsage,
-                   base::Unretained(device_settings_.get()),
-                   request_label, new_request->render_process_id,
-                   new_request->render_view_id, new_request->options,
-                   new_request->security_origin));
-  }
-
   (*label) = request_label;
 }
 
 void MediaStreamManager::EnsureDeviceThreadAndListener() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (device_thread_.get())
     return;
 
   device_thread_.reset(new DeviceThread("MediaStreamDeviceThread"));
   CHECK(device_thread_->Start());
@@ skipping 76 matching lines @@
 
 void MediaStreamManager::Closed(MediaStreamType stream_type,
                                 int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 }
 
 void MediaStreamManager::DevicesEnumerated(
     MediaStreamType stream_type, const StreamDeviceInfoArray& devices) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
+  // Only cache device list when there is EnumerateDevices request, since
+  // other requests don't turn on device monitoring.
+  bool need_update_clients = false;
+  EnumerationCache* cache =
+      (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE ?
+       &audio_enumeration_cache_ : &video_enumeration_cache_);
+  if (HasEnumerationRequest(stream_type)) {
+    if (!cache->valid || cache->devices != devices) {

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

I think this is a case for not needing the |valid| variable.  Just clear the
array if we want to refresh the cache.

[wjia(left Chromium), 2012/08/01 01:28:59]

Without |valid| flag, we have to keep cache always up-to-date. That means we
have to enumerate devices whenever their is a DevicesChanged event. That's what
we don't want to do since enumeration is known to be expensive and we try to
avoid it.

[tommi (sloooow) - chröme, 2012/08/01 10:21:37]

sgtm

+      cache->devices = devices;
+      need_update_clients = true;
+    }
+  }
+
   // Publish the result for all requests waiting for device list(s).
   // Find the requests waiting for this device list, store their labels and
   // release the iterator before calling device settings. We might get a call
   // back from device_settings that will need to iterate through devices.
   std::list<std::string> label_list;
   for (DeviceRequests::iterator it = requests_.begin(); it != requests_.end();
        ++it) {
     if (it->second.state[stream_type] == DeviceRequest::kRequested &&
         Requested(it->second.options, stream_type)) {
-      it->second.state[stream_type] = DeviceRequest::kPendingApproval;
+      if (it->second.type != DeviceRequest::kEnumerateDevices)
+        it->second.state[stream_type] = DeviceRequest::kPendingApproval;
       label_list.push_back(it->first);
     }
   }
   for (std::list<std::string>::iterator it = label_list.begin();
        it != label_list.end(); ++it) {
     DeviceRequest& request = requests_[*it];
     switch (request.type) {
       case DeviceRequest::kEnumerateDevices:
-        request.requester->DevicesEnumerated(*it, devices);
-        requests_.erase(*it);
+        if (need_update_clients)
+          request.requester->DevicesEnumerated(*it, devices);
         break;
       case DeviceRequest::kOpenDevice:
         for (StreamDeviceInfoArray::const_iterator device_it = devices.begin();
              device_it != devices.end(); device_it++) {
           if (request.requested_device_id == device_it->device_id) {
             StreamDeviceInfo device = *device_it;
             device.in_use = false;
             device.session_id =
                 GetDeviceManager(device_it->stream_type)->Open(device);
             request.state[device_it->stream_type] = DeviceRequest::kOpening;
@@ skipping 133 matching lines @@
 }
 
 void MediaStreamManager::UseFakeDevice() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   video_capture_manager()->UseFakeDevice();
   device_settings_->UseFakeUI();
 }
 
 void MediaStreamManager::WillDestroyCurrentMessageLoop() {
   DCHECK_EQ(MessageLoop::current(), io_loop_);
+  DCHECK(requests_.empty());
   if (device_thread_.get()) {
+    StopMonitoring();
+
     video_capture_manager_->Unregister();
     audio_input_device_manager_->Unregister();
     device_thread_.reset();
   }
 
   audio_input_device_manager_ = NULL;
   video_capture_manager_ = NULL;
   io_loop_ = NULL;
   device_settings_.reset();
 }
@@ skipping 72 matching lines @@
     MediaStreamType stream_type) {
   if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE) {
     return video_capture_manager();
   } else if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE) {
     return audio_input_device_manager();
   }
   NOTREACHED();
   return NULL;
 }
 
+void MediaStreamManager::OnDevicesChanged(
+    base::SystemMonitor::DeviceType device_type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  MediaStreamType stream_type;
+  EnumerationCache* cache;
+  if (device_type == base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE) {
+    stream_type = content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE;
+    cache = &audio_enumeration_cache_;
+  } else if (device_type == base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE) {
+    stream_type = content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE;
+    cache = &video_enumeration_cache_;
+  } else {
+    return;  // Uninterested device change.

[tommi (sloooow) - chröme, 2012/07/30 12:10:21]

s/Uninterested/Uninteresting.

[wjia(left Chromium), 2012/08/01 01:28:59]

Done.

+  }
+
+  if (!HasEnumerationRequest(stream_type)) {
+    // There is no request for that type, No need to enumerate devices.
+    // Therefore, invalidate the cache of that type.
+    ClearEnumerationCache(cache);
+  } else if (!enumeration_in_progress_[stream_type]) {
+    enumeration_in_progress_[stream_type] = true;
+    GetDeviceManager(stream_type)->EnumerateDevices();
+  }
+}
+
+bool MediaStreamManager::HasEnumerationRequest() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  for (DeviceRequests::iterator it = requests_.begin();
+       it != requests_.end(); ++it) {
+    DeviceRequest& request = it->second;
+    if (request.type == DeviceRequest::kEnumerateDevices) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool MediaStreamManager::HasEnumerationRequest(
+    MediaStreamType stream_type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  for (DeviceRequests::iterator it = requests_.begin();
+       it != requests_.end(); ++it) {
+    DeviceRequest& request = it->second;
+    if (request.type == DeviceRequest::kEnumerateDevices &&
+        Requested(request.options, stream_type)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace media_stream