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 fa65a2cf2149be9bdcd5294d1c05d8a23db117df..80689331eab5667deb6a7489c2582e9b0779f755 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -734,11 +734,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()) {
+    if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
+      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)));
+    }
+  }

[chfremer, 2016/09/27 00:54:59]

We are changing the behavior of VideoCaptureManager here, but this does not seem
to affect the existing unit tests. Is there a gap in the test coverage?

The test case PauseAndResumeClient exercises this code path, but it does not
verify any calls being made from here to the device, which it should.

What the test case does verify is calls made to an Observer (via interface
MediaStreamProviderListener). Would such an Observer be interested in Suspended
and Resumed events?

[miu, 2016/09/27 23:42:22]

Seems so. I added some extra testing around this to check whether
VideoCaptureDevice::MaybeSuspend() and Resume() are called.

Note: In the follow-up change, the tab capture browser tests are also testing
this functionality (as part of an end-to-end test).
The MediaStreamManager handles session-level events. I don't see a use case for
it to act on internal events like this one, so I don't think it would be
appropriate to add anything to that interface.

 }
 
 void VideoCaptureManager::ResumeCaptureForClient(
@@ -755,11 +764,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(