[Mojo Video Capture] Add unit tests for ServiceVideoCaptureDeviceLauncher

content/browser/renderer_host/media/service_video_capture_device_launcher.cc

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..b435acf9146f54a6ce58866ccf4cfabd52bb1786 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,25 @@ 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);
+  device.set_connection_error_handler(
+      base::Bind(&ServiceVideoCaptureDeviceLauncher::
+                     OnConnectionLostWhileWaitingForCallback,
+                 base::Unretained(this)));

[emircan, 2017/05/23 23:05:40]

Can you add a comment explaining why Unretained is safe?

[chfremer, 2017/05/23 23:39:18]

Done.

+  // 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;
   (*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 +122,38 @@ 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());
+  const bool abort_requested = (state_ == State::DEVICE_START_ABORTING);
+  state_ = State::READY_TO_LAUNCH;
+  Callbacks* callbacks = callbacks_;

[emircan, 2017/05/23 23:05:40]

DCHECK(callbacks_)

[chfremer, 2017/05/23 23:39:18]

Done.

+  callbacks_ = nullptr;
+  ConcludeLaunchDeviceWithFailure(abort_requested, callbacks,
+                                  std::move(done_cb_));
+}
+
 }  // namespace content