Currently, Chromium does not listen for error notifications from QTKit, i.e. they are ignored. QTKit device creation can succeed, and then later fail asynchronously via such a message, never producing any frames. If Chromium doesn't listen for the error notifications, then this results in a getUserMedia call that never fires its success or failure callbacks.

For example, with this change, if too many cameras are connected to the same USB hub, then bandwidth limitations may cause some of the cameras to fail to open.  This results in a QTCaptureSessionRuntimeErrorNotification whose localized text reads "The operation couldn't be completed. (OSStatus error -536870164.)".  With this change, that notification results in a getUserMedia failure callback to the javascript (with event.name == 'PERMISSION_DENIED', which is the only currently supported error code).

BUG=265704

Review URL: https://chromiumcodereview.appspot.com/22982008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218892 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 52 insertions, 1 deletions

diff --git a/media/video/capture/mac/video_capture_device_mac.h b/media/video/capture/mac/video_capture_device_mac.h
index 6ca24f3..dc0935f 100644
--- a/media/video/capture/mac/video_capture_device_mac.h
+++ b/media/video/capture/mac/video_capture_device_mac.h
@@ -10,6 +10,9 @@
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "media/video/capture/video_capture_device.h"
 #include "media/video/capture/video_capture_types.h"
 
@@ -38,6 +41,8 @@ class VideoCaptureDeviceMac : public VideoCaptureDevice {
   void ReceiveFrame(const uint8* video_frame, int video_frame_length,
                     const VideoCaptureCapability& frame_info);
 
+  void ReceiveError(const std::string& reason);
+
  private:
   void SetErrorState(const std::string& reason);
 
@@ -52,8 +57,16 @@ class VideoCaptureDeviceMac : public VideoCaptureDevice {
 
   Name device_name_;
   VideoCaptureDevice::EventHandler* observer_;
+
+  // Only read and write state_ from inside this loop.
+  const scoped_refptr<base::MessageLoopProxy> loop_proxy_;
   InternalState state_;
 
+  // Used with Bind and PostTask to ensure that methods aren't called
+  // after the VideoCaptureDeviceMac is destroyed.
+  base::WeakPtrFactory<VideoCaptureDeviceMac> weak_factory_;
+  base::WeakPtr<VideoCaptureDeviceMac> weak_this_;
+
   VideoCaptureDeviceQTKit* capture_device_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureDeviceMac);
diff --git a/media/video/capture/mac/video_capture_device_mac.mm b/media/video/capture/mac/video_capture_device_mac.mm
index 1891217..957486e 100644
--- a/media/video/capture/mac/video_capture_device_mac.mm
+++ b/media/video/capture/mac/video_capture_device_mac.mm
@@ -6,6 +6,8 @@
 
 #import <QTKit/QTKit.h>
 
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/time/time.h"
 #include "media/video/capture/mac/video_capture_device_qtkit_mac.h"
@@ -94,17 +96,22 @@ VideoCaptureDevice* VideoCaptureDevice::Create(const Name& device_name) {
 VideoCaptureDeviceMac::VideoCaptureDeviceMac(const Name& device_name)
     : device_name_(device_name),
       observer_(NULL),
+      loop_proxy_(base::MessageLoopProxy::current()),
       state_(kNotInitialized),
+      weak_factory_(this),
+      weak_this_(weak_factory_.GetWeakPtr()),
       capture_device_(nil) {
 }
 
 VideoCaptureDeviceMac::~VideoCaptureDeviceMac() {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   [capture_device_ release];
 }
 
 void VideoCaptureDeviceMac::Allocate(
     const VideoCaptureCapability& capture_format,
     EventHandler* observer) {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   if (state_ != kIdle) {
     return;
   }
@@ -153,6 +160,7 @@ void VideoCaptureDeviceMac::Allocate(
 }
 
 void VideoCaptureDeviceMac::Start() {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   DCHECK_EQ(state_, kAllocated);
   if (![capture_device_ startCapture]) {
     SetErrorState("Could not start capture device.");
@@ -162,12 +170,14 @@ void VideoCaptureDeviceMac::Start() {
 }
 
 void VideoCaptureDeviceMac::Stop() {
-  DCHECK_EQ(state_, kCapturing);
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
+  DCHECK(state_ == kCapturing || state_ == kError) << state_;
   [capture_device_ stopCapture];
   state_ = kAllocated;
 }
 
 void VideoCaptureDeviceMac::DeAllocate() {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   if (state_ != kAllocated && state_ != kCapturing) {
     return;
   }
@@ -185,6 +195,7 @@ const VideoCaptureDevice::Name& VideoCaptureDeviceMac::device_name() {
 }
 
 bool VideoCaptureDeviceMac::Init() {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   DCHECK_EQ(state_, kNotInitialized);
 
   Names device_names;
@@ -206,11 +217,20 @@ void VideoCaptureDeviceMac::ReceiveFrame(
     const uint8* video_frame,
     int video_frame_length,
     const VideoCaptureCapability& frame_info) {
+  // This method is safe to call from a device capture thread,
+  // i.e. any thread controlled by QTKit.
   observer_->OnIncomingCapturedFrame(
       video_frame, video_frame_length, base::Time::Now(), 0, false, false);
 }
 
+void VideoCaptureDeviceMac::ReceiveError(const std::string& reason) {
+  loop_proxy_->PostTask(FROM_HERE,
+      base::Bind(&VideoCaptureDeviceMac::SetErrorState, weak_this_,
+          reason));
+}
+
 void VideoCaptureDeviceMac::SetErrorState(const std::string& reason) {
+  DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   DLOG(ERROR) << reason;
   state_ = kError;
   observer_->OnError();
diff --git a/media/video/capture/mac/video_capture_device_qtkit_mac.h b/media/video/capture/mac/video_capture_device_qtkit_mac.h
index 3c9a3db..a2d6f26 100644
--- a/media/video/capture/mac/video_capture_device_qtkit_mac.h
+++ b/media/video/capture/mac/video_capture_device_qtkit_mac.h
@@ -60,6 +60,9 @@ namespace media {
 // Stops video capturing.
 - (void)stopCapture;
 
+// Handle any QTCaptureSessionRuntimeErrorNotifications.
+- (void)handleNotification:(NSNotification *)errorNotification;
+
 @end
 
 #endif  // MEDIA_VIDEO_CAPTURE_MAC_VIDEO_CAPTURE_DEVICE_MAC_QTKIT_H_
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 fa2d7c3..b7e407a 100644
--- a/media/video/capture/mac/video_capture_device_qtkit_mac.mm
+++ b/media/video/capture/mac/video_capture_device_qtkit_mac.mm
@@ -121,6 +121,7 @@
     }
     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];
 
@@ -194,6 +195,12 @@
                   << [[error localizedDescription] UTF8String];
       return NO;
     }
+    NSNotificationCenter * notificationCenter =
+        [NSNotificationCenter defaultCenter];
+    [notificationCenter addObserver:self
+                           selector:@selector(handleNotification:)
+                               name:QTCaptureSessionRuntimeErrorNotification
+                             object:captureSession_];
     [captureSession_ startRunning];
   }
   return YES;
@@ -204,6 +211,8 @@
     [captureSession_ removeInput:captureDeviceInput_];
     [captureSession_ stopRunning];
   }
+
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
 }
 
 // |captureOutput| is called by the capture device to deliver a new frame.
@@ -269,4 +278,10 @@
   [lock_ unlock];
 }
 
+- (void)handleNotification:(NSNotification *)errorNotification {
+  NSError * error = (NSError *)[[errorNotification userInfo]
+      objectForKey:QTCaptureSessionErrorKey];
+  frameReceiver_->ReceiveError([[error localizedDescription] UTF8String]);
+}
+
 @end