Media Galleries: Access MTP devices by file ids rather than file paths.

chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h

Index: chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h
diff --git a/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h b/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h
index 0a698e6c7c2b18bb9ec5c1507211b3f408bc8fa0..86dd7d6e444f6f21cc3d09fb5d58935ba7928078 100644
--- a/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h
+++ b/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h
@@ -5,9 +5,13 @@
 #ifndef CHROME_BROWSER_MEDIA_GALLERIES_LINUX_MTP_DEVICE_DELEGATE_IMPL_LINUX_H_
 #define CHROME_BROWSER_MEDIA_GALLERIES_LINUX_MTP_DEVICE_DELEGATE_IMPL_LINUX_H_
 
+#include <map>
 #include <queue>
+#include <set>
+#include <string>
 
 #include "base/callback.h"
+#include "base/containers/scoped_ptr_hash_map.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -48,6 +52,43 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
     const base::Closure task;
   };
 
+  class MTPFileNode;

[tommycli, 2014/07/09 21:48:31]

Either remove fwd decl or move inner class decl to cc file.

I think I prefer it to be in the cc file.

[Lei Zhang, 2014/07/14 23:46:33]

Done.

+
+  // Maps file ids to file nodes.
+  typedef std::map<uint32, MTPFileNode*> FileIdToMTPFileNodeMap;
+
+  // Container for holding a node's children.
+  typedef base::ScopedPtrHashMap<std::string, MTPFileNode> ChildNodes;

[tommycli, 2014/07/09 21:48:31]

Since this is only used in the inner class, it should be declared in inner
class.

[Lei Zhang, 2014/07/14 23:46:33]

Done.

+
+  // Represents a file on the MTP device.
+  class MTPFileNode {
+   public:
+    MTPFileNode(uint32 file_id,
+                MTPFileNode* parent,
+                FileIdToMTPFileNodeMap* file_id_to_node_map);
+    ~MTPFileNode();
+
+    const MTPFileNode* GetChild(const std::string& name) const;
+
+    void AddChild(const std::string& name, uint32 id);
+
+    // Clears all the children, except those in |children_to_keep|.
+    void ClearChildren(const std::set<std::string>& children_to_keep);
+
+    bool DeleteChild(uint32 file_id);
+
+    uint32 file_id() const { return file_id_; }
+    MTPFileNode* parent() { return parent_; }
+
+   private:
+    const uint32 file_id_;
+    ChildNodes children_;
+    MTPFileNode* const parent_;
+    FileIdToMTPFileNodeMap* file_id_to_node_map_;
+
+    DISALLOW_COPY_AND_ASSIGN(MTPFileNode);
+  };
+
   // Should only be called by CreateMTPDeviceAsyncDelegate() factory call.
   // Defer the device initializations until the first file operation request.
   // Do all the initializations in EnsureInitAndRunTask() function.
@@ -111,16 +152,16 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
                         const base::File::Info& file_info);
 
   // Called when GetFileInfo() succeeds. GetFileInfo() is invoked to
-  // get the |root| directory metadata details. |file_info| specifies the |root|
-  // directory details.
+  // get the |dir_id| directory metadata details. |file_info| specifies the
+  // |dir_id| directory details.
   //
-  // If |root| is a directory, post a task on the UI thread to read the |root|
-  // directory file entries.
+  // If |dir_id| is a directory, post a task on the UI thread to read the
+  // |dir_id| directory file entries.
   //
-  // If |root| is not a directory, |error_callback| is invoked to notify the
+  // If |dir_id| is not a directory, |error_callback| is invoked to notify the
   // caller about the file error and process the next pending request.
   void OnDidGetFileInfoToReadDirectory(
-      const std::string& root,
+      uint32 dir_id,
       const ReadDirectorySuccessCallback& success_callback,
       const ErrorCallback& error_callback,
       const base::File::Info& file_info);
@@ -137,10 +178,12 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
 
   // Called when ReadDirectory() succeeds.
   //
-  // |file_list| contains the directory file entries.
+  // |dir_id| is the directory read.
+  // |file_list| contains the directory file entries with their file ids.
   // |success_callback| is invoked to notify the caller about the directory
   // file entries.
-  void OnDidReadDirectory(const ReadDirectorySuccessCallback& success_callback,
+  void OnDidReadDirectory(uint32 dir_id,
+                          const ReadDirectorySuccessCallback& success_callback,
                           const fileapi::AsyncFileUtil::EntryList& file_list);
 
   // Called when WriteDataIntoSnapshotFile() succeeds.
@@ -168,11 +211,28 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
   void OnDidReadBytes(const ReadBytesSuccessCallback& success_callback,
                       const base::File::Info& file_info, int bytes_read);
 
-  // Handles the device file |error|. |error_callback| is invoked to notify the
-  // caller about the file error.
+  // Handles the device file |error| while operating on |file_id|.
+  // |error_callback| is invoked to notify the caller about the file error.
   void HandleDeviceFileError(const ErrorCallback& error_callback,
+                             uint32 file_id,
                              base::File::Error error);
 
+  // Given a full path, return a non-empty sub-path that needs to be read into
+  // the cache if such an uncached path exists. In which case, also write the
+  // last cached file id to |cached_file_id|.
+  base::FilePath NextUncachedPathComponent(const base::FilePath& path,
+                                           uint32* cached_file_id) const;
+
+  // Fills the file cache using the results from NextUncachedPathComponent().
+  void FillFileCache(const base::FilePath& uncached_path,
+                     uint32 cached_file_id,
+                     const base::Closure success_callback,
+                     const ErrorCallback& error_callback);
+
+  // Given a full path, if it exists in the cache, write the file's id to |id|
+  // and return true.
+  bool CachedPathToId(const base::FilePath& path, uint32* id) const;

[tommycli, 2014/07/09 21:48:31]

Since every caller really just wants a "EnsureCacheFilledThenGetIdForPath"
method, is it possible to just have that one member function?

Maybe these can be in the anonymous namespace?

+
   // MTP device initialization state.
   InitializationState init_state_;
 
@@ -197,6 +257,15 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
   // request at any time.
   scoped_ptr<SnapshotRequestInfo> current_snapshot_request_info_;
 
+  // A mapping for quick lookups into the |root_node_| tree structure. Since
+  // |root_node_| contains pointers to this map, it must be declared after this
+  // so destruction happens in the right order.
+  FileIdToMTPFileNodeMap file_id_to_node_map_;
+
+  // The root node of a tree-structure that caches the directory structure of
+  // the MTP device.
+  MTPFileNode root_node_;
+
   // For callbacks that may run after destruction.
   base::WeakPtrFactory<MTPDeviceDelegateImplLinux> weak_ptr_factory_;