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

chrome/browser/system_monitor/image_capture_camera.h

+// 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.
+
+#ifndef CHROME_BROWSER_SYSTEM_MONITOR_IMAGE_CAPTURE_CAMERA_H_
+#define CHROME_BROWSER_SYSTEM_MONITOR_IMAGE_CAPTURE_CAMERA_H_
+
+#import <Foundation/Foundation.h>
+#import <ImageCaptureCore/ImageCaptureCore.h>
+
+#include "base/file_path.h"
+#include "base/mac/foundation_util.h"
+#include "base/memory/scoped_nsobject.h"
+#include "base/memory/weak_ptr.h"
+#include "base/platform_file.h"
+#include "base/string_util.h"
+#include "base/sys_string_conversions.h"
+
+// Client's use this listener interface to get notifications about
+// events happening as a particular ImageCapture device is interacted with.
+// Clients drive the interaction through the ImageCaptureDeviceBrowserMac
+// and the ImageCaptureCameraInterface classes, and get notifications of
+// events through this interface.
+class ImageCaptureDeviceListener {
+ public:
+  virtual ~ImageCaptureDeviceListener() {}
+
+  // Get a notification that a particular item has been found on the device.
+  // These calls will come automatically after a new device is initialized.
+  virtual void ItemAdded(const std::string& name,
+                         const base::PlatformFileInfo& info) = 0;
+
+  // Called when there are no more items to retrieve.
+  virtual void NoMoreItems() = 0;
+
+  // Called upon completion of a file download request. The |path| is the
+  // requested download file. Note: in NOT_FOUND error case, can be called
+  // inline with the download request.
+  virtual void DownloadedFile(const std::string& name,
+                              base::PlatformFileError error) = 0;
+
+  // Called to let the client know the device is removed.
+  virtual void DeviceRemoved() = 0;
+};
+
+// Interface to a camera device found by ImageCaptureCore. This class manages a
+// session to the camera and provides the backing interactions to present the
+// media files on it to the filesystem delegate. FilePaths will be artificial,
+// like "/$device_id/" + name.
+// Note on thread-safety. This class ends up involving three different threads.
+// The client is expected to interact with setting up and tearing down the
+// implementation within a single thread context which allows blocking
+// (likely the browser blocking pool). The class is thread-compatible with that
+// usage. It receives events from the ImageCaptureCore library on the UI thread,
+// and will forward callbacks to the listener on that thread. It also uses the
+// File thread for renaming files when necessary.
+// In other words, setup and teardown need to be threaded, and for other
+// interactions the caller needs to be thread-safe. For those reasons, the
+// client supplies a WeakPtr callback interface.

[sail, 2012/12/14 19:27:32]

The reference to WeakPtr can be removed. WeakPtr isn't thread safe (it's just
there for asynchronous callbacks).

[Greg Billock, 2012/12/14 22:03:56]

Oops! Editing here was imperfect. Just removed this.

I thought earlier about bouncing all the callbacks through a provided pool.
That's really obnoxious, though. You think it's more obnoxious than demanding
the client be thread-safe? Or is the expected convention that these callbacks
will come on the UI thread, so I should just trampoline them to there. The thing
is, that's doable, but a bit of work, and in practice will still mean the caller
needs to be thread-safe. That's why I thought it wasn't worth it.

I think we need a weakptr due to async callbacks, and if we thread-forward then
obviously the weakptr will be nice for that, so I'm happy with it, but my
instinct in tricky cases like this is usually to push complication out to
clients. (A thread adapter client would be easy to write, for instance, and then
isolates all that stuff away from the guts of this interface.)

On 2012/12/14 19:27:32, sail wrote:

[sail, 2012/12/14 22:14:48]

WeakPtr is not thread safe. That means you can't pass it to RenameFileAndReturn.

By having RenameFileAndReturn perform the callback on the UI thread you
encapsulate all the threading issues into a single function.

+@interface ImageCaptureCameraInterface
+    : NSObject<ICCameraDeviceDelegate, ICCameraDeviceDownloadDelegate> {
+ @private
+  scoped_nsobject<ICCameraDevice> camera_;
+  // Weak pointer to this class' client.

[sail, 2012/12/14 19:27:32]

don't need this comment anymore

[Greg Billock, 2012/12/14 22:03:56]

Done.

+  base::WeakPtr<ImageCaptureDeviceListener> listener_;
+}
+
+- (id)initWithCameraDevice:(ICCameraDevice*)cameraDevice;
+- (void)open;
+- (void)close;
+- (void)setListener:(base::WeakPtr<ImageCaptureDeviceListener>)listener;
+
+// Download the given |file| to the provided |local_path|. Completion notice
+// will be sent to the listener's DownloadedFile method.
+- (void)downloadFile:(const std::string&)name
+           localPath:(const FilePath&)localPath;
+
+@end
+
+#endif  // CHROME_BROWSER_SYSTEM_MONITOR_IMAGE_CAPTURE_CAMERA_H_