Skip to content

Commit

Permalink
[Mojo Video Capture] Stop service when last client disconnects.
Browse files Browse the repository at this point in the history
This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL23.

Purpose of this CL:
Stop service when last client disconnects. Once we host it in a separate process, this will allow the service to automatically restart when a new request comes in.

Changes in this CL:
* Split class video_capture::ServiceImpl into two classes
  ServiceManagerServiceImpl and VideoCaptureServiceImpl.
* In class ServiceManagerServiceImpl, following the example of ImageDecoder [2],
  use a ServiceContextRefFactory to manage service lifetime and implement a
  delayed service shutdown when there are no clients.
* Add tests ServiceQuitsWhenNoClientConnected and
  ServiceQuitsWhenClientDisconnectsWhileUsingDevice
* In service.mojom, add a method for setting the service shutdown delay.
  For the automated tests, set the delay to zero.
* In DeviceFactoryMediaToMojoAdapter, make sure that
  |capture_system_->GetDeviceInfosAsync()| is called before
  |capture_system_->CreateDevice()|. This is required by the implementation
  VideoCaptureSystemImpl in order to fill the cache |devices_info_cache_|.
* In DeviceMediaToMojoAdapter, unsubscibe from
  |receiver.set_connection_error_handler()| in order to avoid use after free
  on service shutdown.
* In ReceiverMojoToMediaAdapter, introduce class ReceiverOnTaskRunner in order
  to ensure that mojom.Receiver methods are posted to the correct thread.
* In VideoCaptureDeviceClient, allow |external_jpeg_decoder_| to be nullptr,
  since in the context of the video_capture service, this will (for now) be the
  case.

BUG=584797
TEST=
  service_unittests --gtest_filter="*Video*"
  content_unittests --gtest_filter="*Video*"
  content_browsertests --gtest_filter="VideoCaptureBrowserTest.*"

[1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing
[2] https://cs.chromium.org/chromium/src/services/data_decoder/data_decoder_service.cc?dr=CSs

Review-Url: https://codereview.chromium.org/2824883005
Cr-Commit-Position: refs/heads/master@{#468358}
  • Loading branch information
chfremer authored and Commit bot committed May 1, 2017
1 parent ced3697 commit 9fe5b74
Show file tree
Hide file tree
Showing 29 changed files with 634 additions and 183 deletions.
3 changes: 2 additions & 1 deletion media/capture/video/video_capture_device_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ void VideoCaptureDeviceClient::OnIncomingCapturedData(
!external_jpeg_decoder_initialized_) {
external_jpeg_decoder_initialized_ = true;
external_jpeg_decoder_ = jpeg_decoder_factory_callback_.Run();
external_jpeg_decoder_->Initialize();
if (external_jpeg_decoder_)
external_jpeg_decoder_->Initialize();
}
}

Expand Down
2 changes: 2 additions & 0 deletions media/capture/video/video_capture_system.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

namespace media {

// GetDeviceInfosAsync() should be called at least once before calling
// CreateDevice(), because otherwise CreateDevice() will allways return nullptr.
class CAPTURE_EXPORT VideoCaptureSystem {
public:
using DeviceInfoCallback =
Expand Down
24 changes: 15 additions & 9 deletions services/video_capture/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import("//testing/test.gni")

service("video_capture") {
sources = [
"service_impl.cc",
"service_impl.h",
"service_main.cc",
]

deps = [
":lib",
"//mojo/public/cpp/system",
"//services/video_capture/public/cpp",
"//services/video_capture/public/interfaces",
]
}

Expand All @@ -29,14 +29,14 @@ source_set("lib") {
sources = [
"device_factory_media_to_mojo_adapter.cc",
"device_factory_media_to_mojo_adapter.h",
"device_factory_provider_impl.cc",
"device_factory_provider_impl.h",
"device_media_to_mojo_adapter.cc",
"device_media_to_mojo_adapter.h",
"public/cpp/device_to_feedback_observer_adapter.cc",
"public/cpp/device_to_feedback_observer_adapter.h",
"public/cpp/receiver_media_to_mojo_adapter.cc",
"public/cpp/receiver_media_to_mojo_adapter.h",
"receiver_mojo_to_media_adapter.cc",
"receiver_mojo_to_media_adapter.h",
"service_impl.cc",
"service_impl.h",
]

public_deps = [
Expand All @@ -47,14 +47,23 @@ source_set("lib") {
"//media/mojo/common:common",
"//mojo/common:common_base",
"//services/service_manager/public/cpp",
"//services/video_capture/public/cpp",
"//services/video_capture/public/interfaces",
"//services/video_capture/public/interfaces:constants",
]

data_deps = [
":manifest",
]
}

source_set("tests") {
testonly = true

sources = [
"test/device_factory_provider_test.cc",
"test/device_factory_provider_test.h",
"test/device_factory_provider_unittest.cc",
"test/fake_device_descriptor_test.cc",
"test/fake_device_descriptor_test.h",
"test/fake_device_descriptor_unittest.cc",
Expand All @@ -68,9 +77,6 @@ source_set("tests") {
"test/mock_device_unittest.cc",
"test/mock_receiver.cc",
"test/mock_receiver.h",
"test/service_test.cc",
"test/service_test.h",
"test/service_unittest.cc",
]

deps = [
Expand Down
36 changes: 33 additions & 3 deletions services/video_capture/device_factory_media_to_mojo_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ static void TranslateDeviceInfos(
callback.Run(translated_device_infos);
}

static void DiscardDeviceInfosAndCallContinuation(
base::Closure continuation,
const std::vector<media::VideoCaptureDeviceInfo>&) {
continuation.Run();
}

} // anonymous namespace

namespace video_capture {
Expand All @@ -79,18 +85,23 @@ DeviceFactoryMediaToMojoAdapter::ActiveDeviceEntry::operator=(
DeviceFactoryMediaToMojoAdapter::ActiveDeviceEntry&& other) = default;

DeviceFactoryMediaToMojoAdapter::DeviceFactoryMediaToMojoAdapter(
std::unique_ptr<service_manager::ServiceContextRef> service_ref,
std::unique_ptr<media::VideoCaptureSystem> capture_system,
const media::VideoCaptureJpegDecoderFactoryCB&
jpeg_decoder_factory_callback)
: capture_system_(std::move(capture_system)),
jpeg_decoder_factory_callback_(jpeg_decoder_factory_callback) {}
: service_ref_(std::move(service_ref)),
capture_system_(std::move(capture_system)),
jpeg_decoder_factory_callback_(jpeg_decoder_factory_callback),
has_called_get_device_infos_(false),
weak_factory_(this) {}

DeviceFactoryMediaToMojoAdapter::~DeviceFactoryMediaToMojoAdapter() = default;

void DeviceFactoryMediaToMojoAdapter::GetDeviceInfos(
const GetDeviceInfosCallback& callback) {
capture_system_->GetDeviceInfosAsync(
base::Bind(&TranslateDeviceInfos, callback));
has_called_get_device_infos_ = true;
}

void DeviceFactoryMediaToMojoAdapter::CreateDevice(
Expand All @@ -112,6 +123,24 @@ void DeviceFactoryMediaToMojoAdapter::CreateDevice(
return;
}

const auto create_and_add_new_device_cb =
base::Bind(&DeviceFactoryMediaToMojoAdapter::CreateAndAddNewDevice,
weak_factory_.GetWeakPtr(), device_id,
base::Passed(&device_request), callback);

if (has_called_get_device_infos_) {
create_and_add_new_device_cb.Run();
return;
}

capture_system_->GetDeviceInfosAsync(base::Bind(
&DiscardDeviceInfosAndCallContinuation, create_and_add_new_device_cb));
}

void DeviceFactoryMediaToMojoAdapter::CreateAndAddNewDevice(
const std::string& device_id,
mojom::DeviceRequest device_request,
const CreateDeviceCallback& callback) {
std::unique_ptr<media::VideoCaptureDevice> media_device =
capture_system_->CreateDevice(device_id);
if (media_device == nullptr) {
Expand All @@ -122,7 +151,8 @@ void DeviceFactoryMediaToMojoAdapter::CreateDevice(
// Add entry to active_devices to keep track of it
ActiveDeviceEntry device_entry;
device_entry.device = base::MakeUnique<DeviceMediaToMojoAdapter>(
std::move(media_device), jpeg_decoder_factory_callback_);
service_ref_->Clone(), std::move(media_device),
jpeg_decoder_factory_callback_);
device_entry.binding = base::MakeUnique<mojo::Binding<mojom::Device>>(
device_entry.device.get(), std::move(device_request));
device_entry.binding->set_connection_error_handler(base::Bind(
Expand Down
13 changes: 12 additions & 1 deletion services/video_capture/device_factory_media_to_mojo_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "media/capture/video/video_capture_device_client.h"
#include "media/capture/video/video_capture_system.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "services/service_manager/public/cpp/service_context_ref.h"
#include "services/video_capture/public/interfaces/device_factory.mojom.h"

namespace video_capture {
Expand All @@ -23,12 +24,13 @@ class DeviceMediaToMojoAdapter;
class DeviceFactoryMediaToMojoAdapter : public mojom::DeviceFactory {
public:
DeviceFactoryMediaToMojoAdapter(
std::unique_ptr<service_manager::ServiceContextRef> service_ref,
std::unique_ptr<media::VideoCaptureSystem> capture_system,
const media::VideoCaptureJpegDecoderFactoryCB&
jpeg_decoder_factory_callback);
~DeviceFactoryMediaToMojoAdapter() override;

// mojom::DeviceFactory:
// mojom::DeviceFactory implementation.
void GetDeviceInfos(const GetDeviceInfosCallback& callback) override;
void CreateDevice(const std::string& device_id,
mojom::DeviceRequest device_request,
Expand All @@ -48,11 +50,20 @@ class DeviceFactoryMediaToMojoAdapter : public mojom::DeviceFactory {
std::unique_ptr<mojo::Binding<mojom::Device>> binding;
};

void CreateAndAddNewDevice(const std::string& device_id,
mojom::DeviceRequest device_request,
const CreateDeviceCallback& callback);
void OnClientConnectionErrorOrClose(const std::string& device_id);

const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
const std::unique_ptr<media::VideoCaptureSystem> capture_system_;
const media::VideoCaptureJpegDecoderFactoryCB jpeg_decoder_factory_callback_;
std::map<std::string, ActiveDeviceEntry> active_devices_by_id_;
bool has_called_get_device_infos_;

base::WeakPtrFactory<DeviceFactoryMediaToMojoAdapter> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(DeviceFactoryMediaToMojoAdapter);
};

} // namespace video_capture
Expand Down
85 changes: 85 additions & 0 deletions services/video_capture/device_factory_provider_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2017 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 "services/video_capture/device_factory_provider_impl.h"

#include "base/memory/ptr_util.h"
#include "media/capture/video/fake_video_capture_device_factory.h"
#include "media/capture/video/video_capture_buffer_pool.h"
#include "media/capture/video/video_capture_buffer_tracker.h"
#include "media/capture/video/video_capture_jpeg_decoder.h"
#include "media/capture/video/video_capture_system_impl.h"
#include "services/service_manager/public/cpp/service_info.h"
#include "services/video_capture/device_factory_media_to_mojo_adapter.h"

namespace {

// TODO(chfremer): Replace with an actual decoder factory.
// https://crbug.com/584797
std::unique_ptr<media::VideoCaptureJpegDecoder> CreateJpegDecoder() {
return nullptr;
}

} // anonymous namespace

namespace video_capture {

DeviceFactoryProviderImpl::DeviceFactoryProviderImpl(
std::unique_ptr<service_manager::ServiceContextRef> service_ref,
base::Callback<void(float)> set_shutdown_delay_cb)
: service_ref_(std::move(service_ref)),
set_shutdown_delay_cb_(std::move(set_shutdown_delay_cb)) {}

DeviceFactoryProviderImpl::~DeviceFactoryProviderImpl() {}

void DeviceFactoryProviderImpl::ConnectToDeviceFactory(
mojom::DeviceFactoryRequest request) {
LazyInitializeDeviceFactory();
factory_bindings_.AddBinding(device_factory_.get(), std::move(request));
}

void DeviceFactoryProviderImpl::ConnectToFakeDeviceFactory(
mojom::DeviceFactoryRequest request) {
LazyInitializeFakeDeviceFactory();
fake_factory_bindings_.AddBinding(fake_device_factory_.get(),
std::move(request));
}

void DeviceFactoryProviderImpl::SetShutdownDelayInSeconds(float seconds) {
set_shutdown_delay_cb_.Run(seconds);
}

void DeviceFactoryProviderImpl::LazyInitializeDeviceFactory() {
if (device_factory_)
return;

// Create the platform-specific device factory.
// The task runner passed to CreateFactory is used for things that need to
// happen on a "UI thread equivalent", e.g. obtaining screen rotation on
// Chrome OS.
std::unique_ptr<media::VideoCaptureDeviceFactory> media_device_factory =
media::VideoCaptureDeviceFactory::CreateFactory(
base::ThreadTaskRunnerHandle::Get());
auto video_capture_system = base::MakeUnique<media::VideoCaptureSystemImpl>(
std::move(media_device_factory));

device_factory_ = base::MakeUnique<DeviceFactoryMediaToMojoAdapter>(
service_ref_->Clone(), std::move(video_capture_system),
base::Bind(CreateJpegDecoder));
}

void DeviceFactoryProviderImpl::LazyInitializeFakeDeviceFactory() {
if (fake_device_factory_)
return;

auto factory = base::MakeUnique<media::FakeVideoCaptureDeviceFactory>();
auto video_capture_system =
base::MakeUnique<media::VideoCaptureSystemImpl>(std::move(factory));

fake_device_factory_ = base::MakeUnique<DeviceFactoryMediaToMojoAdapter>(
service_ref_->Clone(), std::move(video_capture_system),
base::Bind(&CreateJpegDecoder));
}

} // namespace video_capture
49 changes: 49 additions & 0 deletions services/video_capture/device_factory_provider_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2017 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.

#ifndef SERVICES_VIDEO_CAPTURE_DEVICE_FACTORY_PROVIDER_H_
#define SERVICES_VIDEO_CAPTURE_DEVICE_FACTORY_PROVIDER_H_

#include <memory>

#include "mojo/public/cpp/bindings/binding_set.h"
#include "services/service_manager/public/cpp/interface_factory.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_context_ref.h"
#include "services/video_capture/public/interfaces/device_factory_provider.mojom.h"

namespace video_capture {

class DeviceFactoryMediaToMojoAdapter;

class DeviceFactoryProviderImpl : public mojom::DeviceFactoryProvider {
public:
DeviceFactoryProviderImpl(
std::unique_ptr<service_manager::ServiceContextRef> service_ref,
base::Callback<void(float)> set_shutdown_delay_cb);
~DeviceFactoryProviderImpl() override;

// mojom::DeviceFactoryProvider implementation.
void ConnectToDeviceFactory(mojom::DeviceFactoryRequest request) override;
void ConnectToFakeDeviceFactory(mojom::DeviceFactoryRequest request) override;
void SetShutdownDelayInSeconds(float seconds) override;

private:
void LazyInitializeDeviceFactory();
void LazyInitializeFakeDeviceFactory();

mojo::BindingSet<mojom::DeviceFactory> factory_bindings_;
mojo::BindingSet<mojom::DeviceFactory> fake_factory_bindings_;
std::unique_ptr<DeviceFactoryMediaToMojoAdapter> device_factory_;
std::unique_ptr<DeviceFactoryMediaToMojoAdapter> fake_device_factory_;

const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
base::Callback<void(float)> set_shutdown_delay_cb_;

DISALLOW_COPY_AND_ASSIGN(DeviceFactoryProviderImpl);
};

} // namespace video_capture

#endif // SERVICES_VIDEO_CAPTURE_DEVICE_FACTORY_PROVIDER_H_
Loading

0 comments on commit 9fe5b74

Please sign in to comment.