[Mojo Video Capture] Replace RESOURCE_UTILIZATION with interface ReceiverLoadObserver

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 14 matching lines @@
 #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"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
 #include "media/capture/video/video_capture_device.h"
+#include "media/capture/video/video_capture_device_client.h"
 #include "media/capture/video/video_capture_device_factory.h"
 
 #if defined(ENABLE_SCREEN_CAPTURE) && !defined(OS_ANDROID)
 #include "content/browser/media/capture/desktop_capture_device.h"
 #if defined(USE_AURA)
 #include "content/browser/media/capture/desktop_capture_device_aura.h"
 #endif
 #endif
 
 #if defined(ENABLE_SCREEN_CAPTURE) && defined(OS_ANDROID)
@@ skipping 78 matching lines @@
 
 // Counter used for identifying a DeviceRequest to start a capture device.
 static int g_device_start_id = 0;
 
 const media::VideoCaptureSessionId kFakeSessionId = -1;
 
 }  // namespace
 
 namespace content {
 
-// This class owns a pair VideoCaptureDevice - VideoCaptureController.
-// VideoCaptureManager owns all such pairs and is responsible for deleting the
-// instances when they are not used any longer.
+// Instances of this struct go through 3 different phases during their lifetime.
+// Phase 1: When first created (in GetOrCreateDeviceEntry()), this consists of a

[miu, 2016/12/01 05:25:18]

s/consists of a only a controller/consists of only a controller/

[chfremer, 2016/12/02 01:28:28]

Done.

+// only a controller. Clients can already connect to the controller, but there
+// is no device present.
+// Phase 2: When a request to "start" the entry comes in (via
+// HandleQueuedStartRequest()), a VideoCaptureDevice::Client is created
+// via video_capture_controller()->NewDeviceClient() and is used to schedule the
+// creation and start of a VideoCaptureDevice on the Device Thread.
+// Phase 3: As soon as the creation of the VideoCaptureDevice is complete, it
+// is connected to the VideoCaptureDevice::Client as the ConsumerLoadObserver.
 class VideoCaptureManager::DeviceEntry {
  public:
   DeviceEntry(MediaStreamType stream_type,
               const std::string& id,
               std::unique_ptr<VideoCaptureController> controller,
               const media::VideoCaptureParams& params);
   ~DeviceEntry();
 
   const int serial_id;
   const MediaStreamType stream_type;
@@ skipping 265 matching lines @@
       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()->OnLog(
       base::StringPrintf("Stopping device: id: %s", entry->id.c_str()));
+  entry->video_capture_controller()->SetFrameReceiverObserver(nullptr);
 
   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())));
   }
 }
 
@@ skipping 16 matching lines @@
   if (NeedToInitializeCaptureDeviceApi(entry->stream_type)) {
     InitializeCaptureDeviceApiOnUIThread(
         base::Bind(&VideoCaptureManager::HandleQueuedStartRequest, this));
     return;
   }
 #endif
 
   DVLOG(3) << "HandleQueuedStartRequest, Post start to device thread, device = "
            << entry->id << " start id = " << entry->serial_id;
 
+  std::unique_ptr<media::VideoCaptureDeviceClient> device_client =
+      entry->video_capture_controller()->NewDeviceClient();
+  media::VideoCaptureDeviceClient* device_client_ptr = device_client.get();
+
   base::Callback<std::unique_ptr<VideoCaptureDevice>(void)>
       start_capture_function;
 
   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 DeviceInfo* found = GetDeviceInfoById(entry->id);
       if (found) {
         entry->video_capture_controller()->OnLog(
             base::StringPrintf("Starting device: id: %s, name: %s, api: %s",
                                found->descriptor.device_id.c_str(),
                                found->descriptor.GetNameAndModel().c_str(),
                                found->descriptor.GetCaptureApiTypeString()));
 
-        start_capture_function = base::Bind(
-            &VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread, this,
-            found->descriptor, request->params(),
-            base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+        start_capture_function =
+            base::Bind(&VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread,
+                       this, found->descriptor, request->params(),
+                       base::Passed(std::move(device_client)));
       } 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()->OnLog(log_message);
         entry->video_capture_controller()->OnError();
         // Drop the failed start request.
         device_start_queue_.pop_front();
 
         return;
       }
       break;
     }
     case MEDIA_TAB_VIDEO_CAPTURE:
-      start_capture_function = base::Bind(
-          &VideoCaptureManager::DoStartTabCaptureOnDeviceThread, this,
-          entry->id, request->params(),
-          base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+      start_capture_function =
+          base::Bind(&VideoCaptureManager::DoStartTabCaptureOnDeviceThread,
+                     this, entry->id, request->params(),
+                     base::Passed(std::move(device_client)));
       break;
 
     case MEDIA_DESKTOP_VIDEO_CAPTURE:
-      start_capture_function = base::Bind(
-          &VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread, this,
-          entry->id, request->params(),
-          base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+      start_capture_function =
+          base::Bind(&VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread,
+                     this, entry->id, request->params(),
+                     base::Passed(std::move(device_client)));
       break;
 
     default: {
       NOTIMPLEMENTED();
       return;
     }
   }
   base::PostTaskAndReplyWithResult(
       device_task_runner_.get(), FROM_HERE, start_capture_function,
       base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
-                 request->serial_id()));
+                 request->serial_id(), device_client_ptr));

[miu, 2016/12/01 05:25:17]

Is this safe (passing the raw pointer)? Could, in some rare timings, the device
client be destroyed before the task runs? I ask because it looks like we're
going over to the device thread to start the capture device, transferring
ownership of the device client to that thread, and then we are hopping back to
the IO thread to call OnDeviceStarted().

[chfremer, 2016/12/02 01:28:28]

Yes, but that fact that you had to raise the question indicates that it was too
non-obvious why it worked. 

The good news is that with the simplifications enabled by your other
suggestions, we no longer need to do this. The controller can now talk directly
to the device instead of going through the device_client. 

 }
 
 void VideoCaptureManager::OnDeviceStarted(
     int serial_id,
+    media::VideoCaptureDeviceClient* device_client_ptr,
     std::unique_ptr<VideoCaptureDevice> device) {
   DVLOG(3) << __func__;
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK_EQ(serial_id, device_start_queue_.begin()->serial_id());
+  // |device| can be null if creation failed in
+  //  DoStartDeviceCaptureOnDeviceThread.
   if (device_start_queue_.front().abort_start()) {
-    // |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();
     }
   } else {
     DeviceEntry* const entry = GetDeviceEntryBySerialId(serial_id);
     DCHECK(entry);
     DCHECK(!entry->video_capture_device());
+    if (device) {
+      entry->video_capture_controller()->SetFrameReceiverObserver(
+          device_client_ptr);
+    }
     entry->SetVideoCaptureDevice(std::move(device));
 
     if (entry->stream_type == MEDIA_DESKTOP_VIDEO_CAPTURE) {
       const media::VideoCaptureSessionId session_id =
           device_start_queue_.front().session_id();
       DCHECK(session_id != kFakeSessionId);
       MaybePostDesktopCaptureWindowId(session_id);
     }
 
     auto it = photo_request_queue_.begin();
     while (it != photo_request_queue_.end()) {
       auto request = it++;
       DeviceEntry* maybe_entry = GetDeviceEntryBySessionId(request->first);
       if (maybe_entry && maybe_entry->video_capture_device()) {
         request->second.Run(maybe_entry->video_capture_device());
         photo_request_queue_.erase(request);
       }
     }
   }
 
   device_start_queue_.pop_front();
   HandleQueuedStartRequest();
 }
 
 std::unique_ptr<media::VideoCaptureDevice>
 VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread(
     const VideoCaptureDeviceDescriptor& descriptor,
     const media::VideoCaptureParams& params,
-    std::unique_ptr<VideoCaptureDevice::Client> device_client) {
+    std::unique_ptr<media::VideoCaptureDeviceClient> device_client) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(IsOnDeviceThread());
 
   std::unique_ptr<VideoCaptureDevice> video_capture_device;
   video_capture_device =
       video_capture_device_factory_->CreateDevice(descriptor);
 
   if (!video_capture_device) {
     device_client->OnError(FROM_HERE, "Could not create capture device");
     return nullptr;
   }
 
+  device_client->SetConsumerLoadObserver(video_capture_device.get());
   video_capture_device->AllocateAndStart(params, std::move(device_client));
   return video_capture_device;
 }
 
 std::unique_ptr<media::VideoCaptureDevice>
 VideoCaptureManager::DoStartTabCaptureOnDeviceThread(
     const std::string& id,
     const media::VideoCaptureParams& params,
-    std::unique_ptr<VideoCaptureDevice::Client> device_client) {
+    std::unique_ptr<media::VideoCaptureDeviceClient> device_client) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(IsOnDeviceThread());
 
   std::unique_ptr<VideoCaptureDevice> video_capture_device;
   video_capture_device = WebContentsVideoCaptureDevice::Create(id);
 
   if (!video_capture_device) {
     device_client->OnError(FROM_HERE, "Could not create capture device");
     return nullptr;
   }
 
+  device_client->SetConsumerLoadObserver(video_capture_device.get());

[miu, 2016/12/01 05:25:17]

OOC, should we call SetConsumerLoadObserver(nullptr) before StopAndDeallocate()?
Or, do we know that it's impossible for the load observer to have its method
called any time at or after that point? Could there be pending tasks that will
call the observer's method in some other thread's task queue?

[chfremer, 2016/12/02 01:28:28]

Again, this is no longer needed with the other simplifications.
I added the following comment to the place where the controller is connected to
the device:
    // Passing raw pointer |device.get()| to the controller is safe,
    // because we transfer ownership of it to |entry|. We are calling
    // SetConsumerFeedbackObserver(nullptr) before releasing
    // |entry->video_capture_device_| on the |device_task_runner_|.

   video_capture_device->AllocateAndStart(params, std::move(device_client));
   return video_capture_device;
 }
 
 std::unique_ptr<media::VideoCaptureDevice>
 VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread(
     const std::string& id,
     const media::VideoCaptureParams& params,
-    std::unique_ptr<VideoCaptureDevice::Client> device_client) {
+    std::unique_ptr<media::VideoCaptureDeviceClient> device_client) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(IsOnDeviceThread());
 
   std::unique_ptr<VideoCaptureDevice> video_capture_device;
 #if defined(ENABLE_SCREEN_CAPTURE)
   DesktopMediaID desktop_id = DesktopMediaID::Parse(id);
   if (desktop_id.is_null()) {
     device_client->OnError(FROM_HERE, "Desktop media ID is null");
     return nullptr;
   }
@@ skipping 17 matching lines @@
       video_capture_device = DesktopCaptureDevice::Create(desktop_id);
 #endif  // defined (OS_ANDROID)
   }
 #endif  // defined(ENABLE_SCREEN_CAPTURE)
 
   if (!video_capture_device) {
     device_client->OnError(FROM_HERE, "Could not create capture device");
     return nullptr;
   }
 
+  device_client->SetConsumerLoadObserver(video_capture_device.get());
   video_capture_device->AllocateAndStart(params, std::move(device_client));
   return video_capture_device;
 }
 
 void VideoCaptureManager::StartCaptureForClient(
     media::VideoCaptureSessionId session_id,
     const media::VideoCaptureParams& params,
     VideoCaptureControllerID client_id,
     VideoCaptureControllerEventHandler* client_handler,
     const DoneCB& done_cb) {
@@ skipping 625 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