[Mojo Video Capture] Add unit tests for ServiceVideoCaptureDeviceLauncher

content/browser/renderer_host/media/service_video_capture_device_launcher.cc

 // 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 "content/browser/renderer_host/media/service_video_capture_device_launcher.h"
 
 #include "content/browser/renderer_host/media/service_launched_video_capture_device.h"
 #include "content/public/browser/browser_thread.h"
 #include "media/capture/video/video_frame_receiver_on_task_runner.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ skipping 39 matching lines @@
     callbacks->OnDeviceLaunchAborted();
   else
     callbacks->OnDeviceLaunchFailed();
   base::ResetAndReturn(&done_cb).Run();
 }
 
 }  // anonymous namespace
 
 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());
   DCHECK(state_ == State::READY_TO_LAUNCH);
 }
 
 void ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(
     const std::string& device_id,
     MediaStreamType stream_type,
     const media::VideoCaptureParams& params,
@@ skipping 10 matching lines @@
   }
 
   if (!device_factory_->is_bound()) {
     // This can happen when the ServiceVideoCaptureProvider owning
     // |device_factory_| loses connection to the service process and resets
     // |device_factory_|.
     ConcludeLaunchDeviceWithFailure(false, callbacks, std::move(done_cb));
     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;
 }
 
 void ServiceVideoCaptureDeviceLauncher::AbortLaunch() {
   DCHECK(sequence_checker_.CalledOnValidSequence());
   if (state_ == State::DEVICE_START_IN_PROGRESS)
     state_ = State::DEVICE_START_ABORTING;
 }
 
 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