Video Capture: Allow suspension of individual devices.

content/browser/renderer_host/media/video_capture_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/video_capture_manager.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 721 matching lines @@
     VideoCaptureController* controller,
     VideoCaptureControllerID client_id,
     VideoCaptureControllerEventHandler* client_handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(controller);
   DCHECK(client_handler);
   DeviceEntry* entry = GetDeviceEntryByController(controller);
   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(
     media::VideoCaptureSessionId session_id,
     const media::VideoCaptureParams& params,
     VideoCaptureController* controller,
     VideoCaptureControllerID client_id,
     VideoCaptureControllerEventHandler* client_handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(controller);
   DCHECK(client_handler);
 
   DeviceEntry* entry = GetDeviceEntryByController(controller);
   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(
     VideoCaptureController* controller) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (DeviceEntry* entry = GetDeviceEntryByController(controller)) {
     if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
       device_task_runner_->PostTask(
           FROM_HERE,
@@ skipping 486 matching lines @@
     if (!device_in_queue) {
       // Session ID is only valid for Screen capture. So we can fake it to
       // resume video capture devices here.
       QueueStartDevice(kFakeSessionId, entry.get(), entry->parameters);
     }
   }
 }
 #endif  // defined(OS_ANDROID)
 
 }  // namespace content