Mac QTKit Video Capture: Run QTCaptureSession teardown in UI thread, clean up -stopCapture use.

It can be read in the code:
> QTKit achieves thread safety by posting messages to the
> main thread.  As part of -addOutput:, it posts a message to the main
> thread which in turn posts a notification which will run in a future
> spin after the original method returns.  -removeOutput: can post a
> main-thread message in between while holding a lock which the
> notification handler  will need.  Posting either -addOutput: or
> -removeOutput: to the main thread should fix it, remove is likely
> safer. http://crbug.com/152757

In the same spirit, seems that -removeInput: and -stopRunning are
being called from a background thread causing deadlocks every now
and then. This CL temptatively sends _both_ to be run on UI thread,
hence avoiding the potential deadlock.

-stopCapture is called from VideoCaptureDeviceMac::StopAndDeAllocate()
and then maybe in -setCaptureDevice. Remove the first call
and make sure it's always called inside -setCaptureDevice. 


BUG=399792, 285053

Review URL: https://codereview.chromium.org/464363003

Cr-Commit-Position: refs/heads/master@{#291018}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291018 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 28 insertions, 30 deletions

diff --git a/media/video/capture/mac/platform_video_capturing_mac.h b/media/video/capture/mac/platform_video_capturing_mac.h
index 29bc497..33ad7b6 100644
--- a/media/video/capture/mac/platform_video_capturing_mac.h
+++ b/media/video/capture/mac/platform_video_capturing_mac.h
@@ -21,7 +21,7 @@ class VideoCaptureDeviceMac;
 // implementation.
 - (id)initWithFrameReceiver:(media::VideoCaptureDeviceMac*)frameReceiver;
 
-// Set the frame receiver. This method executes the registration in mutual
+// Sets the frame receiver. This method executes the registration in mutual
 // exclusion.
 // TODO(mcasas): This method and stopCapture() are always called in sequence and
 // this one is only used to clear the frameReceiver, investigate if both can be
@@ -31,8 +31,9 @@ class VideoCaptureDeviceMac;
 // Sets which capture device to use by name passed as deviceId argument. The
 // device names are usually obtained via VideoCaptureDevice::GetDeviceNames()
 // method. This method will also configure all device properties except those in
-// setCaptureHeight:widht:frameRate. If |deviceId| is nil, all potential
-// configuration is torn down. Returns YES on sucess, NO otherwise.
+// setCaptureHeight:width:frameRate. If |deviceId| is nil, capture is stopped
+// and all potential configuration is torn down. Returns YES on sucess, NO
+// otherwise.
 - (BOOL)setCaptureDevice:(NSString*)deviceId;
 
 // Configures the capture properties.
@@ -40,7 +41,7 @@ class VideoCaptureDeviceMac;
                    width:(int)width
                frameRate:(float)frameRate;
 
-// Start video capturing, register observers. Returns YES on sucess, NO
+// Starts video capturing, registers observers. Returns YES on sucess, NO
 // otherwise.
 - (BOOL)startCapture;
 
diff --git a/media/video/capture/mac/video_capture_device_mac.mm b/media/video/capture/mac/video_capture_device_mac.mm
index 2c1943b..66019b7 100644
--- a/media/video/capture/mac/video_capture_device_mac.mm
+++ b/media/video/capture/mac/video_capture_device_mac.mm
@@ -440,7 +440,6 @@ void VideoCaptureDeviceMac::AllocateAndStart(
 void VideoCaptureDeviceMac::StopAndDeAllocate() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(state_ == kCapturing || state_ == kError) << state_;
-  [capture_device_ stopCapture];
 
   [capture_device_ setCaptureDevice:nil];
   [capture_device_ setFrameReceiver:nil];
diff --git a/media/video/capture/mac/video_capture_device_qtkit_mac.mm b/media/video/capture/mac/video_capture_device_qtkit_mac.mm
index 86d4e82..dec7415 100644
--- a/media/video/capture/mac/video_capture_device_qtkit_mac.mm
+++ b/media/video/capture/mac/video_capture_device_qtkit_mac.mm
@@ -146,28 +146,8 @@
           stringWithUTF8String:"No video capture device set, on removal."]];
       return YES;
     }
-    if ([[captureSession_ inputs] count] > 0) {
-      // The device is still running.
-      [self stopCapture];
-    }
-    if ([[captureSession_ outputs] count] > 0) {
-      // Only one output is set for |captureSession_|.
-      DCHECK_EQ([[captureSession_ outputs] count], 1u);
-      id output = [[captureSession_ outputs] objectAtIndex:0];
-      [output setDelegate:nil];
-
-      // TODO(shess): QTKit achieves thread safety by posting messages to the
-      // main thread.  As part of -addOutput:, it posts a message to the main
-      // thread which in turn posts a notification which will run in a future
-      // spin after the original method returns.  -removeOutput: can post a
-      // main-thread message in between while holding a lock which the
-      // notification handler  will need.  Posting either -addOutput: or
-      // -removeOutput: to the main thread should fix it, remove is likely
-      // safer. http://crbug.com/152757
-      [captureSession_ performSelectorOnMainThread:@selector(removeOutput:)
-                                        withObject:output
-                                     waitUntilDone:YES];
-    }
+    // Tear down input and output, stop the capture and deregister observers.
+    [self stopCapture];
     [captureSession_ release];
     captureSession_ = nil;
     [captureDeviceInput_ release];
@@ -242,12 +222,30 @@
 }
 
 - (void)stopCapture {
-  if ([[captureSession_ inputs] count] == 1) {
+  // QTKit achieves thread safety and asynchronous execution by posting messages
+  // to the main thread, e.g. -addOutput:. Both -removeOutput: and -removeInput:
+  // post a message to the main thread while holding a lock that the
+  // notification handler might need. To avoid a deadlock, we perform those
+  // tasks in the main thread. See bugs http://crbug.com/152757 and
+  // http://crbug.com/399792.
+  [self performSelectorOnMainThread:@selector(stopCaptureOnUIThread:)
+                         withObject:nil
+                      waitUntilDone:YES];
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+}
+
+- (void)stopCaptureOnUIThread:(id)dummy {
+  if ([[captureSession_ inputs] count] > 0) {
     [captureSession_ removeInput:captureDeviceInput_];
+    DCHECK_EQ([[captureSession_ inputs] count], 1u);
     [captureSession_ stopRunning];
   }
-
-  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  if ([[captureSession_ outputs] count] > 0) {
+    DCHECK_EQ([[captureSession_ outputs] count], 1u);
+    id output = [[captureSession_ outputs] objectAtIndex:0];
+    [output setDelegate:nil];
+    [captureSession_ removeOutput:output];
+  }
 }
 
 // |captureOutput| is called by the capture device to deliver a new frame.