[M65] media: handle StopAndDeAllocate after suspend correctly

Some application (e.g. Hangouts) closes the camera explicitly on system
suspend. As the Chrome OS VCD also closes the camera on system suspend
without going through the full destruction of the VideoCaptureDevice,
this results in StopAndDeAllocate being called back-to-back. The second
call to StopAndDeAllocate should simply return if the device context is
already destroyed.

BUG=b:73436939
TEST=unit tests
TEST=Verify that Hangouts call works correctly on Soraka when system
     suspends

Change-Id: Id6127b751ccc5bc71538d622bc0905bf03951e0d
Reviewed-on: https://chromium-review.googlesource.com/930605
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>
Commit-Queue: Ricky Liang <jcliang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#539091}(cherry picked from commit 7b94276046ba4b6fd719f07706fa102a62a5c366)
Reviewed-on: https://chromium-review.googlesource.com/939061
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#602}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
diff --git a/media/capture/video/chromeos/camera_device_delegate.cc b/media/capture/video/chromeos/camera_device_delegate.cc
index 0383a05..1d489a1 100644
--- a/media/capture/video/chromeos/camera_device_delegate.cc
+++ b/media/capture/video/chromeos/camera_device_delegate.cc
@@ -86,14 +86,13 @@
 }
 
 void CameraDeviceDelegate::StopAndDeAllocate(
-    base::Closure device_close_callback) {
+    base::OnceClosure device_close_callback) {
   DCHECK(ipc_task_runner_->BelongsToCurrentThread());
-  // StopAndDeAllocate may be called at any state except
-  // CameraDeviceContext::State::kStopping.
-  DCHECK_NE(device_context_->GetState(), CameraDeviceContext::State::kStopping);
 
-  if (device_context_->GetState() == CameraDeviceContext::State::kStopped ||
-      !stream_buffer_manager_) {
+  if (!device_context_ ||
+      device_context_->GetState() == CameraDeviceContext::State::kStopped ||
+      (device_context_->GetState() == CameraDeviceContext::State::kError &&
+       !stream_buffer_manager_)) {
     // In case of Mojo connection error the device may be stopped before
     // StopAndDeAllocate is called; in case of device open failure, the state
     // is set to kError and |stream_buffer_manager_| is uninitialized.
@@ -101,6 +100,10 @@
     return;
   }
 
+  // StopAndDeAllocate may be called at any state except
+  // CameraDeviceContext::State::kStopping.
+  DCHECK_NE(device_context_->GetState(), CameraDeviceContext::State::kStopping);
+
   device_close_callback_ = std::move(device_close_callback);
   device_context_->SetState(CameraDeviceContext::State::kStopping);
   if (!device_ops_.is_bound()) {
diff --git a/media/capture/video/chromeos/camera_device_delegate.h b/media/capture/video/chromeos/camera_device_delegate.h
index 9c062e0..9cff56a 100644
--- a/media/capture/video/chromeos/camera_device_delegate.h
+++ b/media/capture/video/chromeos/camera_device_delegate.h
@@ -66,7 +66,7 @@
   // Delegation methods for the VideoCaptureDevice interface.
   void AllocateAndStart(const VideoCaptureParams& params,
                         CameraDeviceContext* device_context);
-  void StopAndDeAllocate(base::Closure device_close_callback);
+  void StopAndDeAllocate(base::OnceClosure device_close_callback);
   void TakePhoto(VideoCaptureDevice::TakePhotoCallback callback);
   void GetPhotoState(VideoCaptureDevice::GetPhotoStateCallback callback);
   void SetPhotoOptions(mojom::PhotoSettingsPtr settings,
@@ -162,7 +162,7 @@
   // Where all the Mojo IPC calls takes place.
   const scoped_refptr<base::SingleThreadTaskRunner> ipc_task_runner_;
 
-  base::Closure device_close_callback_;
+  base::OnceClosure device_close_callback_;
 
   base::WeakPtrFactory<CameraDeviceDelegate> weak_ptr_factory_;
 
diff --git a/media/capture/video/chromeos/camera_device_delegate_unittest.cc b/media/capture/video/chromeos/camera_device_delegate_unittest.cc
index 8103886..8a0316b 100644
--- a/media/capture/video/chromeos/camera_device_delegate_unittest.cc
+++ b/media/capture/video/chromeos/camera_device_delegate_unittest.cc
@@ -576,4 +576,46 @@
   ResetDevice();
 }
 
+// Test that the class handles it correctly when StopAndDeAllocate is called
+// multiple times.
+TEST_F(CameraDeviceDelegateTest, DoubleStopAndDeAllocate) {
+  AllocateDeviceWithDescriptor(kDefaultDescriptor);
+
+  VideoCaptureParams params;
+  params.requested_format = kDefaultCaptureFormat;
+
+  auto* mock_client = ResetDeviceContext();
+  mock_client->SetFrameCb(BindToCurrentLoop(base::BindOnce(
+      &CameraDeviceDelegateTest::QuitRunLoop, base::Unretained(this))));
+  mock_client->SetQuitCb(BindToCurrentLoop(base::BindOnce(
+      &CameraDeviceDelegateTest::QuitRunLoop, base::Unretained(this))));
+  SetUpExpectationUntilCapturing(mock_client);
+  SetUpExpectationForCaptureLoop();
+
+  device_delegate_thread_.task_runner()->PostTask(
+      FROM_HERE, base::BindOnce(&CameraDeviceDelegate::AllocateAndStart,
+                                camera_device_delegate_->GetWeakPtr(), params,
+                                base::Unretained(device_context_.get())));
+
+  // Wait until a frame is received.  MockVideoCaptureClient calls QuitRunLoop()
+  // to stop the run loop.
+  DoLoop();
+
+  EXPECT_EQ(CameraDeviceContext::State::kCapturing, GetState());
+
+  SetUpExpectationForClose();
+
+  WaitForDeviceToClose();
+
+  device_delegate_thread_.task_runner()->PostTask(
+      FROM_HERE, base::BindOnce(&CameraDeviceDelegate::StopAndDeAllocate,
+                                camera_device_delegate_->GetWeakPtr(),
+                                BindToCurrentLoop(base::BindOnce(
+                                    &CameraDeviceDelegateTest::QuitRunLoop,
+                                    base::Unretained(this)))));
+  DoLoop();
+
+  ResetDevice();
+}
+
 }  // namespace media
diff --git a/media/capture/video/chromeos/video_capture_device_arc_chromeos.cc b/media/capture/video/chromeos/video_capture_device_arc_chromeos.cc
index 91da489..934c117 100644
--- a/media/capture/video/chromeos/video_capture_device_arc_chromeos.cc
+++ b/media/capture/video/chromeos/video_capture_device_arc_chromeos.cc
@@ -136,6 +136,9 @@
 void VideoCaptureDeviceArcChromeOS::OpenDevice() {
   DCHECK(capture_task_runner_->BelongsToCurrentThread());
 
+  if (!camera_device_delegate_) {
+    return;
+  }
   // It's safe to pass unretained |device_context_| here since
   // VideoCaptureDeviceArcChromeOS owns |camera_device_delegate_| and makes
   // sure |device_context_| outlives |camera_device_delegate_|.