[Media Galleries] Add an ImageCaptureCore listener for Mac. (part 2)

chrome/browser/system_monitor/image_capture_device_browser_mac.mm

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/system_monitor/image_capture_device_browser_mac.h"
+
+#import "chrome/browser/system_monitor/image_capture_camera.h"
+#include "base/file_util.h"
+#include "base/system_monitor/system_monitor.h"
+#include "chrome/browser/system_monitor/disk_info_mac.h"
+#include "chrome/browser/system_monitor/media_storage_util.h"
+#include "content/public/browser/browser_thread.h"
+
+ImageCaptureDeviceBrowserMac* g_image_capture_device_browser;

[sail, 2012/12/13 02:14:00]

don't need this

+
+@implementation ImageCaptureDeviceBrowserMac
+
+@synthesize cameras = cameras_;
+
+- (id)init {

[sail, 2012/12/13 02:14:00]

if ((self = [super init])) {
}
return self

[Greg Billock, 2012/12/14 00:39:59]

Done. This guards the singleton?

On 2012/12/13 02:14:00, sail wrote:

+  cameras_ = [[NSMutableArray alloc] initWithCapacity:0];

[sail, 2012/12/13 02:14:00]

just init]

[Greg Billock, 2012/12/14 00:39:59]

Done.

+
+  deviceBrowser_.reset([[ICDeviceBrowser alloc] init]);
+  deviceBrowser_.get().delegate = self;

[sail, 2012/12/13 02:14:00]

[deviceBrowser setDelegate:self]
more below

[Greg Billock, 2012/12/14 00:39:59]

Done.

+  deviceBrowser_.get().browsedDeviceTypeMask =
+      deviceBrowser_.get().browsedDeviceTypeMask |

[sail, 2012/12/13 02:14:00]

[deviceBrowser browsedDeviceTypeMask]

[Greg Billock, 2012/12/14 00:39:59]

Done.

+      ICDeviceTypeMaskCamera | ICDeviceLocationTypeMaskLocal;
+  [deviceBrowser_ start];
+  g_image_capture_device_browser = self;
+  return self;
+}
+
+- (void)close {
+  deviceBrowser_.get().delegate = NULL;
+  [deviceBrowser_ stop];
+  [deviceBrowser_ release];
+  [cameras_ release];
+}
+
+- (ImageCaptureCameraInterface*)createCameraInterfaceForUUID:

[sail, 2012/12/13 02:14:00]

break the lien after the ) instead

[Greg Billock, 2012/12/14 00:39:59]

Done.

+    (std::string&)uuid {
+  for (ICCameraDevice* camera in cameras_) {
+    NSString* camera_id = [camera UUIDString];
+    if (base::SysNSStringToUTF8(camera_id) == uuid) {

[sail, 2012/12/13 02:14:00]

no braces

[Greg Billock, 2012/12/14 00:39:59]

Done.

+      return [[[ImageCaptureCameraInterface alloc] init:camera] autorelease];
+    }
+  }
+  return nil;
+}
+
++ (ImageCaptureDeviceBrowserMac*)get {
+  return g_image_capture_device_browser;
+}
+
+// Device browser maintains list of cameras as key-value pairs, so delegate
+// must call willChangeValueForKey to modify list.
+- (void)deviceBrowser:(ICDeviceBrowser*)browser

[sail, 2012/12/13 02:14:00]

align colons

[Greg Billock, 2012/12/14 00:39:59]

Done.

+      didAddDevice:(ICDevice*)addedDevice moreComing:(BOOL)moreComing {

[sail, 2012/12/13 02:14:00]

one line per argument

[Greg Billock, 2012/12/14 00:39:59]

Done.

+  if (!(addedDevice.type & ICDeviceTypeCamera))
+    return;
+
+  ICCameraDevice* cameraDevice = (ICCameraDevice*)addedDevice;

[sail, 2012/12/13 02:14:00]

no C style casts

[Greg Billock, 2012/12/14 00:39:59]

Done. Is there an ObjC style that's better?

On 2012/12/13 02:14:00, sail wrote:

+
+  NSString* name = [addedDevice name];

[sail, 2012/12/13 02:14:00]

no need for local variables if this is only used once
(more below)

[Greg Billock, 2012/12/14 00:39:59]

Done.

+  NSString* mountPoint = [cameraDevice mountPoint];
+  NSString* uuid = [cameraDevice UUIDString];
+
+  // implement manual observer notification for the cameras property
+  [self willChangeValueForKey:@"cameras"];
+  [cameras_ addObject:addedDevice];
+  [self didChangeValueForKey:@"cameras"];
+
+  chrome::DiskInfoMac info = chrome::DiskInfoMac::BuildDiskInfoFromICDevice(

[sail, 2012/12/13 02:14:00]

this isn't need, just use the data directly
the function should be removed from disk_info_mac.h also

[Greg Billock, 2012/12/14 00:39:59]

I'd rather keep the purpose-built factories than add setters for all the
variables.

[sail, 2012/12/14 01:26:32]

I don't understand this.
There's nothing in disk info that you're using.
Just do:
base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
  chrome::MediaStorageUtil::MakeDeviceId(
      MediaStorageUtil::MAC_IMAGE_CAPTURE, uuid),
  name,
  mountPoint)

[Greg Billock, 2012/12/14 17:29:22]

Oy, thanks. The prehistory is that this class was owned by the
RemovableDeviceNotificationsMac, but ObjC includes made that a pain.

I've separately disentangled that, so I wonder whether that may end up making
more sense than having it directly owned by the BrowserPartsMain at this point.
Lei, what do you think? It kind of depends on whether we end up handling mass
storage style attaches through ImageCapture or directly.

+      base::SysNSStringToUTF8(uuid),
+      base::SysNSStringToUTF16(name),
+      FilePath(base::SysNSStringToUTF8(mountPoint)));
+  base::SystemMonitor::Get()->ProcessRemovableStorageAttached(
+      chrome::MediaStorageUtil::MakeDeviceId(info.type(), info.device_id()),
+      info.device_name(), info.mount_point().value());
+}
+
+- (void)deviceBrowser:(ICDeviceBrowser*)browser
+      didRemoveDevice:(ICDevice*)device moreGoing:(BOOL)moreGoing {
+  if (!(device.type & ICDeviceTypeCamera))
+    return;
+
+  NSString* name = [device name];
+
+  ICCameraDevice* cameraDevice = (ICCameraDevice*)device;
+  NSString* mountPoint = [cameraDevice mountPoint];
+  NSString* uuid = [cameraDevice UUIDString];
+
+  // implement manual observer notification for the cameras property
+  [self willChangeValueForKey:@"cameras"];
+  [cameras_ removeObject:device];
+  [self didChangeValueForKey:@"cameras"];
+
+  chrome::DiskInfoMac info = chrome::DiskInfoMac::BuildDiskInfoFromICDevice(
+      base::SysNSStringToUTF8(uuid),
+      base::SysNSStringToUTF16(name),
+      FilePath(base::SysNSStringToUTF8(mountPoint)));
+  base::SystemMonitor::Get()->ProcessRemovableStorageDetached(
+      chrome::MediaStorageUtil::MakeDeviceId(info.type(), info.device_id()));
+}
+
+@end  // ImageCaptureDeviceBrowserMac