Timeout hanging video capture start

Add a timeout when starting video capture for a local device, to
gracefully handle the case where a device never starts.
Deadline initially picked at 10s, and able to be disabled by a Feature
flag in the unlikely case that this has negative effects.

Tested locally by inserting an artificial delay in the VideoCaptureHost
handling.

Bug: 1121470
Change-Id: I1e46b1d00ae860386a34f5704554bf7cfa8972db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475914
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818798}

third_party/blink/renderer/platform/video_capture/video_capture_impl.cc

@@ -40,6 +40,9 @@ namespace blink {
 
 constexpr int kMaxFirstFrameLogs = 5;
 
+const base::Feature kTimeoutHangingVideoCaptureStarts{
+    "TimeoutHangingVideoCaptureStarts", base::FEATURE_ENABLED_BY_DEFAULT};
+
 using VideoFrameBufferHandleType = media::mojom::blink::VideoBufferHandle::Tag;
 
 // A collection of all types of handles that we use to reference a camera buffer
@@ -444,6 +447,9 @@ void VideoCaptureImpl::OnStateChanged(media::mojom::VideoCaptureState state) {
   DVLOG(1) << __func__ << " state: " << state;
   DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
 
+  // Stop the startup deadline timer as something has happened.
+  startup_timeout_.Stop();
+
   switch (state) {
     case media::mojom::VideoCaptureState::STARTED:
       OnLog("VideoCaptureImpl changing state to VIDEO_CAPTURE_STATE_STARTED");
@@ -723,6 +729,8 @@ void VideoCaptureImpl::OnBufferDestroyed(int32_t buffer_id) {
   }
 }
 
+constexpr base::TimeDelta VideoCaptureImpl::kCaptureStartTimeout;
+
 void VideoCaptureImpl::OnAllClientsFinishedConsumingFrame(
     int buffer_id,
     scoped_refptr<BufferContext> buffer_context,
@@ -793,10 +801,22 @@ void VideoCaptureImpl::StartCaptureInternal() {
   state_ = VIDEO_CAPTURE_STATE_STARTING;
   OnLog("VideoCaptureImpl changing state to VIDEO_CAPTURE_STATE_STARTING");
 
+  if (base::FeatureList::IsEnabled(kTimeoutHangingVideoCaptureStarts)) {
+    startup_timeout_.Start(FROM_HERE, kCaptureStartTimeout,
+                           base::BindOnce(&VideoCaptureImpl::OnStartTimedout,
+                                          base::Unretained(this)));
+  }
+
   GetVideoCaptureHost()->Start(device_id_, session_id_, params_,
                                observer_receiver_.BindNewPipeAndPassRemote());
 }
 
+void VideoCaptureImpl::OnStartTimedout() {
+  DCHECK_CALLED_ON_VALID_THREAD(io_thread_checker_);
+  OnLog("VideoCaptureImpl timed out during starting");
+  OnStateChanged(media::mojom::VideoCaptureState::FAILED);
+}
+
 void VideoCaptureImpl::OnDeviceSupportedFormats(
     VideoCaptureDeviceFormatsCallback callback,
     const Vector<media::VideoCaptureFormat>& supported_formats) {

third_party/blink/renderer/platform/video_capture/video_capture_impl.h

@@ -30,6 +30,8 @@ class GpuVideoAcceleratorFactories;
 
 namespace blink {
 
+extern const PLATFORM_EXPORT base::Feature kTimeoutHangingVideoCaptureStarts;
+
 // VideoCaptureImpl represents a capture device in renderer process. It provides
 // an interface for clients to command the capture (Start, Stop, etc), and
 // communicates back to these clients e.g. the capture state or incoming
@@ -93,6 +95,9 @@ class PLATFORM_EXPORT VideoCaptureImpl
                      media::mojom::blink::VideoFrameInfoPtr info) override;
   void OnBufferDestroyed(int32_t buffer_id) override;
 
+  static constexpr base::TimeDelta kCaptureStartTimeout =
+      base::TimeDelta::FromSeconds(10);
+
  private:
   friend class VideoCaptureImplTest;
   friend class MockVideoCaptureImpl;
@@ -143,6 +148,8 @@ class PLATFORM_EXPORT VideoCaptureImpl
       const media::VideoFrameFeedback* feedback,
       BufferFinishedCallback callback_to_io_thread);
 
+  void OnStartTimedout();
+
   // |device_id_| and |session_id_| are different concepts, but we reuse the
   // same numerical value, passed on construction.
   const base::UnguessableToken device_id_;
@@ -184,6 +191,8 @@ class PLATFORM_EXPORT VideoCaptureImpl
 
   THREAD_CHECKER(io_thread_checker_);
 
+  base::OneShotTimer startup_timeout_;
+
   // WeakPtrFactory pointing back to |this| object, for use with
   // media::VideoFrames constructed in OnBufferReceived() from buffers cached
   // in |client_buffers_|.

third_party/blink/renderer/platform/video_capture/video_capture_impl_test.cc

@@ -11,6 +11,7 @@
 #include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/unsafe_shared_memory_region.h"
 #include "base/test/bind_test_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "media/capture/mojom/video_capture.mojom-blink.h"
 #include "media/capture/mojom/video_capture_types.mojom-blink.h"
@@ -227,13 +228,15 @@ class VideoCaptureImplTest : public ::testing::Test {
   }
 
   const base::UnguessableToken session_id_ = base::UnguessableToken::Create();
-  base::test::TaskEnvironment task_environment_;
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
   ScopedTestingPlatformSupport<TestingPlatformSupportForGpuMemoryBuffer>
       platform_;
   std::unique_ptr<VideoCaptureImpl> video_capture_impl_;
   MockMojoVideoCaptureHost mock_video_capture_host_;
   media::VideoCaptureParams params_small_;
   media::VideoCaptureParams params_large_;
+  base::test::ScopedFeatureList feature_list_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureImplTest);
@@ -681,4 +684,39 @@ TEST_F(VideoCaptureImplTest,
   StopCapture(0);
 }
 
+TEST_F(VideoCaptureImplTest, StartTimeout) {
+  EXPECT_CALL(*this, OnStateUpdate(blink::VIDEO_CAPTURE_STATE_ERROR));
+  EXPECT_CALL(mock_video_capture_host_, DoStart(_, session_id_, params_small_));
+
+  ON_CALL(mock_video_capture_host_, DoStart(_, _, _))
+      .WillByDefault(InvokeWithoutArgs([]() {
+        // Do nothing.
+      }));
+
+  StartCapture(0, params_small_);
+  task_environment_.FastForwardBy(VideoCaptureImpl::kCaptureStartTimeout);
+}
+
+TEST_F(VideoCaptureImplTest, StartTimeout_FeatureDisabled) {
+  feature_list_.InitAndDisableFeature(kTimeoutHangingVideoCaptureStarts);
+
+  EXPECT_CALL(mock_video_capture_host_, DoStart(_, session_id_, params_small_));
+  ON_CALL(mock_video_capture_host_, DoStart(_, _, _))
+      .WillByDefault(InvokeWithoutArgs([]() {
+        // Do nothing.
+      }));
+
+  StartCapture(0, params_small_);
+  // Wait past the deadline, nothing should happen.
+  task_environment_.FastForwardBy(2 * VideoCaptureImpl::kCaptureStartTimeout);
+
+  // Finally callback that the capture has started, should respond.
+  EXPECT_CALL(*this, OnStateUpdate(blink::VIDEO_CAPTURE_STATE_STARTED));
+  video_capture_impl_->OnStateChanged(media::mojom::VideoCaptureState::STARTED);
+
+  EXPECT_CALL(*this, OnStateUpdate(blink::VIDEO_CAPTURE_STATE_STOPPED));
+  EXPECT_CALL(mock_video_capture_host_, Stop(_));
+  StopCapture(0);
+}
+
 }  // namespace blink