[Video Capture, Mac] Ensure code is running on a CFRunLoop enabled thread

This is to prevent regression that only manifests in very subtle issues such
as device enumeration becoming unreliable.

Bug: 848042
Change-Id: I4044c2494e2a525ca2eeca22bd67698e651360ff
Reviewed-on: https://chromium-review.googlesource.com/c/1246147Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597293}

media/capture/video/mac/video_capture_device_factory_mac.mm

@@ -19,12 +19,28 @@
 #import "media/capture/video/mac/video_capture_device_decklink_mac.h"
 #include "media/capture/video/mac/video_capture_device_mac.h"
 
-namespace media {
+namespace {
+
+void EnsureRunsOnCFRunLoopEnabledThread() {
+  static bool has_checked_cfrunloop_for_video_capture = false;
+  if (!has_checked_cfrunloop_for_video_capture) {
+    base::ScopedCFTypeRef<CFRunLoopMode> mode(
+        CFRunLoopCopyCurrentMode(CFRunLoopGetCurrent()));
+    CHECK(mode != NULL)
+        << "The MacOS video capture code must be run on a CFRunLoop-enabled "
+           "thread";
+    has_checked_cfrunloop_for_video_capture = true;
+  }
+}
 
 // Blacklisted devices are identified by a characteristic trailing substring of
 // uniqueId. At the moment these are just Blackmagic devices.
 const char* kBlacklistedCamerasIdSignature[] = {"-01FDA82C8A9C"};
 
+}  // anonymous namespace
+
+namespace media {
+
 static bool IsDeviceBlacklisted(
     const VideoCaptureDeviceDescriptor& descriptor) {
   bool is_device_blacklisted = false;
@@ -52,6 +68,7 @@ std::unique_ptr<VideoCaptureDevice> VideoCaptureDeviceFactoryMac::CreateDevice(
     const VideoCaptureDeviceDescriptor& descriptor) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(descriptor.capture_api, VideoCaptureApi::UNKNOWN);
+  EnsureRunsOnCFRunLoopEnabledThread();
 
   std::unique_ptr<VideoCaptureDevice> capture_device;
   if (descriptor.capture_api == VideoCaptureApi::MACOSX_DECKLINK) {
@@ -70,6 +87,8 @@ std::unique_ptr<VideoCaptureDevice> VideoCaptureDeviceFactoryMac::CreateDevice(
 void VideoCaptureDeviceFactoryMac::GetDeviceDescriptors(
     VideoCaptureDeviceDescriptors* device_descriptors) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  EnsureRunsOnCFRunLoopEnabledThread();
+
   // Loop through all available devices and add to |device_descriptors|.
   NSDictionary* capture_devices;
   DVLOG(1) << "Enumerating video capture devices using AVFoundation";

media/capture/video/mac/video_capture_device_factory_mac_unittest.mm

@@ -2,12 +2,35 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 #include "media/capture/video/mac/video_capture_device_factory_mac.h"
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
 #include "media/capture/video/mac/video_capture_device_mac.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace media {
 
+// Video capture code on MacOSX must run on a CFRunLoop enabled thread
+// for interaction with AVFoundation.
+// In order to make the test case run on the actual message loop that has
+// been created for this thread, we need to run it inside a RunLoop. This is
+// required, because on MacOS the capture code must run on a CFRunLoop
+// enabled message loop.
+void RunTestCase(base::OnceClosure test_case) {
+  base::test::ScopedTaskEnvironment scoped_task_environment(
+      base::test::ScopedTaskEnvironment::MainThreadType::UI);
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](base::RunLoop* run_loop, base::OnceClosure* test_case) {
+                       std::move(*test_case).Run();
+                       run_loop->Quit();
+                     },
+                     &run_loop, &test_case));
+  run_loop.Run();
+}
+
 TEST(VideoCaptureDeviceFactoryMacTest, ListDevicesAVFoundation) {
+  RunTestCase(base::BindOnce([]() {
     VideoCaptureDeviceFactoryMac video_capture_device_factory;
 
     VideoCaptureDeviceDescriptors descriptors;
@@ -18,6 +41,7 @@ TEST(VideoCaptureDeviceFactoryMacTest, ListDevicesAVFoundation) {
     }
     for (const auto& descriptor : descriptors)
       EXPECT_EQ(VideoCaptureApi::MACOSX_AVFOUNDATION, descriptor.capture_api);
+  }));
 }
 
 };  // namespace media

media/capture/video/video_capture_device_unittest.cc

@@ -206,11 +206,27 @@ class VideoCaptureDeviceTest
   }
 #endif
 
+  void RunOpenInvalidDeviceTestCase();
+  void RunCaptureWithSizeTestCase();
+  void RunAllocateBadSizeTestCase();
+  void RunReAllocateCameraTestCase();
+  void RunCaptureMjpegTestCase();
+  void RunNoCameraSupportsPixelFormatMaxTestCase();
+  void RunTakePhotoTestCase();
+  void RunGetPhotoStateTestCase();
+
  protected:
   typedef VideoCaptureDevice::Client Client;
 
   VideoCaptureDeviceTest()
-      : device_descriptors_(new VideoCaptureDeviceDescriptors()),
+      :
+#if defined(OS_MACOSX)
+        // Video capture code on MacOSX must run on a CFRunLoop enabled thread
+        // for interaction with AVFoundation.
+        scoped_task_environment_(
+            base::test::ScopedTaskEnvironment::MainThreadType::UI),
+#endif
+        device_descriptors_(new VideoCaptureDeviceDescriptors()),
         main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
         video_capture_client_(CreateDeviceClient()),
         image_capture_client_(new MockImageCaptureClient()) {
@@ -295,7 +311,8 @@ class VideoCaptureDeviceTest
   }
 
   void WaitForCapturedFrame() {
-    run_loop_.reset(new base::RunLoop());
+    run_loop_.reset(
+        new base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed));
     run_loop_->Run();
   }
 
@@ -388,6 +405,27 @@ class VideoCaptureDeviceTest
     return supported_formats.begin()->frame_size;
   }
 
+  void RunTestCase(base::OnceClosure test_case) {
+#if defined(OS_MACOSX)
+    // In order to make the test case run on the actual message loop that has
+    // been created for this thread, we need to run it inside a RunLoop. This is
+    // required, because on MacOS the capture code must run on a CFRunLoop
+    // enabled message loop.
+    base::RunLoop run_loop;
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](base::RunLoop* run_loop, base::OnceClosure* test_case) {
+              std::move(*test_case).Run();
+              run_loop->Quit();
+            },
+            &run_loop, &test_case));
+    run_loop.Run();
+#else
+    std::move(test_case).Run();
+#endif
+  }
+
 #if defined(OS_WIN)
   base::win::ScopedCOMInitializer initialize_com_;
 #endif
@@ -413,6 +451,11 @@ class VideoCaptureDeviceTest
 #endif
 // Tries to allocate an invalid device and verifies it doesn't work.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
+  RunTestCase(
+      base::BindOnce(&VideoCaptureDeviceTest::RunOpenInvalidDeviceTestCase,
+                     base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunOpenInvalidDeviceTestCase() {
   VideoCaptureDeviceDescriptor invalid_descriptor;
   invalid_descriptor.device_id = "jibberish";
   invalid_descriptor.set_display_name("jibberish");
@@ -452,6 +495,11 @@ TEST(VideoCaptureDeviceDescriptor, RemoveTrailingWhitespaceFromDisplayName) {
 
 // Allocates the first enumerated device, and expects a frame.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
+  RunTestCase(
+      base::BindOnce(&VideoCaptureDeviceTest::RunCaptureWithSizeTestCase,
+                     base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunCaptureWithSizeTestCase() {
   const auto descriptor = FindUsableDeviceDescriptor();
   if (!descriptor)
     return;
@@ -502,6 +550,11 @@ INSTANTIATE_TEST_CASE_P(
 // Allocates a device with an uncommon resolution and verifies frames are
 // captured in a close, much more typical one.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
+  RunTestCase(
+      base::BindOnce(&VideoCaptureDeviceTest::RunAllocateBadSizeTestCase,
+                     base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunAllocateBadSizeTestCase() {
   const auto descriptor = FindUsableDeviceDescriptor();
   if (!descriptor)
     return;
@@ -529,6 +582,11 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
 
 // Cause hangs on Windows, Linux. Fails Android. https://crbug.com/417824
 WRAPPED_TEST_P(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
+  RunTestCase(
+      base::BindOnce(&VideoCaptureDeviceTest::RunReAllocateCameraTestCase,
+                     base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunReAllocateCameraTestCase() {
   const auto descriptor = FindUsableDeviceDescriptor();
   if (!descriptor)
     return;
@@ -573,6 +631,10 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
 
 // Starts the camera in 720p to try and capture MJPEG format.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
+  RunTestCase(base::BindOnce(&VideoCaptureDeviceTest::RunCaptureMjpegTestCase,
+                             base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunCaptureMjpegTestCase() {
   std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
       GetFirstDeviceDescriptorSupportingPixelFormat(PIXEL_FORMAT_MJPEG);
   if (!device_descriptor) {
@@ -610,6 +672,11 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
 }
 
 WRAPPED_TEST_P(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
+  RunTestCase(base::BindOnce(
+      &VideoCaptureDeviceTest::RunNoCameraSupportsPixelFormatMaxTestCase,
+      base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunNoCameraSupportsPixelFormatMaxTestCase() {
   // Use PIXEL_FORMAT_MAX to iterate all device names for testing
   // GetDeviceSupportedFormats().
   std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
@@ -622,6 +689,10 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
 // Starts the camera and verifies that a photo can be taken. The correctness of
 // the photo is enforced by MockImageCaptureClient.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
+  RunTestCase(base::BindOnce(&VideoCaptureDeviceTest::RunTakePhotoTestCase,
+                             base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunTakePhotoTestCase() {
   const auto descriptor = FindUsableDeviceDescriptor();
   if (!descriptor)
     return;
@@ -654,7 +725,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
   VideoCaptureDevice::TakePhotoCallback scoped_callback = base::BindOnce(
       &MockImageCaptureClient::DoOnPhotoTaken, image_capture_client_);
 
-  base::RunLoop run_loop;
+  base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
   base::Closure quit_closure = BindToCurrentLoop(run_loop.QuitClosure());
   EXPECT_CALL(*image_capture_client_.get(), OnCorrectPhotoTaken())
       .Times(1)
@@ -668,6 +739,10 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
 
 // Starts the camera and verifies that the photo capabilities can be retrieved.
 WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_GetPhotoState) {
+  RunTestCase(base::BindOnce(&VideoCaptureDeviceTest::RunGetPhotoStateTestCase,
+                             base::Unretained(this)));
+}
+void VideoCaptureDeviceTest::RunGetPhotoStateTestCase() {
   const auto descriptor = FindUsableDeviceDescriptor();
   if (!descriptor)
     return;
@@ -704,7 +779,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_GetPhotoState) {
   // On Chrome OS AllocateAndStart() is asynchronous, so wait until we get the
   // first frame.
   WaitForCapturedFrame();
-  base::RunLoop run_loop;
+  base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
   base::Closure quit_closure = BindToCurrentLoop(run_loop.QuitClosure());
   EXPECT_CALL(*image_capture_client_.get(), OnCorrectGetPhotoState())
       .Times(1)
@@ -765,7 +840,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, CheckPhotoCallbackRelease) {
   VideoCaptureDevice::TakePhotoCallback scoped_callback = base::BindOnce(
       &MockImageCaptureClient::DoOnPhotoTaken, image_capture_client_);
 
-  base::RunLoop run_loop;
+  base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
   base::RepeatingClosure quit_closure =
       BindToCurrentLoop(run_loop.QuitClosure());
   EXPECT_CALL(*image_capture_client_.get(), OnCorrectPhotoTaken())