Video Capture Mojo (1.4b): Implement ability to use fake device instance

services/video_capture/video_capture_device_factory_impl.cc

Index: services/video_capture/video_capture_device_factory_impl.cc
diff --git a/services/video_capture/video_capture_device_factory_impl.cc b/services/video_capture/video_capture_device_factory_impl.cc
index f3ca7dd5b2c5d184a8fe3e66104637d626daae6f..90bed83a0365514bc3bf4d611956360e953374bf 100644
--- a/services/video_capture/video_capture_device_factory_impl.cc
+++ b/services/video_capture/video_capture_device_factory_impl.cc
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
+#include "media/capture/video/fake_video_capture_device.h"
 #include "services/video_capture/video_capture_device_factory_impl.h"
 
 namespace video_capture {
@@ -13,8 +14,11 @@ namespace video_capture {
 VideoCaptureDeviceFactoryImpl::DeviceEntry::DeviceEntry(
     mojom::VideoCaptureDeviceDescriptorPtr descriptor,
     std::unique_ptr<VideoCaptureDeviceProxyImpl> bindable_target)
-    : descriptor_(std::move(descriptor)),
-      device_proxy_(std::move(bindable_target)) {}
+    : descriptor_(std::move(descriptor)) {
+  binding_ = base::MakeUnique<mojo::Binding<mojom::VideoCaptureDeviceProxy>>(
+      bindable_target.get());
+  device_proxy_ = std::move(bindable_target);

[mcasas, 2016/08/31 21:10:35]

Move l.18-19 to the member initializer list.

[chfremer, 2016/08/31 21:49:43]

Done.

+}
 
 VideoCaptureDeviceFactoryImpl::DeviceEntry::~DeviceEntry() = default;
 
@@ -30,6 +34,24 @@ VideoCaptureDeviceFactoryImpl::DeviceEntry::MakeDescriptorCopy() const {
   return descriptor_.Clone();
 }
 
+bool VideoCaptureDeviceFactoryImpl::DeviceEntry::DescriptorEquals(
+    const mojom::VideoCaptureDeviceDescriptorPtr& other) const {
+  return descriptor_.Equals(other);
+}
+
+bool VideoCaptureDeviceFactoryImpl::DeviceEntry::is_bound() const {
+  return binding_->is_bound();
+}
+
+void VideoCaptureDeviceFactoryImpl::DeviceEntry::Bind(
+    mojom::VideoCaptureDeviceProxyRequest request) {
+  binding_->Bind(std::move(request));
+}
+
+void VideoCaptureDeviceFactoryImpl::DeviceEntry::Unbind() {
+  binding_->Unbind();
+}
+
 VideoCaptureDeviceFactoryImpl::VideoCaptureDeviceFactoryImpl() = default;
 
 VideoCaptureDeviceFactoryImpl::~VideoCaptureDeviceFactoryImpl() = default;
@@ -56,9 +78,18 @@ void VideoCaptureDeviceFactoryImpl::GetSupportedFormats(
 
 void VideoCaptureDeviceFactoryImpl::CreateDeviceProxy(
     mojom::VideoCaptureDeviceDescriptorPtr device_descriptor,
-    mojom::VideoCaptureDeviceProxyRequest request,
+    mojom::VideoCaptureDeviceProxyRequest proxy_request,
     const CreateDeviceProxyCallback& callback) {
-  callback.Run(mojom::DeviceAccessResultCode::SUCCESS);
+  for (auto& entry : devices_) {
+    if (entry.DescriptorEquals(device_descriptor)) {
+      if (entry.is_bound())
+        entry.Unbind();

[mcasas, 2016/08/31 21:10:35]

Question: if there is a VCDeviceProxy correctly bound, 
wouldn't this hit the CHECK in [1]? 

// This method may only be called if the object has been bound to a message
// pipe and there are no associated interfaces running.

If this were the case, we could not re-Bind() the Proxy 
and l.85-86 should just be a CHECK(!entry.is_bound());
which would be superfluous as is enforced by [1].


[1]
https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/binding.h?q=moj...

[chfremer, 2016/08/31 21:49:43]

I'll ping you to discuss about this.

[chfremer, 2016/08/31 22:30:52]

I got clarification from yzshen. "Associated interfaces" [1] is a Mojo concept I
didn't know yet, and we are not using it here. Therefore it is legal to call
Unbind() here.

The follow-up CL will also contain tests that verify that requesting a device
proxy for a second time behaves as expected.

[1]
https://www.chromium.org/developers/design-documents/mojo/associated-interfaces

+      entry.Bind(std::move(proxy_request));
+      callback.Run(mojom::DeviceAccessResultCode::SUCCESS);
+      return;
+    }
+  }
+  callback.Run(mojom::DeviceAccessResultCode::ERROR_DEVICE_NOT_FOUND);
 }
 
 }  // namespace video_capture