Add logging of video device details on device open and close.

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 "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "content/browser/media/capture/web_contents_video_capture_device.h"
 #include "content/browser/media/media_internals.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/desktop_media_id.h"
 #include "content/public/common/media_stream_request.h"
@@ skipping 261 matching lines @@
     if (request->serial_id() == entry->serial_id) {
       request->set_abort_start();
       DVLOG(3) << "DoStopDevice, aborting start request for device "
                << entry->id << " serial_id = " << entry->serial_id;
       return;
     }
   }
 
   DVLOG(3) << "DoStopDevice. Send stop request for device = " << entry->id
            << " serial_id = " << entry->serial_id << ".";
+  entry->video_capture_controller()->DoLogOnIOThread(
+      base::StringPrintf("Stopping device: id: %s\n", entry->id.c_str()));
+
   if (entry->video_capture_device()) {
     // |entry->video_capture_device| can be null if creating the device fails.
     device_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread, this,
                    base::Passed(entry->ReleaseVideoCaptureDevice())));
   }
 }
 
 void VideoCaptureManager::HandleQueuedStartRequest() {
@@ skipping 11 matching lines @@
   DeviceEntries::iterator entry_it = std::find_if(
       devices_.begin(), devices_.end(),
       [serial_id] (const DeviceEntry* e) {
         return e->serial_id == serial_id;
       });
   DCHECK(entry_it != devices_.end());
   DeviceEntry* entry =  (*entry_it);
 
   DVLOG(3) << "HandleQueuedStartRequest, Post start to device thread, device = "
            << entry->id << " start id = " << entry->serial_id;
-  base::PostTaskAndReplyWithResult(
-      device_task_runner_.get(),
-      FROM_HERE,
-      base::Bind(
-          &VideoCaptureManager::DoStartDeviceOnDeviceThread,
-          this,
-          request->session_id(),
-          entry->id,
-          entry->stream_type,
-          request->params(),
-          base::Passed(entry->video_capture_controller()->NewDeviceClient())),
-      base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
-                 request->serial_id()));
+

[mcasas, 2015/12/01 15:30:38]

So my suggestion was to add here:

base::Callback<scoped_ptr<media::VideoCaptureDevice>(
    const std::string &, const media::VideoCaptureParams &,
    scoped_ptr<media::VideoCaptureDevice::Client>)> start_capture_function;

then in each case statement update it, e.g. l.350:

start_capture_function = base::bind(
    &VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread, this, found->name,
    request->params(),
    base::Passed(entry->video_capture_controller()->NewDeviceClient()));

and finally, out of the switch-case:
if (start_capture_function.is_null())
  return;
base::PostTaskAndReplyWithResult(
    device_task_runner_.get(), FROM_HERE, start_capture_function,
    base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
               request->serial_id()));

Not sure if it would read better but it'd take 3 similar superstatements
out of the cases... up to you.

[nisse-chromium (ooo August 14), 2015/12/02 14:26:00]

Done. Please have a look and see if it seems right. I don't quite understand the
base::Bind machinery.

[tommi (sloooow) - chröme, 2015/12/02 15:01:27]

This is good

+  switch (entry->stream_type) {
+    case MEDIA_DEVICE_VIDEO_CAPTURE: {
+      // We look up the device id from the renderer in our local enumeration
+      // since the renderer does not have all the information that might be
+      // held in the browser-side VideoCaptureDevice::Name structure.
+      const media::VideoCaptureDeviceInfo* found =
+          FindDeviceInfoById(entry->id, devices_info_cache_);
+      if (found) {
+        entry->video_capture_controller()->DoLogOnIOThread(base::StringPrintf(
+            "Starting device: id: %s, name: %s, api: %s",
+            found->name.id().c_str(), found->name.GetNameAndModel().c_str(),
+            found->name.GetCaptureApiTypeString()));
+
+        base::PostTaskAndReplyWithResult(
+            device_task_runner_.get(), FROM_HERE,
+            base::Bind(
+                &VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread, this,
+                found->name, request->params(),
+                base::Passed(
+                    entry->video_capture_controller()->NewDeviceClient())),
+            base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
+                       request->serial_id()));
+      } else {
+        // Errors from DoStartDeviceCaptureOnDeviceThread go via
+        // VideoCaptureDeviceClient::OnError, which needs some thread
+        // dancing to get errors processed on the IO thread. But since
+        // we're on that thread, we call VideoCaptureController
+        // methods directly.
+        const std::string log_message = base::StringPrintf(
+            "Error on %s:%d: device %s unknown. Maybe recently disconnected?",
+            __FILE__, __LINE__, entry->id.c_str());
+        DLOG(ERROR) << log_message;
+        entry->video_capture_controller()->DoLogOnIOThread(log_message);
+        entry->video_capture_controller()->DoErrorOnIOThread();
+      }
+      break;
+    }
+    case MEDIA_TAB_VIDEO_CAPTURE: {
+      base::PostTaskAndReplyWithResult(
+          device_task_runner_.get(), FROM_HERE,
+          base::Bind(&VideoCaptureManager::DoStartTabCaptureOnDeviceThread,
+                     this, entry->id, request->params(),
+                     base::Passed(
+                         entry->video_capture_controller()->NewDeviceClient())),
+          base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
+                     request->serial_id()));
+      break;
+    }
+    case MEDIA_DESKTOP_VIDEO_CAPTURE: {
+      base::PostTaskAndReplyWithResult(
+          device_task_runner_.get(), FROM_HERE,
+          base::Bind(&VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread,
+                     this, entry->id, request->params(),
+                     base::Passed(
+                         entry->video_capture_controller()->NewDeviceClient())),
+          base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
+                     request->serial_id()));
+
+      break;
+    }
+    default: {
+      NOTIMPLEMENTED();
+      break;
+    }
+  }
 }
 
 void VideoCaptureManager::OnDeviceStarted(
     int serial_id,
     scoped_ptr<media::VideoCaptureDevice> device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(serial_id == device_start_queue_.begin()->serial_id());
   DVLOG(3) << "OnDeviceStarted";
   if (device_start_queue_.front().abort_start()) {
-    // |device| can be null if creation failed in DoStartDeviceOnDeviceThread.
+    // |device| can be null if creation failed in
+    // DoStartDeviceCaptureOnDeviceThread.
     // The device is no longer wanted. Stop the device again.
     DVLOG(3) << "OnDeviceStarted but start request have been aborted.";
     media::VideoCaptureDevice* device_ptr = device.get();
     base::Closure closure =
         base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread, this,
                    base::Passed(&device));
     if (device_ptr && !device_task_runner_->PostTask(FROM_HERE, closure)) {
       // PostTask failed. The device must be stopped anyway.
       device_ptr->StopAndDeAllocate();
     }
@@ skipping 13 matching lines @@
           device_start_queue_.front().session_id();
       MaybePostDesktopCaptureWindowId(session_id);
     }
   }
 
   device_start_queue_.pop_front();
   HandleQueuedStartRequest();
 }
 
 scoped_ptr<media::VideoCaptureDevice>
-VideoCaptureManager::DoStartDeviceOnDeviceThread(
-    media::VideoCaptureSessionId session_id,
-    const std::string& id,
-    MediaStreamType stream_type,
+VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread(
+    const media::VideoCaptureDevice::Name& name,
     const media::VideoCaptureParams& params,
     scoped_ptr<media::VideoCaptureDevice::Client> device_client) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(IsOnDeviceThread());
 
   scoped_ptr<media::VideoCaptureDevice> video_capture_device;
-  switch (stream_type) {
-    case MEDIA_DEVICE_VIDEO_CAPTURE: {
-      // We look up the device id from the renderer in our local enumeration
-      // since the renderer does not have all the information that might be
-      // held in the browser-side VideoCaptureDevice::Name structure.
-      const media::VideoCaptureDeviceInfo* found =
-          FindDeviceInfoById(id, devices_info_cache_);
-      if (found) {
-        video_capture_device =
-            video_capture_device_factory_->Create(found->name);
-      }
-      break;
-    }
-    case MEDIA_TAB_VIDEO_CAPTURE: {
-      video_capture_device.reset(
-          WebContentsVideoCaptureDevice::Create(id));
-      break;
-    }
-    case MEDIA_DESKTOP_VIDEO_CAPTURE: {
+  video_capture_device = video_capture_device_factory_->Create(name);
+
+  if (!video_capture_device) {
+    device_client->OnError(FROM_HERE, "Could not create capture device");
+    return nullptr;
+  }
+
+  video_capture_device->AllocateAndStart(params, device_client.Pass());
+  return video_capture_device.Pass();
+}
+
+scoped_ptr<media::VideoCaptureDevice>
+VideoCaptureManager::DoStartTabCaptureOnDeviceThread(
+    const std::string& id,
+    const media::VideoCaptureParams& params,
+    scoped_ptr<media::VideoCaptureDevice::Client> device_client) {
+  SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
+  DCHECK(IsOnDeviceThread());
+
+  scoped_ptr<media::VideoCaptureDevice> video_capture_device;
+  video_capture_device.reset(WebContentsVideoCaptureDevice::Create(id));
+
+  if (!video_capture_device) {
+    device_client->OnError(FROM_HERE, "Could not create capture device");
+    return nullptr;
+  }
+
+  video_capture_device->AllocateAndStart(params, device_client.Pass());
+  return video_capture_device.Pass();
+}
+
+scoped_ptr<media::VideoCaptureDevice>
+VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread(
+    const std::string& id,
+    const media::VideoCaptureParams& params,
+    scoped_ptr<media::VideoCaptureDevice::Client> device_client) {
+  SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
+  DCHECK(IsOnDeviceThread());
+
+  scoped_ptr<media::VideoCaptureDevice> video_capture_device;
 #if defined(ENABLE_SCREEN_CAPTURE)
-      DesktopMediaID desktop_id = DesktopMediaID::Parse(id);
-      if (!desktop_id.is_null()) {
+  DesktopMediaID desktop_id = DesktopMediaID::Parse(id);
+  if (!desktop_id.is_null()) {
 #if defined(USE_AURA)
-        video_capture_device = DesktopCaptureDeviceAura::Create(desktop_id);
+    video_capture_device = DesktopCaptureDeviceAura::Create(desktop_id);
 #endif
-        if (!video_capture_device)
-          video_capture_device = DesktopCaptureDevice::Create(desktop_id);
-      }
+    if (!video_capture_device)
+      video_capture_device = DesktopCaptureDevice::Create(desktop_id);
 #endif  // defined(ENABLE_SCREEN_CAPTURE)
-      break;
-    }
-    default: {
-      NOTIMPLEMENTED();
-      break;
-    }
   }

[mcasas, 2015/12/01 15:30:38]

 l.500 should go after the closing bracket.

[nisse-chromium (ooo August 14), 2015/12/02 14:25:59]

Done.

 
   if (!video_capture_device) {
     device_client->OnError(FROM_HERE, "Could not create capture device");
     return nullptr;
   }
 
   video_capture_device->AllocateAndStart(params, device_client.Pass());
   return video_capture_device.Pass();
 }
 
@@ skipping 417 matching lines @@
   DCHECK(IsOnDeviceThread());
 #if defined(ENABLE_SCREEN_CAPTURE)
   DesktopCaptureDevice* desktop_device =
       static_cast<DesktopCaptureDevice*>(device);
   desktop_device->SetNotificationWindowId(window_id);
   VLOG(2) << "Screen capture notification window passed on device thread.";
 #endif
 }
 
 }  // namespace content