Mac AVFoundation/QTKit: delay DeviceMonitorMac startup to first GetUserMedia.

Mac AVFoundation/QTKit: delay DeviceMonitorMac startup to first GetUserMedia.

Currently DeviceMonitorMac is constructed from BrowserMainLoop in the
first compasses of Chrome bringup. DeviceMonitorMac creates either a
QTKitMonitorImpl or a AVFoundationMonitorImpl, that immediately
starts observing the system capture devices. This monitoring is
not needed until the user actively starts operating the devices,
that happens in MediaStreamManager.

This CL adds a method StartMonitoring() to DeviceMonitorMac that is
exercised from MediaStreamManager::StartMonitoring().

The performance regression in bugs 349616 and 348020 affects only
AVFoundation, but both implementations would benefit from a delayed
startup.

Tested locally via using --[enable/disable]-avfoundation, connecting
and disconnecting devices etc.

BUG=288562, 349616, 348020
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256483

[mcasas, 2014-03-10 11:58:17]

rsesek@, tommi@ could you comment on the approach? Thanks!

[Robert Sesek, 2014-03-10 17:22:46]

https://codereview.chromium.org/189443010/diff/1/content/browser/browser_main...
File content/browser/browser_main_loop.h (right):

https://codereview.chromium.org/189443010/diff/1/content/browser/browser_main...
content/browser/browser_main_loop.h:88: void StartMonitoring();
This is not the right place to put this. Expose a getter below, instead.

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/media/media_stream_manager.cc:798: void
MediaStreamManager::StartMonitoringOnUIThread() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/media/media_stream_manager.cc:800:
BrowserThread::PostTask(
Why do you need to post a task to do this?

[mcasas, 2014-03-10 18:19:44]

rsesek@, tommi@ PTAL.

https://codereview.chromium.org/189443010/diff/1/content/browser/browser_main...
File content/browser/browser_main_loop.h (right):

https://codereview.chromium.org/189443010/diff/1/content/browser/browser_main...
content/browser/browser_main_loop.h:88: void StartMonitoring();
On 2014/03/10 17:22:46, rsesek wrote:
> This is not the right place to put this. Expose a getter below, instead.

Done.

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/media/media_stream_manager.cc:798: void
MediaStreamManager::StartMonitoringOnUIThread() {
On 2014/03/10 17:22:46, rsesek wrote:
> DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

Done.

https://codereview.chromium.org/189443010/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/media/media_stream_manager.cc:800:
BrowserThread::PostTask(
On 2014/03/10 17:22:46, rsesek wrote:
> Why do you need to post a task to do this?

Not needed now with the accessor device_monitor_mac().

[tommi (sloooow) - chröme, 2014-03-10 18:38:26]

nice. lgtm

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
File content/browser/device_monitor_mac.mm (right):

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
content/browser/device_monitor_mac.mm:348: void
DeviceMonitorMac::StartMonitoring() {
Can you add a DCHECK that when we get here we're on the same thread as the
constructor was called on?

[Robert Sesek, 2014-03-10 18:45:25]

lgtm

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
File content/browser/device_monitor_mac.h (right):

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
content/browser/device_monitor_mac.h:25: void StartMonitoring();
nit: comment

[mcasas, 2014-03-11 07:57:32]

avi@ OWNERS stamp please.

rsesek@, tommi@, nits/comments taken in, thanks!.

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
File content/browser/device_monitor_mac.h (right):

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
content/browser/device_monitor_mac.h:25: void StartMonitoring();
On 2014/03/10 18:45:25, rsesek wrote:
> nit: comment

Done.

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
File content/browser/device_monitor_mac.mm (right):

https://codereview.chromium.org/189443010/diff/20001/content/browser/device_m...
content/browser/device_monitor_mac.mm:348: void
DeviceMonitorMac::StartMonitoring() {
On 2014/03/10 18:38:26, tommi wrote:
> Can you add a DCHECK that when we get here we're on the same thread as the
> constructor was called on?

Done.

[Avi (use Gerrit), 2014-03-11 13:31:50]

lgtm

stampity stamp

[mcasas, 2014-03-11 14:03:23]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-03-11 14:03:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mcasas@chromium.org/189443010/40001

[commit-bot: I haz the power, 2014-03-11 14:35:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-11 14:35:59]

Try jobs failed on following builders: linux_chromium_rel

[mcasas, 2014-03-11 18:09:09]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-03-11 18:12:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mcasas@chromium.org/189443010/60001

[commit-bot: I haz the power, 2014-03-11 21:53:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-11 21:53:01]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[mcasas, 2014-03-12 05:36:40]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-03-12 05:37:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mcasas@chromium.org/189443010/60001

[commit-bot: I haz the power, 2014-03-12 09:27:28]

Change committed as 256483