Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1401)

Unified Diff: content/browser/renderer_host/media/service_video_capture_device_launcher.cc

Issue 2886303002: [Mojo Video Capture] Add unit tests for ServiceVideoCaptureDeviceLauncher (Closed)
Patch Set: Incorporated suggestions from Patch Set 2 Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/renderer_host/media/service_video_capture_device_launcher.cc
diff --git a/content/browser/renderer_host/media/service_video_capture_device_launcher.cc b/content/browser/renderer_host/media/service_video_capture_device_launcher.cc
index 4c86c448638cc39f1665e20a9a52664c24e6452e..522570ae017b9cb83091ee4e9f63c4ee556b43f1 100644
--- a/content/browser/renderer_host/media/service_video_capture_device_launcher.cc
+++ b/content/browser/renderer_host/media/service_video_capture_device_launcher.cc
@@ -57,7 +57,9 @@ void ConcludeLaunchDeviceWithFailure(
ServiceVideoCaptureDeviceLauncher::ServiceVideoCaptureDeviceLauncher(
video_capture::mojom::DeviceFactoryPtr* device_factory)
- : device_factory_(device_factory), state_(State::READY_TO_LAUNCH) {}
+ : device_factory_(device_factory),
+ state_(State::READY_TO_LAUNCH),
+ callbacks_(nullptr) {}
ServiceVideoCaptureDeviceLauncher::~ServiceVideoCaptureDeviceLauncher() {
DCHECK(sequence_checker_.CalledOnValidSequence());
@@ -88,18 +90,27 @@ void ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(
return;
}
video_capture::mojom::DevicePtr device;
- // We need the temporary variable |device_request| in order to guarantee that
- // mojo::MakeRequest(&device) happens before base::Passed(&device).
auto device_request = mojo::MakeRequest(&device);
+ // Ownership of |done_cb| is moved to |this|. It is not sufficient to attach
+ // it to the callback passed to |(*device_factory_)->CreateDevice()|, because
+ // |device_factory_| may get torn down before the callback is invoked.
+ done_cb_ = std::move(done_cb);
+ callbacks_ = callbacks;
+ // Use of Unretained(this) is safe, because |done_cb_| guarantees that |this|
+ // stays alive.
+ device.set_connection_error_handler(
+ base::Bind(&ServiceVideoCaptureDeviceLauncher::
+ OnConnectionLostWhileWaitingForCallback,
+ base::Unretained(this)));
(*device_factory_)
->CreateDevice(
device_id, std::move(device_request),
base::Bind(
- // Use of Unretained |this| is safe, because |done_cb| guarantees
+ // Use of Unretained |this| is safe, because |done_cb_| guarantees
// that |this| stays alive.
&ServiceVideoCaptureDeviceLauncher::OnCreateDeviceCallback,
base::Unretained(this), params, base::Passed(&device),
- std::move(receiver), callbacks, base::Passed(&done_cb)));
+ std::move(receiver)));
state_ = State::DEVICE_START_IN_PROGRESS;
}
@@ -113,24 +124,39 @@ void ServiceVideoCaptureDeviceLauncher::OnCreateDeviceCallback(
const media::VideoCaptureParams& params,
video_capture::mojom::DevicePtr device,
base::WeakPtr<media::VideoFrameReceiver> receiver,
- Callbacks* callbacks,
- base::OnceClosure done_cb,
video_capture::mojom::DeviceAccessResultCode result_code) {
DCHECK(sequence_checker_.CalledOnValidSequence());
+ DCHECK(callbacks_);
+ DCHECK(done_cb_);
+ device.set_connection_error_handler(base::Bind(&base::DoNothing));
const bool abort_requested = (state_ == State::DEVICE_START_ABORTING);
state_ = State::READY_TO_LAUNCH;
+ Callbacks* callbacks = callbacks_;
+ callbacks_ = nullptr;
switch (result_code) {
case video_capture::mojom::DeviceAccessResultCode::SUCCESS:
ConcludeLaunchDeviceWithSuccess(abort_requested, params,
std::move(device), std::move(receiver),
- callbacks, std::move(done_cb));
+ callbacks, std::move(done_cb_));
return;
case video_capture::mojom::DeviceAccessResultCode::ERROR_DEVICE_NOT_FOUND:
case video_capture::mojom::DeviceAccessResultCode::NOT_INITIALIZED:
ConcludeLaunchDeviceWithFailure(abort_requested, callbacks,
- std::move(done_cb));
+ std::move(done_cb_));
return;
}
}
+void ServiceVideoCaptureDeviceLauncher::
+ OnConnectionLostWhileWaitingForCallback() {
+ DCHECK(sequence_checker_.CalledOnValidSequence());
+ DCHECK(callbacks_);
+ const bool abort_requested = (state_ == State::DEVICE_START_ABORTING);
+ state_ = State::READY_TO_LAUNCH;
+ Callbacks* callbacks = callbacks_;
+ callbacks_ = nullptr;
+ ConcludeLaunchDeviceWithFailure(abort_requested, callbacks,
+ std::move(done_cb_));
+}
+
} // namespace content

Powered by Google App Engine
This is Rietveld 408576698