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

File paths can be very slow to access because finding a file by path requires
O(N) metadata access to figure out which file id corresponds to a given name.

BUG=385307

Review URL: https://codereview.chromium.org/377383002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283732 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/media_galleries/fileapi/mtp_device_async_delegate.cc

@@ -7,11 +7,10 @@
 #include "net/base/io_buffer.h"
 
 MTPDeviceAsyncDelegate::ReadBytesRequest::ReadBytesRequest(
-    const std::string& device_file_relative_path,
-    net::IOBuffer* buf, int64 offset, int buf_len,
+    uint32 file_id, net::IOBuffer* buf, int64 offset, int buf_len,
     const ReadBytesSuccessCallback& success_callback,
     const ErrorCallback& error_callback)
-    : device_file_relative_path(device_file_relative_path),
+    : file_id(file_id),
       buf(buf),
       offset(offset),
       buf_len(buf_len),

chrome/browser/media_galleries/fileapi/mtp_device_async_delegate.h

@@ -49,13 +49,13 @@ class MTPDeviceAsyncDelegate {
            int bytes_read)> ReadBytesSuccessCallback;
 
   struct ReadBytesRequest {
-    ReadBytesRequest(const std::string& device_file_relative_path,
+    ReadBytesRequest(uint32 file_id,
                      net::IOBuffer* buf, int64 offset, int buf_len,
                      const ReadBytesSuccessCallback& success_callback,
                      const ErrorCallback& error_callback);
     ~ReadBytesRequest();
 
-    std::string device_file_relative_path;
+    uint32 file_id;
     scoped_refptr<net::IOBuffer> buf;
     int64 offset;
     int buf_len;

chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h

@@ -5,19 +5,20 @@
 #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 <queue>
+#include <deque>
+#include <map>
+#include <string>
 
 #include "base/callback.h"
+#include "base/containers/scoped_ptr_hash_map.h"
+#include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/media_galleries/fileapi/mtp_device_async_delegate.h"
+#include "content/public/browser/browser_thread.h"
 #include "webkit/browser/fileapi/async_file_util.h"
 
-namespace base {
-class FilePath;
-}
-
 struct SnapshotRequestInfo;
 
 // MTPDeviceDelegateImplLinux communicates with the media transfer protocol
@@ -40,14 +41,24 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
 
   // Used to represent pending task details.
   struct PendingTaskInfo {
-    PendingTaskInfo(const tracked_objects::Location& location,
+    PendingTaskInfo(const base::FilePath& path,
+                    content::BrowserThread::ID thread_id,
+                    const tracked_objects::Location& location,
                     const base::Closure& task);
     ~PendingTaskInfo();
 
+    base::FilePath path;
+    base::FilePath cached_path;
+    const content::BrowserThread::ID thread_id;
     const tracked_objects::Location location;
     const base::Closure task;
   };
 
+  class MTPFileNode;
+
+  // Maps file ids to file nodes.
+  typedef std::map<uint32, MTPFileNode*> FileIdToMTPFileNodeMap;
+
   // 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.
@@ -77,17 +88,41 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
       const ErrorCallback& error_callback) OVERRIDE;
   virtual void CancelPendingTasksAndDeleteDelegate() OVERRIDE;
 
-  // Ensures the device is initialized for communication by doing a
-  // call-and-reply to the UI thread. |task_info.task| runs on the UI thread.
-  //
-  // If the device is already initialized, post the |task_info.task| immediately
-  // on the UI thread.
+  // The internal methods correspond to the similarly named methods above.
+  // The |root_node_| cache should be filled at this point.
+  virtual void GetFileInfoInternal(
+      const base::FilePath& file_path,
+      const GetFileInfoSuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+  virtual void ReadDirectoryInternal(
+      const base::FilePath& root,
+      const ReadDirectorySuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+  virtual void CreateSnapshotFileInternal(
+      const base::FilePath& device_file_path,
+      const base::FilePath& local_path,
+      const CreateSnapshotFileSuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+  virtual void ReadBytesInternal(
+      const base::FilePath& device_file_path,
+      net::IOBuffer* buf, int64 offset, int buf_len,
+      const ReadBytesSuccessCallback& success_callback,
+      const ErrorCallback& error_callback);
+
+  // Ensures the device is initialized for communication.
+  // If the device is already initialized, call RunTask().
   //
   // If the device is uninitialized, store the |task_info| in a pending task
-  // list and runs all the pending tasks once the device is successfully
-  // initialized.
+  // queue and runs the pending tasks in the queue once the device is
+  // successfully initialized.
   void EnsureInitAndRunTask(const PendingTaskInfo& task_info);
 
+  // Runs a task. If |task_info.path| is empty, or if the path is cached, runs
+  // the task immediately.
+  // Otherwise, fills the cache first before running the task.
+  // |task_info.task| runs on the UI thread.
+  void RunTask(const PendingTaskInfo& task_info);
+
   // Writes data from the device to the snapshot file path based on the
   // parameters in |current_snapshot_request_info_| by doing a call-and-reply to
   // the UI thread.
@@ -114,16 +149,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);
@@ -140,10 +175,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.
@@ -171,11 +208,34 @@ 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.
+  // Called when FillFileCache() succeeds.
+  void OnDidFillFileCache(const base::FilePath& path,
+                          const fileapi::AsyncFileUtil::EntryList& file_list,
+                          bool has_more);
+
+  // Called when FillFileCache() fails.
+  void OnFillFileCacheFailed(base::File::Error 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, returns a non-empty sub-path that needs to be read into
+  // the cache if such a uncached path exists.
+  // |cached_path| is the portion of |path| that has had cache lookup attempts.
+  base::FilePath NextUncachedPathComponent(
+      const base::FilePath& path,
+      const base::FilePath& cached_path) const;
+
+  // Fills the file cache using the results from NextUncachedPathComponent().
+  void FillFileCache(const base::FilePath& uncached_path);
+
+  // Given a full path, if it exists in the cache, writes the file's id to |id|
+  // and return true.
+  bool CachedPathToId(const base::FilePath& path, uint32* id) const;
+
   // MTP device initialization state.
   InitializationState init_state_;
 
@@ -193,7 +253,7 @@ class MTPDeviceDelegateImplLinux : public MTPDeviceAsyncDelegate {
 
   // A list of pending tasks that needs to be run when the device is
   // initialized or when the current task in progress is complete.
-  std::queue<PendingTaskInfo> pending_tasks_;
+  std::deque<PendingTaskInfo> pending_tasks_;
 
   // Used to track the current snapshot file request. A snapshot file is created
   // incrementally. CreateSnapshotFile request reads the device file and writes
@@ -202,6 +262,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.
+  scoped_ptr<MTPFileNode> root_node_;
+
   // For callbacks that may run after destruction.
   base::WeakPtrFactory<MTPDeviceDelegateImplLinux> weak_ptr_factory_;
 

chrome/browser/media_galleries/linux/mtp_device_task_helper.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
+#include "base/strings/string_number_conversions.h"
 #include "chrome/browser/media_galleries/linux/mtp_device_object_enumerator.h"
 #include "chrome/browser/media_galleries/linux/mtp_read_file_worker.h"
 #include "chrome/browser/media_galleries/linux/snapshot_file_details.h"
@@ -74,33 +75,33 @@ void MTPDeviceTaskHelper::OpenStorage(const std::string& storage_name,
                  callback));
 }
 
-void MTPDeviceTaskHelper::GetFileInfoByPath(
-    const std::string& file_path,
+void MTPDeviceTaskHelper::GetFileInfoById(
+    uint32 file_id,
     const GetFileInfoSuccessCallback& success_callback,
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (device_handle_.empty())
     return HandleDeviceError(error_callback, base::File::FILE_ERROR_FAILED);
 
-  GetMediaTransferProtocolManager()->GetFileInfoByPath(
-      device_handle_, file_path,
+  GetMediaTransferProtocolManager()->GetFileInfoById(
+      device_handle_, file_id,
       base::Bind(&MTPDeviceTaskHelper::OnGetFileInfo,
                  weak_ptr_factory_.GetWeakPtr(),
                  success_callback,
                  error_callback));
 }
 
-void MTPDeviceTaskHelper::ReadDirectoryByPath(
-    const std::string& dir_path,
+void MTPDeviceTaskHelper::ReadDirectoryById(
+    uint32 dir_id,
     const ReadDirectorySuccessCallback& success_callback,
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (device_handle_.empty())
     return HandleDeviceError(error_callback, base::File::FILE_ERROR_FAILED);
 
-  GetMediaTransferProtocolManager()->ReadDirectoryByPath(
-      device_handle_, dir_path,
-      base::Bind(&MTPDeviceTaskHelper::OnDidReadDirectoryByPath,
+  GetMediaTransferProtocolManager()->ReadDirectoryById(
+      device_handle_, dir_id,
+      base::Bind(&MTPDeviceTaskHelper::OnDidReadDirectoryById,
                  weak_ptr_factory_.GetWeakPtr(),
                  success_callback,
                  error_callback));
@@ -129,8 +130,8 @@ void MTPDeviceTaskHelper::ReadBytes(
                              base::File::FILE_ERROR_FAILED);
   }
 
-  GetMediaTransferProtocolManager()->GetFileInfoByPath(
-      device_handle_, request.device_file_relative_path,
+  GetMediaTransferProtocolManager()->GetFileInfoById(
+      device_handle_, request.file_id,
       base::Bind(&MTPDeviceTaskHelper::OnGetFileInfoToReadBytes,
                  weak_ptr_factory_.GetWeakPtr(), request));
 }
@@ -171,7 +172,7 @@ void MTPDeviceTaskHelper::OnGetFileInfo(
       base::Bind(success_callback, FileInfoFromMTPFileEntry(file_entry)));
 }
 
-void MTPDeviceTaskHelper::OnDidReadDirectoryByPath(
+void MTPDeviceTaskHelper::OnDidReadDirectoryById(
     const ReadDirectorySuccessCallback& success_callback,
     const ErrorCallback& error_callback,
     const std::vector<MtpFileEntry>& file_entries,
@@ -186,6 +187,11 @@ void MTPDeviceTaskHelper::OnDidReadDirectoryByPath(
   while (!(current = file_enum.Next()).empty()) {
     fileapi::DirectoryEntry entry;
     entry.name = fileapi::VirtualPath::BaseName(current).value();
+    uint32 file_id = 0;
+    bool ret = file_enum.GetEntryId(&file_id);
+    DCHECK(ret);
+    entry.name.push_back(',');
+    entry.name += base::UintToString(file_id);
     entry.is_directory = file_enum.IsDirectory();
     entry.size = file_enum.Size();
     entry.last_modified_time = file_enum.LastModifiedTime();
@@ -202,7 +208,7 @@ void MTPDeviceTaskHelper::OnGetFileInfoToReadBytes(
     bool error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(request.buf);
-  DCHECK(request.buf_len >= 0);
+  DCHECK_GE(request.buf_len, 0);
   DCHECK_GE(request.offset, 0);
   if (error) {
     return HandleDeviceError(request.error_callback,
@@ -229,9 +235,9 @@ void MTPDeviceTaskHelper::OnGetFileInfoToReadBytes(
       base::checked_cast<uint32>(request.buf_len),
       base::saturated_cast<uint32>(file_info.size - request.offset));
 
-  GetMediaTransferProtocolManager()->ReadFileChunkByPath(
+  GetMediaTransferProtocolManager()->ReadFileChunkById(
       device_handle_,
-      request.device_file_relative_path,
+      request.file_id,
       base::checked_cast<uint32>(request.offset),
       bytes_to_read,
       base::Bind(&MTPDeviceTaskHelper::OnDidReadBytes,

chrome/browser/media_galleries/linux/mtp_device_task_helper.h

@@ -31,6 +31,8 @@ class MTPDeviceTaskHelper {
   typedef MTPDeviceAsyncDelegate::GetFileInfoSuccessCallback
       GetFileInfoSuccessCallback;
 
+  // NOTE: The file names in the entry list have their file id appended at the
+  // end. e.g. foo.jpg with file id 45 becomes foo.jpg,45.
   typedef base::Callback<void(const fileapi::AsyncFileUtil::EntryList&)>
       ReadDirectorySuccessCallback;
 
@@ -48,34 +50,36 @@ class MTPDeviceTaskHelper {
   void OpenStorage(const std::string& storage_name,
                    const OpenStorageCallback& callback);
 
-  // Dispatches the GetFileInfoByPath request to the
+  // Dispatches the GetFileInfoById request to the
   // MediaTransferProtocolManager.
   //
-  // |file_path| specifies the relative of the file whose details are requested.
+  // |file_id| specifies the id of the file whose details are requested.
   //
   // If the file details are fetched successfully, |success_callback| is invoked
   // on the IO thread to notify the caller about the file details.
   //
   // If there is an error, |error_callback| is invoked on the IO thread to
   // notify the caller about the file error.
-  void GetFileInfoByPath(
-      const std::string& file_path,
+  void GetFileInfoById(
+      uint32 file_id,
       const GetFileInfoSuccessCallback& success_callback,
       const ErrorCallback& error_callback);
 
   // Dispatches the read directory request to the MediaTransferProtocolManager.
   //
-  // |dir_path| specifies the directory file path.
+  // |dir_id| specifies the directory id.
   //
   // If the directory file entries are enumerated successfully,
   // |success_callback| is invoked on the IO thread to notify the caller about
-  // the directory file entries.
+  // the directory file entries. Please see the note in the
+  // ReadDirectorySuccessCallback typedef regarding the special treatment of
+  // file names.
   //
   // If there is an error, |error_callback| is invoked on the IO thread to
   // notify the caller about the file error.
-  void ReadDirectoryByPath(const std::string& dir_path,
-                           const ReadDirectorySuccessCallback& success_callback,
-                           const ErrorCallback& error_callback);
+  void ReadDirectoryById(uint32 dir_id,
+                         const ReadDirectorySuccessCallback& success_callback,
+                         const ErrorCallback& error_callback);
 
   // Forwards the WriteDataIntoSnapshotFile request to the MTPReadFileWorker
   // object.
@@ -122,7 +126,7 @@ class MTPDeviceTaskHelper {
                      const MtpFileEntry& file_entry,
                      bool error) const;
 
-  // Query callback for ReadDirectoryByPath().
+  // Query callback for ReadDirectoryById().
   //
   // If there is no error, |error| is set to false, |file_entries| has the
   // directory file entries and |success_callback| is invoked on the IO thread
@@ -130,7 +134,7 @@ class MTPDeviceTaskHelper {
   //
   // If there is an error, |error| is set to true, |file_entries| is empty
   // and |error_callback| is invoked on the IO thread to notify the caller.
-  void OnDidReadDirectoryByPath(
+  void OnDidReadDirectoryById(
       const ReadDirectorySuccessCallback& success_callback,
       const ErrorCallback& error_callback,
       const std::vector<MtpFileEntry>& file_entries,

chrome/browser/media_galleries/linux/mtp_read_file_worker.cc

@@ -66,9 +66,9 @@ void MTPReadFileWorker::ReadDataChunkFromDeviceFile(
 
   device::MediaTransferProtocolManager* mtp_device_manager =
       StorageMonitor::GetInstance()->media_transfer_protocol_manager();
-  mtp_device_manager->ReadFileChunkByPath(
+  mtp_device_manager->ReadFileChunkById(
       device_handle_,
-      snapshot_file_details_ptr->device_file_path(),
+      snapshot_file_details_ptr->file_id(),
       snapshot_file_details_ptr->bytes_written(),
       snapshot_file_details_ptr->BytesToRead(),
       base::Bind(&MTPReadFileWorker::OnDidReadDataChunkFromDeviceFile,

chrome/browser/media_galleries/linux/snapshot_file_details.cc

@@ -11,12 +11,12 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 SnapshotRequestInfo::SnapshotRequestInfo(
-    const std::string& device_file_path,
+    uint32 file_id,
     const base::FilePath& snapshot_file_path,
     const MTPDeviceAsyncDelegate::CreateSnapshotFileSuccessCallback&
         success_callback,
     const MTPDeviceAsyncDelegate::ErrorCallback& error_callback)
-    : device_file_path(device_file_path),
+    : file_id(file_id),
       snapshot_file_path(snapshot_file_path),
       success_callback(success_callback),
       error_callback(error_callback) {

chrome/browser/media_galleries/linux/snapshot_file_details.h

@@ -16,15 +16,15 @@
 // Used to represent snapshot file request params.
 struct SnapshotRequestInfo {
   SnapshotRequestInfo(
-      const std::string& device_file_path,
+      uint32 file_id,
       const base::FilePath& snapshot_file_path,
       const MTPDeviceAsyncDelegate::CreateSnapshotFileSuccessCallback&
           success_callback,
       const MTPDeviceAsyncDelegate::ErrorCallback& error_callback);
   ~SnapshotRequestInfo();
 
-  // MTP device file path.
-  const std::string device_file_path;
+  // MTP device file id.
+  const uint32 file_id;
 
   // Local platform path of the snapshot file.
   const base::FilePath snapshot_file_path;
@@ -47,8 +47,8 @@ class SnapshotFileDetails {
 
   ~SnapshotFileDetails();
 
-  std::string device_file_path() const {
-    return request_info_.device_file_path;
+  uint32 file_id() const {
+    return request_info_.file_id;
   }
 
   base::FilePath snapshot_file_path() const {