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

chrome/browser/media_gallery/win/recursive_mtp_device_object_enumerator.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_MEDIA_GALLERY_WIN_RECURSIVE_MTP_DEVICE_OBJECT_ENUMERATOR_H_
+#define CHROME_BROWSER_MEDIA_GALLERY_WIN_RECURSIVE_MTP_DEVICE_OBJECT_ENUMERATOR_H_
+
+#include <portabledeviceapi.h>

[Lei Zhang, 2013/01/14 23:25:30]

nit: empty line between C and C++ headers. Here and elsewhere.

[kmadhusu, 2013/01/15 19:08:17]

I had an empty line before. Reviewers asked me to remove the extra line.

[Lei Zhang, 2013/01/15 21:00:13]

I ran into the same issue on my readability review. In the end, we got one of
the C++ style guide authors to clarify this:

"... (thus there’s a mix of styles in the codebase), and the text of the guide
does not require or forbid any particular convention for blank line, then the
“when in Rome” principle should apply. In Chrome code, the only style we have
places a blank line between the C and C++ system header sections."

That said, I've noticed even base/*.h is not consistent on this. I would also
encourage consistency with surrounding code, but there aren't any other files
that includes C headers in c/b/media_galleries. The closest relative is
c/b/system_monitor, which has the blank line.

[kmadhusu, 2013/01/15 23:41:58]

Done.

+#include <queue>
+#include <vector>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/string16.h"
+#include "base/threading/thread_checker.h"
+#include "base/time.h"
+#include "base/win/scoped_comptr.h"
+#include "chrome/browser/media_gallery/win/mtp_device_object_entry.h"
+#include "webkit/fileapi/file_system_file_util.h"
+
+class FilePath;
+
+namespace chrome {
+
+// RecursiveMTPDeviceObjectEnumerator is used to recursively enumerate the
+// media transfer protocol (MTP) device storage objects from a given media file
+// object entries set. RecursiveMTPDeviceObjectEnumerator communicates with the

[Lei Zhang, 2013/01/14 23:25:30]

Doesn't it go through MTPDeviceObjectEnumerator?

[kmadhusu, 2013/01/15 19:08:17]

RecursiveMTPDeviceObjectEnumerator communicates with the device to get the
subdirectory object entries. RecursiveMTPDeviceObjectEnumerator uses
MTPDeviceObjectEnumerator to iterate the current directory objects.

[Lei Zhang, 2013/01/15 21:00:13]

That should have gone in the comments and not in the reply here.

[kmadhusu, 2013/01/15 23:41:58]

Done.

+// MTP device to get the removable storage objects details.
+// RecursiveMTPDeviceObjectEnumerator supports media file system operations.

[Lei Zhang, 2013/01/14 23:25:30]

What does this mean exactly? Are you trying to say it implements
AbstractFileEnumerator?

[kmadhusu, 2013/01/15 19:08:17]

RecursiveMTPDeviceObjectEnumerator is used to complete the DOMFileSystem
operation, e.g DirectoryEntry.removeRecursively interface. In our case, it is a
media gallery file system operation.

[Lei Zhang, 2013/01/15 21:00:13]

Ok, but I don't see how this comment is useful. Aren't all the classes in this
directory used to "support media file system operations" ?

[kmadhusu, 2013/01/15 23:41:58]

Removed.

+// RecursiveMTPDeviceObjectEnumerator may only be used on a single thread. MTP
+// device is already opened for communication.

[Lei Zhang, 2013/01/14 23:25:30]

Are you trying to say the |device| passed into the ctor should already be
initialized and ready for communication? Shouldn't this statement be in the ctor
comment?

[kmadhusu, 2013/01/15 19:08:17]

Done.

+class RecursiveMTPDeviceObjectEnumerator
+    : public fileapi::FileSystemFileUtil::AbstractFileEnumerator {
+ public:
+  RecursiveMTPDeviceObjectEnumerator(IPortableDevice* device,
+                                     const MTPDeviceObjectEntries& entries);
+  virtual ~RecursiveMTPDeviceObjectEnumerator();
+
+  // AbstractFileEnumerator:
+  virtual FilePath Next() OVERRIDE;
+  virtual int64 Size() OVERRIDE;
+  virtual bool IsDirectory() OVERRIDE;
+  virtual base::Time LastModifiedTime() OVERRIDE;
+
+ private:
+  typedef string16 DirectoryObjectId;
+
+  void MaybeUpdateCurrentObjectList();
+
+  // The portable device.
+  base::win::ScopedComPtr<IPortableDevice> device_;
+
+  // List of current directory object entries.

[Lei Zhang, 2013/01/14 23:25:30]

Can you add a TODO to remove |curr_object_entries_| and |object_entry_iter_| ?

[kmadhusu, 2013/01/15 19:08:17]

Done.

+  MTPDeviceObjectEntries curr_object_entries_;
+
+  // Iterator to access the individual file object entries.
+  MTPDeviceObjectEntries::const_iterator object_entry_iter_;
+
+  // Enumerator to access current directory object entries.
+  scoped_ptr<fileapi::FileSystemFileUtil::AbstractFileEnumerator>
+      current_enumerator_;
+
+  // Used to recursively enumerate the sub-directory objects.
+  std::queue<DirectoryObjectId> unparsed_directory_object_ids_;
+
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(RecursiveMTPDeviceObjectEnumerator);
+};
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_MEDIA_GALLERY_WIN_RECURSIVE_MTP_DEVICE_OBJECT_ENUMERATOR_H_