Skip to content

Commit

Permalink
Migrate references to the PushVideoStreamSubscription interface
Browse files Browse the repository at this point in the history
Convert the remaining bits referencing the PushVideoStreamSubscription
mojo interface (from video_capture service) using the old APIs to the
new mojo types, and adapt unit tests.

Bug: 955171
Change-Id: Ic1b588835553340a22272d3f4ac43b79ce3f0ef6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1886614
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711310}
  • Loading branch information
mariospr authored and Commit Bot committed Oct 31, 2019
1 parent 24c14ee commit 12a04c9
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ namespace content {

ServiceLaunchedVideoCaptureDevice::ServiceLaunchedVideoCaptureDevice(
video_capture::mojom::VideoSourcePtr source,
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription,
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription>
subscription,
base::OnceClosure connection_lost_cb)
: source_(std::move(source)),
subscription_(std::move(subscription)),
Expand All @@ -24,7 +25,7 @@ ServiceLaunchedVideoCaptureDevice::ServiceLaunchedVideoCaptureDevice(
OnLostConnectionToSourceOrSubscription,
base::Unretained(this)));
// Unretained |this| is safe, because |this| owns |subscription_|.
subscription_.set_connection_error_handler(
subscription_.set_disconnect_handler(
base::BindOnce(&ServiceLaunchedVideoCaptureDevice::
OnLostConnectionToSourceOrSubscription,
base::Unretained(this)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "content/browser/renderer_host/media/video_capture_provider.h"
#include "content/public/browser/video_capture_device_launcher.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/video_capture/public/mojom/video_source.mojom.h"

namespace content {
Expand All @@ -17,7 +18,8 @@ class ServiceLaunchedVideoCaptureDevice : public LaunchedVideoCaptureDevice {
public:
ServiceLaunchedVideoCaptureDevice(
video_capture::mojom::VideoSourcePtr source,
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription,
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription>
subscription,
base::OnceClosure connection_lost_cb);
~ServiceLaunchedVideoCaptureDevice() override;

Expand Down Expand Up @@ -51,7 +53,7 @@ class ServiceLaunchedVideoCaptureDevice : public LaunchedVideoCaptureDevice {
media::mojom::BlobPtr blob);

video_capture::mojom::VideoSourcePtr source_;
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription_;
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription> subscription_;
base::OnceClosure connection_lost_cb_;
base::SequenceChecker sequence_checker_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ namespace {

void ConcludeLaunchDeviceWithSuccess(
video_capture::mojom::VideoSourcePtr source,
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription,
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription>
subscription,
base::OnceClosure connection_lost_cb,
VideoCaptureDeviceLauncher::Callbacks* callbacks,
base::OnceClosure done_cb) {
Expand Down Expand Up @@ -122,13 +123,13 @@ void ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(
std::move(receiver_adapter),
pending_remote_proxy.InitWithNewPipeAndPassReceiver());

video_capture::mojom::PushVideoStreamSubscriptionPtr subscription;
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription> subscription;
// Create message pipe so that we can subsequently call
// subscription.set_connection_error_handler().
auto subscription_request = mojo::MakeRequest(&subscription);
// subscription.set_disconnect_handler().
auto subscription_receiver = subscription.BindNewPipeAndPassReceiver();
// Use of Unretained(this) is safe, because |done_cb_| guarantees that |this|
// stays alive.
subscription.set_connection_error_handler(
subscription.set_disconnect_handler(
base::BindOnce(&ServiceVideoCaptureDeviceLauncher::
OnConnectionLostWhileWaitingForCallback,
base::Unretained(this)));
Expand All @@ -155,7 +156,7 @@ void ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(
// service indicating that the device closing is complete.
source->CreatePushSubscription(
std::move(pending_remote_proxy), new_params,
true /*force_reopen_with_new_settings*/, std::move(subscription_request),
true /*force_reopen_with_new_settings*/, std::move(subscription_receiver),
base::BindOnce(
// Use of Unretained |this| is safe, because |done_cb_| guarantees
// that |this| stays alive.
Expand All @@ -173,14 +174,15 @@ void ServiceVideoCaptureDeviceLauncher::AbortLaunch() {

void ServiceVideoCaptureDeviceLauncher::OnCreatePushSubscriptionCallback(
video_capture::mojom::VideoSourcePtr source,
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription,
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription>
subscription,
base::OnceClosure connection_lost_cb,
video_capture::mojom::CreatePushSubscriptionResultCode result_code,
const media::VideoCaptureParams& params) {
DCHECK(sequence_checker_.CalledOnValidSequence());
DCHECK(callbacks_);
DCHECK(done_cb_);
subscription.set_connection_error_handler(base::DoNothing());
subscription.set_disconnect_handler(base::DoNothing());
const bool abort_requested = (state_ == State::DEVICE_START_ABORTING);
state_ = State::READY_TO_LAUNCH;
Callbacks* callbacks = callbacks_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "content/browser/renderer_host/media/ref_counted_video_source_provider.h"
#include "content/browser/renderer_host/media/video_capture_provider.h"
#include "content/public/browser/video_capture_device_launcher.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/video_capture/public/mojom/device_factory.mojom.h"
#include "third_party/blink/public/common/mediastream/media_stream_request.h"

Expand Down Expand Up @@ -47,7 +48,8 @@ class CONTENT_EXPORT ServiceVideoCaptureDeviceLauncher

void OnCreatePushSubscriptionCallback(
video_capture::mojom::VideoSourcePtr source,
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription,
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription>
subscription,
base::OnceClosure connection_lost_cb,
video_capture::mojom::CreatePushSubscriptionResultCode result_code,
const media::VideoCaptureParams& params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "services/video_capture/public/cpp/mock_push_subscription.h"
#include "services/video_capture/public/cpp/mock_video_source.h"
#include "services/video_capture/public/cpp/mock_video_source_provider.h"
Expand Down Expand Up @@ -53,9 +54,7 @@ class ServiceVideoCaptureDeviceLauncherTest : public testing::Test {

void CloseSourceBinding() { source_binding_.reset(); }

void CloseSubscriptionBindings() {
subscription_bindings_.CloseAllBindings();
}
void CloseSubscriptionReceivers() { subscription_receivers_.Clear(); }

protected:
void SetUp() override {
Expand Down Expand Up @@ -104,8 +103,8 @@ class ServiceVideoCaptureDeviceLauncherTest : public testing::Test {
subscription,
video_capture::mojom::VideoSource::
CreatePushSubscriptionCallback& callback) {
subscription_bindings_.AddBinding(&mock_subscription_,
std::move(subscription));
subscription_receivers_.Add(&mock_subscription_,
std::move(subscription));
std::move(callback).Run(
video_capture::mojom::CreatePushSubscriptionResultCode::
kCreatedWithRequestedSettings,
Expand All @@ -131,8 +130,8 @@ class ServiceVideoCaptureDeviceLauncherTest : public testing::Test {
std::unique_ptr<mojo::Binding<video_capture::mojom::VideoSource>>
source_binding_;
video_capture::MockPushSubcription mock_subscription_;
mojo::BindingSet<video_capture::mojom::PushVideoStreamSubscription>
subscription_bindings_;
mojo::ReceiverSet<video_capture::mojom::PushVideoStreamSubscription>
subscription_receivers_;
std::unique_ptr<ServiceVideoCaptureDeviceLauncher> launcher_;
base::MockCallback<base::OnceClosure> connection_lost_cb_;
base::MockCallback<base::OnceClosure> done_cb_;
Expand Down Expand Up @@ -217,7 +216,8 @@ void ServiceVideoCaptureDeviceLauncherTest::RunLaunchingDeviceIsAbortedTest(
// Prepare the callback, but save it for now instead of invoking it.
create_push_subscription_success_answer_cb = base::BindOnce(
[](const media::VideoCaptureParams& requested_settings,
video_capture::mojom::PushVideoStreamSubscriptionRequest
mojo::PendingReceiver<
video_capture::mojom::PushVideoStreamSubscription>
subscription,
video_capture::mojom::VideoSource::
CreatePushSubscriptionCallback callback,
Expand Down Expand Up @@ -277,7 +277,8 @@ TEST_F(ServiceVideoCaptureDeviceLauncherTest,
[](mojo::PendingRemote<video_capture::mojom::Receiver>
subscriber,
const media::VideoCaptureParams& requested_settings,
video_capture::mojom::PushVideoStreamSubscriptionRequest
mojo::PendingReceiver<
video_capture::mojom::PushVideoStreamSubscription>
subscription,
video_capture::mojom::VideoSource::
CreatePushSubscriptionCallback callback) {
Expand Down Expand Up @@ -394,7 +395,7 @@ TEST_F(ServiceVideoCaptureDeviceLauncherTest,
TEST_F(ServiceVideoCaptureDeviceLauncherTest,
ConnectionToSourceLostAfterSuccessfulLaunch) {
RunConnectionLostAfterSuccessfulStartTest(base::BindOnce(
&ServiceVideoCaptureDeviceLauncherTest::CloseSubscriptionBindings,
&ServiceVideoCaptureDeviceLauncherTest::CloseSubscriptionReceivers,
base::Unretained(this)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,10 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest
requestable_settings.requested_format.frame_size = kVideoSize;
requestable_settings.buffer_type = buffer_type_to_request;

video_capture::mojom::PushVideoStreamSubscriptionPtr subscription;
video_source_->CreatePushSubscription(
std::move(subscriber_), requestable_settings,
false /*force_reopen_with_new_settings*/,
mojo::MakeRequest(&subscription_),
subscription_.BindNewPipeAndPassReceiver(),
base::BindOnce(&WebRtcVideoCaptureSharedDeviceBrowserTest::
OnCreatePushSubscriptionCallback,
weak_factory_.GetWeakPtr()));
Expand All @@ -200,7 +199,7 @@ class WebRtcVideoCaptureSharedDeviceBrowserTest
// For multi-client API case only
video_capture::mojom::VideoSourceProviderPtr video_source_provider_;
video_capture::mojom::VideoSourcePtr video_source_;
video_capture::mojom::PushVideoStreamSubscriptionPtr subscription_;
mojo::Remote<video_capture::mojom::PushVideoStreamSubscription> subscription_;

mojo::PendingRemote<video_capture::mojom::Receiver> subscriber_;
base::WeakPtrFactory<WebRtcVideoCaptureSharedDeviceBrowserTest> weak_factory_{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "media/capture/video/video_capture_device.h"
#include "media/capture/video/video_capture_system_impl.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/video_capture/device_factory_media_to_mojo_adapter.h"
#include "services/video_capture/device_media_to_mojo_adapter.h"
#include "services/video_capture/public/cpp/mock_receiver.h"
Expand Down Expand Up @@ -85,7 +86,7 @@ class MockDeviceSharedAccessTest : public ::testing::Test {
source_->CreatePushSubscription(
std::move(receiver_1_), requestable_settings_,
false /*force_reopen_with_new_settings*/,
mojo::MakeRequest(&subscription_1_),
subscription_1_.BindNewPipeAndPassReceiver(),
base::BindOnce(
[](base::RunLoop* run_loop,
media::VideoCaptureParams* requested_settings,
Expand Down Expand Up @@ -114,7 +115,8 @@ class MockDeviceSharedAccessTest : public ::testing::Test {
base::RunLoop run_loop;
source_->CreatePushSubscription(
std::move(receiver_2_), requestable_settings_,
force_reopen_with_new_settings, mojo::MakeRequest(&subscription_2_),
force_reopen_with_new_settings,
subscription_2_.BindNewPipeAndPassReceiver(),
base::BindOnce(
[](base::RunLoop* run_loop,
media::VideoCaptureParams* requested_settings,
Expand Down Expand Up @@ -151,7 +153,7 @@ class MockDeviceSharedAccessTest : public ::testing::Test {
source_->CreatePushSubscription(
std::move(receiver_1_), requestable_settings_,
false /*force_reopen_with_new_settings*/,
mojo::MakeRequest(&subscription_1_),
subscription_1_.BindNewPipeAndPassReceiver(),
base::BindOnce(
[](base::RunLoop* run_loop_1,
media::VideoCaptureParams* requested_settings,
Expand All @@ -174,7 +176,7 @@ class MockDeviceSharedAccessTest : public ::testing::Test {
source_->CreatePushSubscription(
std::move(receiver_2_), different_settings,
false /*force_reopen_with_new_settings*/,
mojo::MakeRequest(&subscription_2_),
subscription_2_.BindNewPipeAndPassReceiver(),
base::BindOnce(
[](base::RunLoop* run_loop_2,
media::VideoCaptureParams* requested_settings,
Expand Down Expand Up @@ -256,10 +258,10 @@ class MockDeviceSharedAccessTest : public ::testing::Test {
mojom::VideoSourcePtr source_;
media::VideoCaptureParams requestable_settings_;

mojom::PushVideoStreamSubscriptionPtr subscription_1_;
mojo::Remote<mojom::PushVideoStreamSubscription> subscription_1_;
mojo::PendingRemote<mojom::Receiver> receiver_1_;
MockReceiver mock_receiver_1_;
mojom::PushVideoStreamSubscriptionPtr subscription_2_;
mojo::Remote<mojom::PushVideoStreamSubscription> subscription_2_;
mojo::PendingRemote<mojom::Receiver> receiver_2_;
MockReceiver mock_receiver_2_;

Expand Down

0 comments on commit 12a04c9

Please sign in to comment.