Video Capture: Allow suspension of individual devices.

content/browser/renderer_host/media/video_capture_manager.cc

Index: content/browser/renderer_host/media/video_capture_manager.cc
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index 380c626ba34760249828cf06abb3a42de254a9a1..ed851f1ea805a857531813dca4d68e333ca74fc9 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -739,11 +739,20 @@ void VideoCaptureManager::PauseCaptureForClient(
   if (!entry)
     NOTREACHED() << "Got Null entry while pausing capture";
 
-  // Do not pause Content Video Capture devices, e.g. Tab or Screen capture.
-  if (entry->stream_type != MEDIA_DEVICE_VIDEO_CAPTURE)
-    return;
-
+  const bool had_active_client = controller->HasActiveClient();
   controller->PauseClient(client_id, client_handler);
+  if (had_active_client && !controller->HasActiveClient()) {

[mcasas, 2016/09/28 21:34:44]

Early return? 

if (!had_active_client || controller->HasActiveClient())
  return;
// else...

Also in l.774

[miu, 2016/09/28 22:35:15]

Done.

+    if (media::VideoCaptureDevice* device = entry->video_capture_device()) {

[mcasas, 2016/09/28 21:34:44]

nit: I thought assignment-and-testing were discouraged
in Chromium, but can't find any reference to that in the
style guide (either Cr or G).  I'm fine either way but wdyt?

[miu, 2016/09/28 22:35:15]

I've used it all over the place. Never had anyone say it was a bad idea before.
;)

I can understand the confusion between good 'ol == vs = logic errors, but the
added type declaration mitigates that. Compare this:

 if (device = entry->video_capture_device())

to:

 if (media::VideoCaptureDevice* device = entry->video_capture_device())

The first form raises red flags, and looks very much like a logic error. That's
because |device| is initialized somewhere else and could be used after this
statement. The second form has the type declaration, so you can clearly see the
intention is initialization (not comparison), and the scoping of the variable is
restricted to the body of the if-statement.

+      device_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&VideoCaptureDevice::MaybeSuspend,
+                     // Unretained is safe to use here because |device| would be
+                     // null if it was scheduled for shutdown and destruction,
+                     // and because this task is guaranteed to run before the
+                     // task that destroys the |device|.
+                     base::Unretained(device)));
+    }
+  }
 }
 
 void VideoCaptureManager::ResumeCaptureForClient(
@@ -760,11 +769,20 @@ void VideoCaptureManager::ResumeCaptureForClient(
   if (!entry)
     NOTREACHED() << "Got Null entry while resuming capture";
 
-  // Do not resume Content Video Capture devices, e.g. Tab or Screen capture.
-  if (entry->stream_type != MEDIA_DEVICE_VIDEO_CAPTURE)
-    return;
-
+  const bool had_active_client = controller->HasActiveClient();
   controller->ResumeClient(client_id, client_handler);
+  if (!had_active_client && controller->HasActiveClient()) {
+    if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
+      device_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&VideoCaptureDevice::Resume,
+                     // Unretained is safe to use here because |device| would be
+                     // null if it was scheduled for shutdown and destruction,
+                     // and because this task is guaranteed to run before the
+                     // task that destroys the |device|.
+                     base::Unretained(device)));
+    }
+  }
 }
 
 void VideoCaptureManager::RequestRefreshFrameForClient(