DeviceMonitorMac AVFoundation: move mgmt of |suspend_observer_| to a class living in Device Thread.

This CL addresses a potential problem due to a race: 
|suspend_observer_| [1] could have been deallocated in 
~AVFoundationMonitorImpl while still being in use 
in Device Thread [2]. This race didn't show up in normal
circumstances since it needed the Device Thread to
be active for a very short time; xians@ stumbled upon
it during some unrelated code activities that made a
browser_test fail.

This CL then cleans up the management of 
|suspend_observer_| from the  AVFoundationMonitorImpl
to a new class SuspendObserverDelegate, the former 
living exclusively in UI thread, the latter in Device 
Thread. SuspendObserverDelegate gets the Device
Thread parts of AVFoundationMonitorImpl.

The new class sits between AVFoundationMonitorImpl
and the Objective-C++ CrAVFoundationDeviceObserver.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/device_monitor_mac.mm&sq=package:chromium&type=cs&l=259
[2] https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/device_monitor_mac.mm&sq=package:chromium&type=cs&l=336

BUG=288562, 372418

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

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

content/browser/device_monitor_mac.mm

@@ -11,6 +11,8 @@
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/mac/scoped_nsobject.h"
+#include "base/threading/thread_checker.h"
+#include "content/public/browser/browser_thread.h"
 #import "media/video/capture/mac/avfoundation_glue.h"
 
 namespace {
@@ -208,22 +210,21 @@ void QTKitMonitorImpl::OnDeviceChanged() {
 }
 
 // Forward declaration for use by CrAVFoundationDeviceObserver.
-class AVFoundationMonitorImpl;
+class SuspendObserverDelegate;
 
 }  // namespace
 
 // This class is a Key-Value Observer (KVO) shim. It is needed because C++
-// classes cannot observe Key-Values directly. This class is used by
-// AVfoundationMonitorImpl and executed in its |device_task_runner_|, a.k.a.
-// "Device Thread". -stopObserving is called dutifully on -dealloc on UI thread.
+// classes cannot observe Key-Values directly. Created, manipulated, and
+// destroyed on the Device Thread by SuspendedObserverDelegate.
 @interface CrAVFoundationDeviceObserver : NSObject {
  @private
-  AVFoundationMonitorImpl* receiver_;
+  SuspendObserverDelegate* receiver_;  // weak
   // Member to keep track of the devices we are already monitoring.
   std::set<CrAVCaptureDevice*> monitoredDevices_;
 }
 
-- (id)initWithChangeReceiver:(AVFoundationMonitorImpl*)receiver;
+- (id)initWithChangeReceiver:(SuspendObserverDelegate*)receiver;
 - (void)startObserving:(CrAVCaptureDevice*)device;
 - (void)stopObserving:(CrAVCaptureDevice*)device;
 
@@ -231,10 +232,91 @@ class AVFoundationMonitorImpl;
 
 namespace {
 
+// This class owns and manages the lifetime of a CrAVFoundationDeviceObserver.
+// Provides a callback for this device observer to indicate that there has been
+// a device change of some kind. Created by AVFoundationMonitorImpl in UI thread
+// but living in Device Thread.
+class SuspendObserverDelegate :
+    public base::RefCountedThreadSafe<SuspendObserverDelegate> {
+ public:
+  explicit SuspendObserverDelegate(DeviceMonitorMacImpl* monitor)
+      : avfoundation_monitor_impl_(monitor) {
+    device_thread_checker_.DetachFromThread();
+  }
+
+  void OnDeviceChanged();
+  void StartObserver();
+  void ResetDeviceMonitorOnUIThread();
+
+ private:
+  friend class base::RefCountedThreadSafe<SuspendObserverDelegate>;
+
+  virtual ~SuspendObserverDelegate() {}
+
+  void OnDeviceChangedOnUIThread(
+      const std::vector<DeviceInfo>& snapshot_devices);
+
+  base::ThreadChecker device_thread_checker_;
+  base::scoped_nsobject<CrAVFoundationDeviceObserver> suspend_observer_;
+  DeviceMonitorMacImpl* avfoundation_monitor_impl_;
+};
+
+void SuspendObserverDelegate::OnDeviceChanged() {
+  DCHECK(device_thread_checker_.CalledOnValidThread());
+  NSArray* devices = [AVCaptureDeviceGlue devices];
+  std::vector<DeviceInfo> snapshot_devices;
+  for (CrAVCaptureDevice* device in devices) {
+    [suspend_observer_ startObserving:device];
+    BOOL suspended = [device respondsToSelector:@selector(isSuspended)] &&
+        [device isSuspended];
+    DeviceInfo::DeviceType device_type = DeviceInfo::kUnknown;
+    if ([device hasMediaType:AVFoundationGlue::AVMediaTypeVideo()]) {
+      if (suspended)
+        continue;
+      device_type = DeviceInfo::kVideo;
+    } else if ([device hasMediaType:AVFoundationGlue::AVMediaTypeMuxed()]) {
+      device_type = suspended ? DeviceInfo::kAudio : DeviceInfo::kMuxed;
+    } else if ([device hasMediaType:AVFoundationGlue::AVMediaTypeAudio()]) {
+      device_type = DeviceInfo::kAudio;
+    }
+    snapshot_devices.push_back(DeviceInfo([[device uniqueID] UTF8String],
+                                          device_type));
+  }
+  // Post the consolidation of enumerated devices to be done on UI thread.
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&SuspendObserverDelegate::OnDeviceChangedOnUIThread,
+          this, snapshot_devices));
+}
+
+void SuspendObserverDelegate::StartObserver() {
+  DCHECK(device_thread_checker_.CalledOnValidThread());
+  suspend_observer_.reset([[CrAVFoundationDeviceObserver alloc]
+                               initWithChangeReceiver:this]);
+  for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices])
+    [suspend_observer_ startObserving:device];
+}
+
+void SuspendObserverDelegate::ResetDeviceMonitorOnUIThread() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  avfoundation_monitor_impl_ = NULL;
+}
+
+void SuspendObserverDelegate::OnDeviceChangedOnUIThread(
+    const std::vector<DeviceInfo>& snapshot_devices) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  // |avfoundation_monitor_impl_| might have been NULLed asynchronously before
+  // arriving at this line.
+  if (avfoundation_monitor_impl_) {
+    avfoundation_monitor_impl_->ConsolidateDevicesListAndNotify(
+        snapshot_devices);
+  }
+}
+
 // AVFoundation implementation of the Mac Device Monitor, registers as a global
 // device connect/disconnect observer and plugs suspend/wake up device observers
-// per device. This class is created and lives in UI thread; device enumeration
-// and operations involving |suspend_observer_| happen on |device_task_runner_|.
+// per device. Owns a SuspendObserverDelegate living in |device_task_runner_|
+// and gets notified when a device is suspended/resumed. This class is created
+// and lives in UI thread;
 class AVFoundationMonitorImpl : public DeviceMonitorMacImpl {
  public:
   AVFoundationMonitorImpl(
@@ -245,18 +327,14 @@ class AVFoundationMonitorImpl : public DeviceMonitorMacImpl {
   virtual void OnDeviceChanged() OVERRIDE;
 
  private:
-  void OnDeviceChangedOnDeviceThread(
-      const scoped_refptr<base::MessageLoopProxy>& ui_thread);
-  void StartObserverOnDeviceThread();
-
   base::ThreadChecker thread_checker_;
 
   // {Video,AudioInput}DeviceManager's "Device" thread task runner used for
-  // device enumeration, valid after MediaStreamManager calls StartMonitoring().
+  // posting tasks to |suspend_observer_delegate_|; valid after
+  // MediaStreamManager calls StartMonitoring().
   const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
 
-  // Created and executed in |device_task_runnner_|.
-  base::scoped_nsobject<CrAVFoundationDeviceObserver> suspend_observer_;
+  scoped_refptr<SuspendObserverDelegate> suspend_observer_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AVFoundationMonitorImpl);
 };
@@ -265,7 +343,8 @@ AVFoundationMonitorImpl::AVFoundationMonitorImpl(
     content::DeviceMonitorMac* monitor,
     const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner)
     : DeviceMonitorMacImpl(monitor),
-      device_task_runner_(device_task_runner) {
+      device_task_runner_(device_task_runner),
+      suspend_observer_delegate_(new SuspendObserverDelegate(this)) {
   NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
   device_arrival_ =
       [nc addObserverForName:AVFoundationGlue::
@@ -282,11 +361,13 @@ AVFoundationMonitorImpl::AVFoundationMonitorImpl(
                   usingBlock:^(NSNotification* notification) {
                       OnDeviceChanged();}];
   device_task_runner_->PostTask(FROM_HERE,
-      base::Bind(&AVFoundationMonitorImpl::StartObserverOnDeviceThread,
-          base::Unretained(this)));
+      base::Bind(&SuspendObserverDelegate::StartObserver,
+                 suspend_observer_delegate_));
 }
 
 AVFoundationMonitorImpl::~AVFoundationMonitorImpl() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  suspend_observer_delegate_->ResetDeviceMonitorOnUIThread();
   NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
   [nc removeObserver:device_arrival_];
   [nc removeObserver:device_removal_];
@@ -295,52 +376,15 @@ AVFoundationMonitorImpl::~AVFoundationMonitorImpl() {
 void AVFoundationMonitorImpl::OnDeviceChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
   device_task_runner_->PostTask(FROM_HERE,
-      base::Bind(&AVFoundationMonitorImpl::OnDeviceChangedOnDeviceThread,
-          base::Unretained(this),
-          base::MessageLoop::current()->message_loop_proxy()));
-}
-
-void AVFoundationMonitorImpl::OnDeviceChangedOnDeviceThread(
-    const scoped_refptr<base::MessageLoopProxy>& ui_thread) {
-  DCHECK(device_task_runner_->BelongsToCurrentThread());
-  NSArray* devices = [AVCaptureDeviceGlue devices];
-  std::vector<DeviceInfo> snapshot_devices;
-  for (CrAVCaptureDevice* device in devices) {
-    [suspend_observer_ startObserving:device];
-    BOOL suspended = [device respondsToSelector:@selector(isSuspended)] &&
-        [device isSuspended];
-    DeviceInfo::DeviceType device_type = DeviceInfo::kUnknown;
-    if ([device hasMediaType:AVFoundationGlue::AVMediaTypeVideo()]) {
-      if (suspended)
-        continue;
-      device_type = DeviceInfo::kVideo;
-    } else if ([device hasMediaType:AVFoundationGlue::AVMediaTypeMuxed()]) {
-      device_type = suspended ? DeviceInfo::kAudio : DeviceInfo::kMuxed;
-    } else if ([device hasMediaType:AVFoundationGlue::AVMediaTypeAudio()]) {
-      device_type = DeviceInfo::kAudio;
-    }
-    snapshot_devices.push_back(DeviceInfo([[device uniqueID] UTF8String],
-                                          device_type));
-  }
-  // Post the consolidation of enumerated devices to be done on UI thread.
-  ui_thread->PostTask(FROM_HERE,
-      base::Bind(&DeviceMonitorMacImpl::ConsolidateDevicesListAndNotify,
-          base::Unretained(this), snapshot_devices));
-}
-
-void AVFoundationMonitorImpl::StartObserverOnDeviceThread() {
-  DCHECK(device_task_runner_->BelongsToCurrentThread());
-  suspend_observer_.reset([[CrAVFoundationDeviceObserver alloc]
-                               initWithChangeReceiver:this]);
-  for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices])
-    [suspend_observer_ startObserving:device];
+      base::Bind(&SuspendObserverDelegate::OnDeviceChanged,
+                 suspend_observer_delegate_));
 }
 
 }  // namespace
 
 @implementation CrAVFoundationDeviceObserver
 
-- (id)initWithChangeReceiver:(AVFoundationMonitorImpl*)receiver {
+- (id)initWithChangeReceiver:(SuspendObserverDelegate*)receiver {
   if ((self = [super init])) {
     DCHECK(receiver != NULL);
     receiver_ = receiver;