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

chrome/browser/system_monitor/image_capture_device.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.h"
+
+#include "base/file_util.h"
+#include "base/system_monitor/system_monitor.h"
+#include "chrome/browser/system_monitor/media_storage_util.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace {
+
+void RenameFileAndReturn(const std::string& name,
+                         const FilePath& downloaded_filename,
+                         const FilePath& desired_filename,
+                         ImageCaptureDevice* device) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+  bool success = file_util::ReplaceFile(downloaded_filename, desired_filename);
+  [device renamedFile:name
+               result:(success ? base::PLATFORM_FILE_OK
+                               : base::PLATFORM_FILE_ERROR_NOT_FOUND)];
+}
+
+base::Time NSDateToBaseTime(NSDate* date) {
+  return base::Time::FromDoubleT([date timeIntervalSince1970]);
+}
+
+void ListenerItemAdded(base::WeakPtr<ImageCaptureDeviceListener> listener,
+                       std::string name,
+                       base::PlatformFileInfo info) {
+  listener->ItemAdded(name, info);
+}
+
+void ListenerNoMoreItems(base::WeakPtr<ImageCaptureDeviceListener> listener) {
+  listener->NoMoreItems();
+}
+
+void ListenerDownloadedFile(base::WeakPtr<ImageCaptureDeviceListener> listener,
+                            std::string name,
+                            base::PlatformFileError error) {
+  listener->DownloadedFile(name, error);
+}
+
+void ListenerDeviceRemoved(base::WeakPtr<ImageCaptureDeviceListener> listener) {
+  listener->DeviceRemoved();
+}
+
+}  // namespace
+
+@implementation ImageCaptureDevice
+
+- (id)initWithCameraDevice:(ICCameraDevice*)cameraDevice {
+  if ((self = [super init])) {
+    camera_.reset([cameraDevice retain]);
+    [camera_ setDelegate:self];
+  }
+  return self;
+}
+
+- (void)setListener:(base::WeakPtr<ImageCaptureDeviceListener>)listener
+         taskRunner:(base::TaskRunner*)pool {
+  listener_ = listener;
+  pool_ = pool;
+}
+
+- (void)open {
+  [camera_ requestOpenSession];
+}
+
+- (void)dealloc {
+  // Make sure the session was closed before destruction.
+  DCHECK(![camera_ delegate]);
+  [super dealloc];
+}
+
+- (void)close {
+  [camera_ requestCloseSession];
+  [camera_ setDelegate:nil];
+}
+
+- (void)downloadFile:(const std::string&)name
+           localPath:(const FilePath&)localPath {
+  // Find the file with that name and start download.
+  for (ICCameraItem* item in [camera_ mediaFiles]) {
+    std::string itemName = base::SysNSStringToUTF8([item name]);
+    if (itemName == name) {
+      // To create save options for ImageCapture, we need to
+      // split the target filename into directory/name
+      // and encode the directory as a URL.
+      NSString* saveDirectory =
+          base::mac::FilePathToNSString(localPath.DirName());
+      NSString* saveFilename =
+          base::mac::FilePathToNSString(localPath.BaseName());
+
+      NSMutableDictionary* options =
+          [NSMutableDictionary dictionaryWithCapacity:3];
+      [options setObject:[NSURL fileURLWithPath:saveDirectory isDirectory:YES]
+                  forKey:ICDownloadsDirectoryURL];
+      [options setObject:saveFilename forKey:ICSaveAsFilename];
+      [options setObject:[NSNumber numberWithBool:YES] forKey:ICOverwrite];
+
+      [camera_ requestDownloadFile:base::mac::ObjCCastStrict<ICCameraFile>(item)
+                           options:options
+                  downloadDelegate:self
+               didDownloadSelector:
+                   @selector(didDownloadFile:error:options:contextInfo:)
+                       contextInfo:NULL];
+      return;
+    }
+  }
+
+  if (pool_.get()) {
+    pool_->PostTask(FROM_HERE,
+                    base::Bind(&ListenerDownloadedFile,
+                               listener_,
+                               name,
+                               base::PLATFORM_FILE_ERROR_NOT_FOUND));
+  }
+}
+
+- (void)cameraDevice:(ICCameraDevice*)camera didAddItem:(ICCameraItem*)item {
+  std::string name = base::SysNSStringToUTF8([item name]);
+  base::PlatformFileInfo info;
+  if ([[item UTI] isEqualToString:base::mac::CFToNSCast(kUTTypeFolder)])
+    info.is_directory = true;
+  else
+    info.size = [base::mac::ObjCCastStrict<ICCameraFile>(item) fileSize];
+  info.last_modified = NSDateToBaseTime([item modificationDate]);
+  info.creation_time = NSDateToBaseTime([item creationDate]);
+  info.last_accessed = info.last_modified;
+
+  if (pool_.get()) {
+    pool_->PostTask(FROM_HERE,
+                    base::Bind(&ListenerItemAdded,
+                               listener_,
+                               name, info));
+  }
+}
+- (void)cameraDevice:(ICCameraDevice*)camera didAddItems:(NSArray*)items {
+  for (ICCameraItem* item in items)
+    [self cameraDevice:camera didAddItem:item];
+}
+
+- (void)didRemoveDevice:(ICDevice*)device {
+  device.delegate = NULL;
+  if (pool_.get()) {
+    pool_->PostTask(FROM_HERE,
+                    base::Bind(&ListenerDeviceRemoved,
+                               listener_));
+  }
+}
+
+// Notifies that a session was opened with the given device; potentially
+// with an error.
+- (void)device:(ICDevice*)device didOpenSessionWithError:(NSError*)error {
+  if (error)
+    [self didRemoveDevice:camera_];
+}
+
+- (void)device:(ICDevice*)device didEncounterError:(NSError*)error {
+  if (error) {
+    if (pool_.get()) {
+      pool_->PostTask(FROM_HERE,
+                      base::Bind(&ListenerDeviceRemoved, listener_));
+    }
+  }
+}
+
+// When this message is received, all media metadata is now loaded.
+- (void)deviceDidBecomeReadyWithCompleteContentCatalog:(ICDevice*)device {
+  if (pool_.get()) {
+    pool_->PostTask(FROM_HERE,
+                    base::Bind(&ListenerNoMoreItems, listener_));
+  }
+}
+
+- (void)didDownloadFile:(ICCameraFile*)file
+                  error:(NSError*)error
+                options:(NSDictionary*)options
+            contextInfo:(void*)contextInfo {
+  std::string name = base::SysNSStringToUTF8([file name]);
+
+  if (error) {
+    if (pool_.get()) {
+      pool_->PostTask(FROM_HERE,
+                      base::Bind(&ListenerDownloadedFile,
+                                 listener_,
+                                 name,
+                                 base::PLATFORM_FILE_ERROR_FAILED));
+    }
+    return;
+  }
+
+  std::string savedFilename =
+      base::SysNSStringToUTF8([options objectForKey:ICSavedFilename]);
+  std::string saveAsFilename =
+      base::SysNSStringToUTF8([options objectForKey:ICSaveAsFilename]);
+  if (savedFilename == saveAsFilename) {
+    if (pool_.get()) {
+      pool_->PostTask(FROM_HERE,
+                      base::Bind(&ListenerDownloadedFile,
+                                 listener_,
+                                 name,
+                                 base::PLATFORM_FILE_OK));
+    }
+    return;
+  }
+
+  // ImageCapture did not save the file into the name we give it in the
+  // options. It picks a new name according to its best lights, so we need
+  // to rename the file.
+  FilePath save_dir(base::SysNSStringToUTF8(
+      [[options objectForKey:ICDownloadsDirectoryURL] path]));
+  FilePath saveAsPath = save_dir.Append(saveAsFilename);
+  FilePath savedPath = save_dir.Append(savedFilename);
+  [self retain];
+  content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&RenameFileAndReturn,
+                 name, savedPath, saveAsPath, self));
+}
+
+// Called upon successful rename
+- (void)renamedFile:(const std::string&)name
+             result:(base::PlatformFileError)errorCode {
+  [self release];

[sail, 2012/12/19 20:41:39]

this isn't a good idea on the file thread

[Greg Billock, 2012/12/19 21:19:17]

Really? are the NSObject refcounters not threadsafe? Will creation be safe,
then? Because that's likely done in neither the UI nor FILE threads.

[sail, 2012/12/19 21:32:49]

The release may cause the object to be deleted. This means that the classes
destructor and all member variables need to be thread safe which they are not.

[Greg Billock, 2012/12/19 22:08:55]

Docs specifically say it is safe to delete WeakPtr on another thread. Which
member variable is the concern?

I'm still a bit worried that there's a hole here where WeakPtr can be
invalidated one place and then called, but I think I've read it carefully enough
now that I'm sure that won't happen. Still, it is a bit subtle. I currently
believe that if the caller manages itself on a single thread, the way WeakPtr
wants to be, that this class is OK.

[sail, 2012/12/19 22:52:05]

I don't think you are correct.

Basically WeakPtr<Foo> is the same as Foo* except that it does some magic when
Foo is deleted.

So instead of WeakPtr<Foo> my_ptr, lets imagine you just have Foo* my_ptr. In
this function you are referencing my_ptr at the same time another function (lets
say setListener) might be referencing it as well.

To fix this you either have to make this class thread safe or do a (very very
very simple) modification I suggested 6 days ago.

+  if (pool_.get()) {
+    pool_->PostTask(FROM_HERE,
+                    base::Bind(&ListenerDownloadedFile,
+                               listener_, name, errorCode));

[sail, 2012/12/19 20:41:39]

you can't do this on the file thread

[Greg Billock, 2012/12/19 21:19:17]

What do you mean by "this"?

[sail, 2012/12/19 21:32:49]

this -> listener_

[Greg Billock, 2012/12/19 22:08:55]

The docs say specifically that you can copy WeakPtr on other threads, just not
use it.

On 2012/12/19 21:32:49, sail wrote:

+  }
+}
+
+@end  // ImageCaptureDevice