[Video Capture Service] Keep VideoCaptureSystemImpl instance alive to avoid losing cached formats

The video capture service is designed to release resources and shut down when
not in use. However, this leads to information about supported formats of
the available devices, which is otherwise cached, having to be re-queried.
On some platforms, in particular on Android, querying these formats can take
a long time (i.e. several seconds).

This CL changes the video capture service such that it holds on to
the cached formats until the service is shut down. Before, this cache would
be released as soon as the last client has disconnected, even if
a client reconnects within the service shutdown timeout of 5 seconds.

This fixes increased delay of camera capture start observed
on Android.

Bug: 878304, 792621
Change-Id: I8279e73d093006cf46f64c3a8cb02591bd4f4a59
Reviewed-on: https://chromium-review.googlesource.com/c/1228776Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596010}

services/video_capture/device_factory_media_to_mojo_adapter.cc

@@ -77,12 +77,10 @@ DeviceFactoryMediaToMojoAdapter::ActiveDeviceEntry::operator=(
     DeviceFactoryMediaToMojoAdapter::ActiveDeviceEntry&& other) = default;
 
 DeviceFactoryMediaToMojoAdapter::DeviceFactoryMediaToMojoAdapter(
-    std::unique_ptr<service_manager::ServiceContextRef> service_ref,
     std::unique_ptr<media::VideoCaptureSystem> capture_system,
     media::MojoJpegDecodeAcceleratorFactoryCB jpeg_decoder_factory_callback,
     scoped_refptr<base::SequencedTaskRunner> jpeg_decoder_task_runner)
-    : service_ref_(std::move(service_ref)),
-      capture_system_(std::move(capture_system)),
+    : capture_system_(std::move(capture_system)),
       jpeg_decoder_factory_callback_(std::move(jpeg_decoder_factory_callback)),
       jpeg_decoder_task_runner_(std::move(jpeg_decoder_task_runner)),
       has_called_get_device_infos_(false),
@@ -90,6 +88,11 @@ DeviceFactoryMediaToMojoAdapter::DeviceFactoryMediaToMojoAdapter(
 
 DeviceFactoryMediaToMojoAdapter::~DeviceFactoryMediaToMojoAdapter() = default;
 
+void DeviceFactoryMediaToMojoAdapter::SetServiceRef(
+    std::unique_ptr<service_manager::ServiceContextRef> service_ref) {
+  service_ref_ = std::move(service_ref);
+}
+
 void DeviceFactoryMediaToMojoAdapter::GetDeviceInfos(
     GetDeviceInfosCallback callback) {
   capture_system_->GetDeviceInfosAsync(
@@ -129,6 +132,7 @@ void DeviceFactoryMediaToMojoAdapter::CreateDevice(
   capture_system_->GetDeviceInfosAsync(
       base::Bind(&DiscardDeviceInfosAndCallContinuation,
                  base::Passed(&create_and_add_new_device_cb)));
+  has_called_get_device_infos_ = true;
 }
 
 void DeviceFactoryMediaToMojoAdapter::AddSharedMemoryVirtualDevice(
@@ -154,6 +158,7 @@ void DeviceFactoryMediaToMojoAdapter::CreateAndAddNewDevice(
     const std::string& device_id,
     mojom::DeviceRequest device_request,
     CreateDeviceCallback callback) {
+  DCHECK(service_ref_);
   std::unique_ptr<media::VideoCaptureDevice> media_device =
       capture_system_->CreateDevice(device_id);
   if (media_device == nullptr) {

services/video_capture/device_factory_media_to_mojo_adapter.h

@@ -24,12 +24,14 @@ class DeviceMediaToMojoAdapter;
 class DeviceFactoryMediaToMojoAdapter : public mojom::DeviceFactory {
  public:
   DeviceFactoryMediaToMojoAdapter(
-      std::unique_ptr<service_manager::ServiceContextRef> service_ref,
       std::unique_ptr<media::VideoCaptureSystem> capture_system,
       media::MojoJpegDecodeAcceleratorFactoryCB jpeg_decoder_factory_callback,
       scoped_refptr<base::SequencedTaskRunner> jpeg_decoder_task_runner);
   ~DeviceFactoryMediaToMojoAdapter() override;
 
+  void SetServiceRef(
+      std::unique_ptr<service_manager::ServiceContextRef> service_ref);
+
   // mojom::DeviceFactory implementation.
   void GetDeviceInfos(GetDeviceInfosCallback callback) override;
   void CreateDevice(const std::string& device_id,
@@ -65,7 +67,7 @@ class DeviceFactoryMediaToMojoAdapter : public mojom::DeviceFactory {
                              CreateDeviceCallback callback);
   void OnClientConnectionErrorOrClose(const std::string& device_id);
 
-  const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
+  std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
   const std::unique_ptr<media::VideoCaptureSystem> capture_system_;
   const media::MojoJpegDecodeAcceleratorFactoryCB
       jpeg_decoder_factory_callback_;

services/video_capture/device_factory_provider_impl.cc

@@ -67,9 +67,13 @@ class DeviceFactoryProviderImpl::GpuDependenciesContext {
   base::WeakPtrFactory<GpuDependenciesContext> weak_factory_for_gpu_io_thread_;
 };
 
-DeviceFactoryProviderImpl::DeviceFactoryProviderImpl(
-    std::unique_ptr<service_manager::ServiceContextRef> service_ref)
-    : service_ref_(std::move(service_ref)) {}
+DeviceFactoryProviderImpl::DeviceFactoryProviderImpl() {
+  // Unretained |this| is safe because |factory_bindings_| is owned by
+  // |this|.
+  factory_bindings_.set_connection_error_handler(base::BindRepeating(
+      &DeviceFactoryProviderImpl::OnFactoryClientDisconnected,
+      base::Unretained(this)));
+}
 
 DeviceFactoryProviderImpl::~DeviceFactoryProviderImpl() {
   factory_bindings_.CloseAllBindings();
@@ -80,6 +84,11 @@ DeviceFactoryProviderImpl::~DeviceFactoryProviderImpl() {
   }
 }
 
+void DeviceFactoryProviderImpl::SetServiceRef(
+    std::unique_ptr<service_manager::ServiceContextRef> service_ref) {
+  service_ref_ = std::move(service_ref);
+}
+
 void DeviceFactoryProviderImpl::InjectGpuDependencies(
     mojom::AcceleratorFactoryPtr accelerator_factory) {
   LazyInitializeGpuDependenciesContext();
@@ -91,7 +100,10 @@ void DeviceFactoryProviderImpl::InjectGpuDependencies(
 
 void DeviceFactoryProviderImpl::ConnectToDeviceFactory(
     mojom::DeviceFactoryRequest request) {
+  DCHECK(service_ref_);
   LazyInitializeDeviceFactory();
+  if (factory_bindings_.empty())
+    device_factory_->SetServiceRef(service_ref_->Clone());
   factory_bindings_.AddBinding(device_factory_.get(), std::move(request));
 }
 
@@ -119,13 +131,22 @@ void DeviceFactoryProviderImpl::LazyInitializeDeviceFactory() {
       std::move(media_device_factory));
 
   device_factory_ = std::make_unique<VirtualDeviceEnabledDeviceFactory>(
-      service_ref_->Clone(),
       std::make_unique<DeviceFactoryMediaToMojoAdapter>(
-          service_ref_->Clone(), std::move(video_capture_system),
+          std::move(video_capture_system),
           base::BindRepeating(
               &GpuDependenciesContext::CreateJpegDecodeAccelerator,
               gpu_dependencies_context_->GetWeakPtr()),
           gpu_dependencies_context_->GetTaskRunner()));
 }
 
+void DeviceFactoryProviderImpl::OnFactoryClientDisconnected() {
+  // If last client has disconnected, release service ref so that service
+  // shutdown timeout starts if no other references are still alive.
+  // We keep the |device_factory_| instance alive in order to avoid
+  // losing state that would be expensive to reinitialize, e.g. having
+  // already enumerated the available devices.
+  if (factory_bindings_.empty())
+    device_factory_->SetServiceRef(nullptr);
+}
+
 }  // namespace video_capture

services/video_capture/device_factory_provider_impl.h

@@ -17,12 +17,16 @@
 
 namespace video_capture {
 
+class VirtualDeviceEnabledDeviceFactory;
+
 class DeviceFactoryProviderImpl : public mojom::DeviceFactoryProvider {
  public:
-  DeviceFactoryProviderImpl(
-      std::unique_ptr<service_manager::ServiceContextRef> service_ref);
+  DeviceFactoryProviderImpl();
   ~DeviceFactoryProviderImpl() override;
 
+  void SetServiceRef(
+      std::unique_ptr<service_manager::ServiceContextRef> service_ref);
+
   // mojom::DeviceFactoryProvider implementation.
   void InjectGpuDependencies(
       mojom::AcceleratorFactoryPtr accelerator_factory) override;
@@ -33,10 +37,11 @@ class DeviceFactoryProviderImpl : public mojom::DeviceFactoryProvider {
 
   void LazyInitializeGpuDependenciesContext();
   void LazyInitializeDeviceFactory();
+  void OnFactoryClientDisconnected();
 
   mojo::BindingSet<mojom::DeviceFactory> factory_bindings_;
-  std::unique_ptr<mojom::DeviceFactory> device_factory_;
-  const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
+  std::unique_ptr<VirtualDeviceEnabledDeviceFactory> device_factory_;
+  std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
   std::unique_ptr<GpuDependenciesContext> gpu_dependencies_context_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceFactoryProviderImpl);

services/video_capture/service_impl.cc

@@ -79,6 +79,8 @@ void ServiceImpl::OnStart() {
       base::Bind(&ServiceImpl::OnTestingControlsRequest,
                  base::Unretained(this)));
 
+  // Unretained |this| is safe because |factory_provider_bindings_| is owned by
+  // |this|.
   factory_provider_bindings_.set_connection_error_handler(base::BindRepeating(
       &ServiceImpl::OnProviderClientDisconnected, base::Unretained(this)));
 }
@@ -112,6 +114,8 @@ void ServiceImpl::OnDeviceFactoryProviderRequest(
     mojom::DeviceFactoryProviderRequest request) {
   DCHECK(thread_checker_.CalledOnValidThread());
   LazyInitializeDeviceFactoryProvider();
+  if (factory_provider_bindings_.empty())
+    device_factory_provider_->SetServiceRef(ref_factory_->CreateRef());
   factory_provider_bindings_.AddBinding(device_factory_provider_.get(),
                                         std::move(request));
 
@@ -132,15 +136,17 @@ void ServiceImpl::LazyInitializeDeviceFactoryProvider() {
   if (device_factory_provider_)
     return;
 
-  device_factory_provider_ =
-      std::make_unique<DeviceFactoryProviderImpl>(ref_factory_->CreateRef());
+  device_factory_provider_ = std::make_unique<DeviceFactoryProviderImpl>();
 }
 
 void ServiceImpl::OnProviderClientDisconnected() {
-  // Reset factory provider if no client is connected.
-  if (factory_provider_bindings_.empty()) {
-    device_factory_provider_.reset();
-  }
+  // If last client has disconnected, release service ref so that service
+  // shutdown timeout starts if no other references are still alive.
+  // We keep the |device_factory_provider_| instance alive in order to avoid
+  // losing state that would be expensive to reinitialize, e.g. having
+  // already enumerated the available devices.
+  if (factory_provider_bindings_.empty())
+    device_factory_provider_->SetServiceRef(nullptr);
 
   if (!factory_provider_client_disconnected_cb_.is_null()) {
     factory_provider_client_disconnected_cb_.Run();

services/video_capture/test/mock_device_test.cc

@@ -31,8 +31,9 @@ void MockDeviceTest::SetUp() {
       std::move(mock_device_factory));
   mock_device_factory_adapter_ =
       std::make_unique<DeviceFactoryMediaToMojoAdapter>(
-          ref_factory_.CreateRef(), std::move(video_capture_system),
-          base::DoNothing(), base::ThreadTaskRunnerHandle::Get());
+          std::move(video_capture_system), base::DoNothing(),
+          base::ThreadTaskRunnerHandle::Get());
+  mock_device_factory_adapter_->SetServiceRef(ref_factory_.CreateRef());
 
   mock_factory_binding_ = std::make_unique<mojo::Binding<mojom::DeviceFactory>>(
       mock_device_factory_adapter_.get(), mojo::MakeRequest(&factory_));

services/video_capture/virtual_device_enabled_device_factory.cc

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "media/capture/video/video_capture_device_info.h"
+#include "services/video_capture/device_factory_media_to_mojo_adapter.h"
 #include "services/video_capture/shared_memory_virtual_device_mojo_adapter.h"
 #include "services/video_capture/texture_virtual_device_mojo_adapter.h"
 
@@ -84,15 +85,21 @@ class VirtualDeviceEnabledDeviceFactory::VirtualDeviceEntry {
 };
 
 VirtualDeviceEnabledDeviceFactory::VirtualDeviceEnabledDeviceFactory(
-    std::unique_ptr<service_manager::ServiceContextRef> service_ref,
-    std::unique_ptr<mojom::DeviceFactory> device_factory)
-    : service_ref_(std::move(service_ref)),
-      device_factory_(std::move(device_factory)),
-      weak_factory_(this) {}
+    std::unique_ptr<DeviceFactoryMediaToMojoAdapter> device_factory)
+    : device_factory_(std::move(device_factory)), weak_factory_(this) {}
 
 VirtualDeviceEnabledDeviceFactory::~VirtualDeviceEnabledDeviceFactory() =
     default;
 
+void VirtualDeviceEnabledDeviceFactory::SetServiceRef(
+    std::unique_ptr<service_manager::ServiceContextRef> service_ref) {
+  if (service_ref)
+    device_factory_->SetServiceRef(service_ref->Clone());
+  else
+    device_factory_->SetServiceRef(nullptr);
+  service_ref_ = std::move(service_ref);
+}
+
 void VirtualDeviceEnabledDeviceFactory::GetDeviceInfos(
     GetDeviceInfosCallback callback) {
   device_factory_->GetDeviceInfos(

services/video_capture/virtual_device_enabled_device_factory.h

@@ -15,15 +15,19 @@
 
 namespace video_capture {
 
+class DeviceFactoryMediaToMojoAdapter;
+
 // Decorator that adds support for virtual devices to a given
 // mojom::DeviceFactory.
 class VirtualDeviceEnabledDeviceFactory : public mojom::DeviceFactory {
  public:
-  VirtualDeviceEnabledDeviceFactory(
-      std::unique_ptr<service_manager::ServiceContextRef> service_ref,
-      std::unique_ptr<mojom::DeviceFactory> factory);
+  explicit VirtualDeviceEnabledDeviceFactory(
+      std::unique_ptr<DeviceFactoryMediaToMojoAdapter> factory);
   ~VirtualDeviceEnabledDeviceFactory() override;
 
+  void SetServiceRef(
+      std::unique_ptr<service_manager::ServiceContextRef> service_ref);
+
   // mojom::DeviceFactory implementation.
   void GetDeviceInfos(GetDeviceInfosCallback callback) override;
   void CreateDevice(const std::string& device_id,
@@ -56,8 +60,8 @@ class VirtualDeviceEnabledDeviceFactory : public mojom::DeviceFactory {
       mojom::DevicesChangedObserverPtr* observer);
 
   std::map<std::string, VirtualDeviceEntry> virtual_devices_by_id_;
-  const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
-  const std::unique_ptr<mojom::DeviceFactory> device_factory_;
+  const std::unique_ptr<DeviceFactoryMediaToMojoAdapter> device_factory_;
+  std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
   std::vector<mojom::DevicesChangedObserverPtr> devices_changed_observers_;
 
   base::WeakPtrFactory<VirtualDeviceEnabledDeviceFactory> weak_factory_;