[Media Gallery] Added code to support mtp device media file system on Windows.

chrome/browser/media_gallery/win/mtp_get_storage_info_worker.h

Index: chrome/browser/media_gallery/win/mtp_get_storage_info_worker.h
diff --git a/chrome/browser/media_gallery/win/mtp_get_storage_info_worker.h b/chrome/browser/media_gallery/win/mtp_get_storage_info_worker.h
new file mode 100644
index 0000000000000000000000000000000000000000..0900a347223464bf256f531fa5a8ba5f16ae8875
--- /dev/null
+++ b/chrome/browser/media_gallery/win/mtp_get_storage_info_worker.h
@@ -0,0 +1,118 @@
+// 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.
+//
+// This class gets the media transfer protocol (MTP) device storage details to
+// support MTP device file operations.
+
+#ifndef CHROME_BROWSER_MEDIA_GALLERY_WIN_MTP_GET_STORAGE_INFO_WORKER_H_
+#define CHROME_BROWSER_MEDIA_GALLERY_WIN_MTP_GET_STORAGE_INFO_WORKER_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/sequenced_task_runner_helpers.h"
+#include "base/string16.h"
+#include "base/synchronization/cancellation_flag.h"
+
+namespace base {
+class SequencedTaskRunner;
+class WaitableEvent;
+}
+
+namespace chrome {
+
+class MTPDeviceDelegateImplWin;
+struct MTPGetStorageInfoWorkerDeleter;
+
+// Gets the details of the MTP device storage specified by the storage path.
+// This object lives on a blocking pool thread, except for DoWorkOnUIThread()
+// which happens on the UI thread.
+class MTPGetStorageInfoWorker
+    : public base::RefCountedThreadSafe<MTPGetStorageInfoWorker,
+                                        MTPGetStorageInfoWorkerDeleter> {

[Ryan Sleevi, 2013/01/05 03:00:58]

Objects that live on multiple threads should be strongly discouraged as a design
principle. It appears you made this RefCounted solely for that reason, which is
one of the Chromium anti-patterns.

+ public:
+  MTPGetStorageInfoWorker(const string16& storage_path,
+                          base::SequencedTaskRunner* task_runner,
+                          MTPDeviceDelegateImplWin* device_delegate,
+                          base::WaitableEvent* task_completed_event);
+
+  // This function is invoked on the blocking pool thread to post the task on
+  // the UI thread. This blocks the blocking pool thread until the task is
+  // complete.
+  void Run();

[Ryan Sleevi, 2013/01/05 03:00:58]

DESIGN: This is a broken design here. You should not arbitrarily block a thread
on the UI thread. That's not what the "blocking" pool is for.

+
+  // Returns true and populates |pnp_device_id| and |storage_object_id|, if the
+  // storage information was fetched successfully. Otherwise, returns false.
+  bool GetDeviceStorageInfo(string16* pnp_device_id,
+                            string16* storage_object_id) const;
+
+  // Returns the blocking pool thread associated with this worker object.
+  // This function is exposed for MTPGetStorageInfoWorkerDeleter struct to
+  // access the blocking pool thread.
+  base::SequencedTaskRunner* media_task_runner() const {
+    return media_task_runner_.get();
+  }
+
+ private:
+  friend struct MTPGetStorageInfoWorkerDeleter;
+  friend class base::DeleteHelper<MTPGetStorageInfoWorker>;
+  friend class base::RefCountedThreadSafe<MTPGetStorageInfoWorker,
+                                          MTPGetStorageInfoWorkerDeleter>;
+
+  // Destructed via MTPGetStorageInfoWorkerDeleter.
+  virtual ~MTPGetStorageInfoWorker();
+
+  // Gets the storage details and signals to unblock the blocking pool thread.
+  void DoWorkOnUIThread();
+
+  // Registered device storage path.
+  const string16 storage_path_;
+
+  // The task runner where most of the class lives and runs (save for
+  // MTPGetStorageInfoWorkerDeleter).
+  scoped_refptr<base::SequencedTaskRunner> media_task_runner_;
+
+  // Used to ignore the request results. Dereferenced on the blocking pool
+  // thread. It is safe to use this as raw pointer because
+  // MTPDeviceDelegateImplWin is destructed by the sequenced task runner on the
+  // blocking pool thread (by the time MTPGetStorageInfoWorker access this
+  // pointer, |device_delegate_| is guaranteed to be valid).
+  MTPDeviceDelegateImplWin* device_delegate_;

[Ryan Sleevi, 2013/01/05 03:00:58]

DESIGN: This sort of design coupling between classes is a signal of poor
ownership semantics that will be an ongoing maintenance burden.

+
+  /***************************************************************************/
+  // Following member variables are normally accessed on the blocking pool
+  // thread. But it is also safe to access on the UI thread when the blocking
+  // pool thread is blocked.
+
+  // The plug and play device identifier,
+  // e.g. "\\?\usb#vid_04a9&pid_3073#12#{6ac27878-a6fa-4155-ba85-f1d4f33}".
+  string16 pnp_device_id_;
+
+  // The MTP device storage partition object identifier, e.g. "s10001".
+  string16 storage_object_id_;
+
+  // Device storage information request result.
+  bool result_;
+  /***************************************************************************/
+
+  // The blocking pool thread can wait on this event until the storage
+  // information is fetched. MTPDeviceDelegateImplWin signals this event during
+  // browser shutdown to unblock the blocking pool thread.
+  // TODO(kmadhusu): Remove this WaitableEvent after modifying the
+  // DeviceMediaFileUtil functions as asynchronous functions (crbug.com/154835).
+  base::WaitableEvent* on_task_completed_event_;

[Ryan Sleevi, 2013/01/05 03:00:58]

I'm very concerned that this sort of technical debt is only going to be added,
and that there's no incentives to pay it down once this code lands.

I think this should be prioritized over the Windows implementation.

+
+  // Set to ignore the request results. This will be set when
+  // MTPDeviceDelegateImplWin object is scheduled to be deleted.
+  // |on_task_completed_event_| should not be dereferenced when this is set.
+  base::CancellationFlag cancel_tasks_flag_;
+
+  DISALLOW_COPY_AND_ASSIGN(MTPGetStorageInfoWorker);
+};
+
+struct MTPGetStorageInfoWorkerDeleter {
+  static void Destruct(const MTPGetStorageInfoWorker* worker);
+};
+
+}  // namepsace chrome
+
+#endif  // CHROME_BROWSER_MEDIA_GALLERY_WIN_MTP_GET_STORAGE_INFO_WORKER_H_