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

chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc

Index: chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
diff --git a/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc b/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
index 0cdbb83f1890058a40727abdc3f5a841e5c6e5ad..28089098574d9045ed398ffb2309fe26f856c002 100644
--- a/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
+++ b/chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
@@ -5,16 +5,20 @@
 #include "chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h"
 
 #include <algorithm>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/numerics/safe_conversions.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/media_galleries/linux/mtp_device_task_helper.h"
 #include "chrome/browser/media_galleries/linux/mtp_device_task_helper_map_service.h"
 #include "chrome/browser/media_galleries/linux/snapshot_file_details.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/io_buffer.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace {
 
@@ -80,7 +84,7 @@ void OpenStorageOnUIThread(
   task_helper->OpenStorage(storage_name, reply_callback);
 }
 
-// Enumerates the |root| directory file entries.
+// Enumerates the |dir_id| directory file entries.
 //
 // Called on the UI thread to dispatch the request to the
 // MediaTransferProtocolManager.
@@ -91,7 +95,7 @@ void OpenStorageOnUIThread(
 // |success_callback| and |error_callback| runs on the IO thread.
 void ReadDirectoryOnUIThread(
     const std::string& storage_name,
-    const std::string& root,
+    uint32 dir_id,
     const base::Callback<
         void(const fileapi::AsyncFileUtil::EntryList&)>& success_callback,
     const base::Callback<void(base::File::Error)>& error_callback) {
@@ -100,7 +104,7 @@ void ReadDirectoryOnUIThread(
       GetDeviceTaskHelperForStorage(storage_name);
   if (!task_helper)
     return;
-  task_helper->ReadDirectoryByPath(root, success_callback, error_callback);
+  task_helper->ReadDirectoryById(dir_id, success_callback, error_callback);
 }
 
 // Gets the |file_path| details.
@@ -114,7 +118,7 @@ void ReadDirectoryOnUIThread(
 // |success_callback| and |error_callback| runs on the IO thread.
 void GetFileInfoOnUIThread(
     const std::string& storage_name,
-    const std::string& file_path,
+    uint32 file_id,
     const base::Callback<void(const base::File::Info&)>& success_callback,
     const base::Callback<void(base::File::Error)>& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -122,7 +126,7 @@ void GetFileInfoOnUIThread(
       GetDeviceTaskHelperForStorage(storage_name);
   if (!task_helper)
     return;
-  task_helper->GetFileInfoByPath(file_path, success_callback, error_callback);
+  task_helper->GetFileInfoById(file_id, success_callback, error_callback);
 }
 
 // Copies the contents of |device_file_path| to |snapshot_file_path|.
@@ -185,6 +189,14 @@ void CloseStorageAndDestroyTaskHelperOnUIThread(
       storage_name);
 }
 
+void ReadDirectoryCallbackAdapter(
+    const base::Closure& closure,
+    const fileapi::AsyncFileUtil::EntryList& /* file_list */,
+    bool /* has_more */) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  closure.Run();
+}
+
 }  // namespace
 
 MTPDeviceDelegateImplLinux::PendingTaskInfo::PendingTaskInfo(
@@ -197,11 +209,72 @@ MTPDeviceDelegateImplLinux::PendingTaskInfo::PendingTaskInfo(
 MTPDeviceDelegateImplLinux::PendingTaskInfo::~PendingTaskInfo() {
 }
 
+MTPDeviceDelegateImplLinux::MTPFileNode::MTPFileNode(
+    uint32 file_id,
+    MTPFileNode* parent,
+    FileIdToMTPFileNodeMap* file_id_to_node_map)
+    : file_id_(file_id),
+      parent_(parent),
+      file_id_to_node_map_(file_id_to_node_map) {
+  DCHECK(file_id_to_node_map_);
+  DCHECK(!ContainsKey(*file_id_to_node_map_, file_id_));
+  (*file_id_to_node_map_)[file_id_] = this;
+}
+
+MTPDeviceDelegateImplLinux::MTPFileNode::~MTPFileNode() {
+  size_t erased = file_id_to_node_map_->erase(file_id_);
+  DCHECK_EQ(1U, erased);
+}
+
+const MTPDeviceDelegateImplLinux::MTPFileNode*
+MTPDeviceDelegateImplLinux::MTPFileNode::GetChild(
+    const std::string& name) const {
+  return children_.get(name);
+}
+
+void MTPDeviceDelegateImplLinux::MTPFileNode::AddChild(const std::string& name,
+                                                       uint32 id) {
+  const MTPFileNode* child = GetChild(name);
+  if (child && child->file_id() == id)
+    return;
+
+  children_.set(
+      name,
+      make_scoped_ptr(new MTPFileNode(id, this, file_id_to_node_map_)));
+}
+
+void MTPDeviceDelegateImplLinux::MTPFileNode::ClearChildren(
+    const std::set<std::string>& children_to_keep) {
+  std::set<std::string> children_to_erase;
+  for (ChildNodes::const_iterator it = children_.begin();
+       it != children_.end(); ++it) {
+    if (ContainsKey(children_to_keep, it->first))
+      continue;
+    children_to_erase.insert(it->first);
+  }
+  for (std::set<std::string>::iterator it = children_to_erase.begin();
+       it != children_to_erase.end(); ++it) {
+    children_.take_and_erase(*it);
+  }
+}
+
+bool MTPDeviceDelegateImplLinux::MTPFileNode::DeleteChild(uint32 file_id) {
+  for (ChildNodes::iterator it = children_.begin();
+       it != children_.end(); ++it) {
+    if (it->second->file_id() == file_id) {
+      children_.erase(it);
+      return true;
+    }
+  }
+  return false;
+}
+
 MTPDeviceDelegateImplLinux::MTPDeviceDelegateImplLinux(
     const std::string& device_location)
     : init_state_(UNINITIALIZED),
       task_in_progress_(false),
       device_path_(device_location),
+      root_node_(mtpd::kRootFileId, NULL, &file_id_to_node_map_),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!device_path_.empty());
@@ -219,16 +292,38 @@ void MTPDeviceDelegateImplLinux::GetFileInfo(
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!file_path.empty());
+
+  uint32 cached_file_id;

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

This block could be abstracted into the EnsureCacheFilledAndGetId .

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

|readdir_success_callback| is different for each one of these calls. So we're
only going to save a few lines at the cost of having to create the Closure every
time.

+  base::FilePath uncached_path =
+      NextUncachedPathComponent(file_path, &cached_file_id);
+  if (!uncached_path.empty()) {
+    base::Closure readdir_success_callback =
+        base::Bind(&MTPDeviceDelegateImplLinux::GetFileInfo,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   file_path,
+                   success_callback,
+                   error_callback);
+    FillFileCache(uncached_path, cached_file_id, readdir_success_callback,
+                  error_callback);
+    return;
+  }
+
+  uint32 file_id;
+  if (!CachedPathToId(file_path, &file_id)) {
+    error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return;
+  }
   base::Closure call_closure =
       base::Bind(&GetFileInfoOnUIThread,
                  storage_name_,
-                 GetDeviceRelativePath(device_path_, file_path),
+                 file_id,
                  base::Bind(&MTPDeviceDelegateImplLinux::OnDidGetFileInfo,
                             weak_ptr_factory_.GetWeakPtr(),
                             success_callback),
                  base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
                             weak_ptr_factory_.GetWeakPtr(),
-                            error_callback));
+                            error_callback,
+                            file_id));
   EnsureInitAndRunTask(PendingTaskInfo(FROM_HERE, call_closure));
 }
 
@@ -238,22 +333,43 @@ void MTPDeviceDelegateImplLinux::ReadDirectory(
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!root.empty());
-  std::string device_file_relative_path = GetDeviceRelativePath(device_path_,
-                                                                root);
+
+  uint32 cached_file_id;
+  base::FilePath uncached_path =
+      NextUncachedPathComponent(root, &cached_file_id);
+  if (!uncached_path.empty()) {
+    base::Closure readdir_success_callback =
+        base::Bind(&MTPDeviceDelegateImplLinux::ReadDirectory,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   root,
+                   success_callback,
+                   error_callback);
+    FillFileCache(uncached_path, cached_file_id, readdir_success_callback,
+                  error_callback);
+    return;
+  }
+
+  uint32 dir_id;
+  if (!CachedPathToId(root, &dir_id)) {
+    error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return;
+  }
+
   base::Closure call_closure =
       base::Bind(
           &GetFileInfoOnUIThread,
           storage_name_,
-          device_file_relative_path,
+          dir_id,
           base::Bind(
               &MTPDeviceDelegateImplLinux::OnDidGetFileInfoToReadDirectory,
               weak_ptr_factory_.GetWeakPtr(),
-              device_file_relative_path,
+              dir_id,
               success_callback,
               error_callback),
           base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
                      weak_ptr_factory_.GetWeakPtr(),
-                     error_callback));
+                     error_callback,
+                     dir_id));
   EnsureInitAndRunTask(PendingTaskInfo(FROM_HERE, call_closure));
 }
 
@@ -265,10 +381,30 @@ void MTPDeviceDelegateImplLinux::CreateSnapshotFile(
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!device_file_path.empty());
   DCHECK(!snapshot_file_path.empty());
-  std::string device_file_relative_path =
-      GetDeviceRelativePath(device_path_, device_file_path);
+
+  uint32 cached_file_id;
+  base::FilePath uncached_path =
+      NextUncachedPathComponent(device_file_path, &cached_file_id);
+  if (!uncached_path.empty()) {
+    base::Closure readdir_success_callback =
+        base::Bind(&MTPDeviceDelegateImplLinux::CreateSnapshotFile,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   device_file_path,
+                   snapshot_file_path,
+                   success_callback,
+                   error_callback);
+    FillFileCache(uncached_path, cached_file_id, readdir_success_callback,
+                  error_callback);
+    return;
+  }
+
+  uint32 file_id;
+  if (!CachedPathToId(device_file_path, &file_id)) {
+    error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return;
+  }
   scoped_ptr<SnapshotRequestInfo> request_info(
-      new SnapshotRequestInfo(device_file_relative_path,
+      new SnapshotRequestInfo(file_id,
                               snapshot_file_path,
                               success_callback,
                               error_callback));
@@ -276,14 +412,15 @@ void MTPDeviceDelegateImplLinux::CreateSnapshotFile(
       base::Bind(
           &GetFileInfoOnUIThread,
           storage_name_,
-          device_file_relative_path,
+          file_id,
           base::Bind(
               &MTPDeviceDelegateImplLinux::OnDidGetFileInfoToCreateSnapshotFile,
               weak_ptr_factory_.GetWeakPtr(),
               base::Passed(&request_info)),
           base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
                      weak_ptr_factory_.GetWeakPtr(),
-                     error_callback));
+                     error_callback,
+                     file_id));
   EnsureInitAndRunTask(PendingTaskInfo(FROM_HERE, call_closure));
 }
 
@@ -298,15 +435,40 @@ void MTPDeviceDelegateImplLinux::ReadBytes(
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!device_file_path.empty());
-  std::string device_file_relative_path =
-      GetDeviceRelativePath(device_path_, device_file_path);
+
+  uint32 cached_file_id;
+  base::FilePath uncached_path =
+      NextUncachedPathComponent(device_file_path, &cached_file_id);
+  if (!uncached_path.empty()) {
+    base::Closure readdir_success_callback =
+        base::Bind(&MTPDeviceDelegateImplLinux::ReadBytes,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   device_file_path,
+                   make_scoped_refptr(buf),
+                   offset,
+                   buf_len,
+                   success_callback,
+                   error_callback);
+    FillFileCache(uncached_path, cached_file_id, readdir_success_callback,
+                  error_callback);
+    return;
+  }
+
+  uint32 file_id;
+  if (!CachedPathToId(device_file_path, &file_id)) {
+    error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    return;
+  }
 
   ReadBytesRequest request(
-      device_file_relative_path, buf, offset, buf_len,
+      file_id, buf, offset, buf_len,
       base::Bind(&MTPDeviceDelegateImplLinux::OnDidReadBytes,
-                 weak_ptr_factory_.GetWeakPtr(), success_callback),
+                 weak_ptr_factory_.GetWeakPtr(),
+                 success_callback),
       base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
-                 weak_ptr_factory_.GetWeakPtr(), error_callback));
+                 weak_ptr_factory_.GetWeakPtr(),
+                 error_callback,
+                 file_id));
 
   base::Closure call_closure =
       base::Bind(base::Bind(&ReadBytesOnUIThread, storage_name_, request));
@@ -355,7 +517,7 @@ void MTPDeviceDelegateImplLinux::WriteDataIntoSnapshotFile(
   DCHECK_GT(file_info.size, 0);
   task_in_progress_ = true;
   SnapshotRequestInfo request_info(
-      current_snapshot_request_info_->device_file_path,
+      current_snapshot_request_info_->file_id,
       current_snapshot_request_info_->snapshot_file_path,
       base::Bind(
           &MTPDeviceDelegateImplLinux::OnDidWriteDataIntoSnapshotFile,
@@ -404,7 +566,7 @@ void MTPDeviceDelegateImplLinux::OnDidGetFileInfo(
 }
 
 void MTPDeviceDelegateImplLinux::OnDidGetFileInfoToReadDirectory(
-    const std::string& root,
+    uint32 dir_id,
     const ReadDirectorySuccessCallback& success_callback,
     const ErrorCallback& error_callback,
     const base::File::Info& file_info) {
@@ -412,19 +574,22 @@ void MTPDeviceDelegateImplLinux::OnDidGetFileInfoToReadDirectory(
   DCHECK(task_in_progress_);
   if (!file_info.is_directory) {
     return HandleDeviceFileError(error_callback,
+                                 dir_id,
                                  base::File::FILE_ERROR_NOT_A_DIRECTORY);
   }
 
   base::Closure task_closure =
       base::Bind(&ReadDirectoryOnUIThread,
                  storage_name_,
-                 root,
+                 dir_id,
                  base::Bind(&MTPDeviceDelegateImplLinux::OnDidReadDirectory,
                             weak_ptr_factory_.GetWeakPtr(),
+                            dir_id,
                             success_callback),
                  base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
                             weak_ptr_factory_.GetWeakPtr(),
-                            error_callback));
+                            error_callback,
+                            dir_id));
   content::BrowserThread::PostTask(content::BrowserThread::UI,
                                    FROM_HERE,
                                    task_closure);
@@ -444,7 +609,9 @@ void MTPDeviceDelegateImplLinux::OnDidGetFileInfoToCreateSnapshotFile(
     error = base::File::FILE_ERROR_FAILED;
 
   if (error != base::File::FILE_OK)
-    return HandleDeviceFileError(snapshot_request_info->error_callback, error);
+    return HandleDeviceFileError(snapshot_request_info->error_callback,
+                                 snapshot_request_info->file_id,
+                                 error);
 
   base::File::Info snapshot_file_info(file_info);
   // Modify the last modified time to null. This prevents the time stamp
@@ -461,10 +628,38 @@ void MTPDeviceDelegateImplLinux::OnDidGetFileInfoToCreateSnapshotFile(
 }
 
 void MTPDeviceDelegateImplLinux::OnDidReadDirectory(
+    uint32 dir_id,
     const ReadDirectorySuccessCallback& success_callback,
     const fileapi::AsyncFileUtil::EntryList& file_list) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  success_callback.Run(file_list, false /*no more entries*/);
+
+  FileIdToMTPFileNodeMap::iterator it = file_id_to_node_map_.find(dir_id);
+  DCHECK(it != file_id_to_node_map_.end());
+  MTPFileNode* dir_node = it->second;
+  std::set<std::string> children_to_keep;
+
+  fileapi::AsyncFileUtil::EntryList normalized_file_list;

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

My initial reaction was: Why not just clear the list of children, then re-add.
But now I understand you want to keep the subtrees.

Maybe we should change the names?
AddChild's real intent is "EnsureChildExists"
and 
ClearChildren's real intent is "ClearNonexistentChildren"

I'm not sure how to make this more clear, since it's a pretty complicated
operation.

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

Done.

+  for (size_t i = 0; i < file_list.size(); ++i) {
+    normalized_file_list.push_back(file_list[i]);
+    fileapi::DirectoryEntry& entry = normalized_file_list.back();
+
+    // |entry.name| has the file id encoded in it. Decode here.
+    size_t separator_idx = entry.name.find_last_of(',');
+    DCHECK_NE(std::string::npos, separator_idx);
+    std::string file_id_str = entry.name.substr(separator_idx);
+    file_id_str = file_id_str.substr(1);  // Get rid of the comma.
+    uint32 file_id = 0;
+    bool ret = base::StringToUint(file_id_str, &file_id);
+    DCHECK(ret);
+    entry.name = entry.name.substr(0, separator_idx);
+
+    // Refresh the in memory tree.
+    dir_node->AddChild(entry.name, file_id);
+    children_to_keep.insert(entry.name);
+  }
+  dir_node->ClearChildren(children_to_keep);
+
+  success_callback.Run(normalized_file_list, false /*no more entries*/);
   task_in_progress_ = false;
   ProcessNextPendingRequest();
 }
@@ -505,14 +700,94 @@ void MTPDeviceDelegateImplLinux::OnDidReadBytes(
 
 void MTPDeviceDelegateImplLinux::HandleDeviceFileError(
     const ErrorCallback& error_callback,
+    uint32 file_id,
     base::File::Error error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  DCHECK(task_in_progress_);
+
+  FileIdToMTPFileNodeMap::iterator it = file_id_to_node_map_.find(file_id);
+  if (it != file_id_to_node_map_.end()) {
+    MTPFileNode* parent = it->second->parent();
+    if (parent) {
+      bool ret = parent->DeleteChild(file_id);
+      DCHECK(ret);
+    }
+  }
   error_callback.Run(error);
   task_in_progress_ = false;
   ProcessNextPendingRequest();
 }
 
+base::FilePath MTPDeviceDelegateImplLinux::NextUncachedPathComponent(

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

I really like how this method is structured.

+    const base::FilePath& path,
+    uint32* cached_file_id) const {
+  DCHECK(cached_file_id);
+
+  base::FilePath uncached_path;
+  std::string device_relpath = GetDeviceRelativePath(device_path_, path);
+  if (!device_relpath.empty() && device_relpath != kRootPath) {
+    uncached_path = device_path_;
+    std::vector<std::string> device_relpath_components;
+    base::SplitString(device_relpath, '/', &device_relpath_components);
+    DCHECK(!device_relpath_components.empty());
+    bool all_components_cached = true;
+    uint32 file_id;
+    const MTPFileNode* current_node = &root_node_;
+    for (size_t i = 0; i < device_relpath_components.size(); ++i) {
+      file_id = current_node->file_id();
+
+      current_node = current_node->GetChild(device_relpath_components[i]);
+      if (!current_node) {
+        all_components_cached = false;
+        break;
+      }
+      uncached_path = uncached_path.Append(device_relpath_components[i]);
+    }
+    if (all_components_cached)
+      uncached_path.clear();
+    else
+      *cached_file_id = file_id;
+  }
+  return uncached_path;
+}
+
+void MTPDeviceDelegateImplLinux::FillFileCache(
+    const base::FilePath& uncached_path,
+    uint32 cached_file_id,
+    const base::Closure success_callback,
+    const ErrorCallback& error_callback) {
+  DCHECK(!uncached_path.empty());
+
+  ReadDirectorySuccessCallback readdir_success_callback =
+      base::Bind(&ReadDirectoryCallbackAdapter, success_callback);
+  ReadDirectory(uncached_path,
+                readdir_success_callback,
+                base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
+                           weak_ptr_factory_.GetWeakPtr(),
+                           error_callback,
+                           cached_file_id));
+}
+
+
+bool MTPDeviceDelegateImplLinux::CachedPathToId(const base::FilePath& path,
+                                                uint32* id) const {
+  DCHECK(id);
+
+  std::string device_relpath = GetDeviceRelativePath(device_path_, path);
+  if (device_relpath.empty())
+    return false;
+  std::vector<std::string> device_relpath_components;
+  if (device_relpath != kRootPath)
+    base::SplitString(device_relpath, '/', &device_relpath_components);
+  const MTPFileNode* current_node = &root_node_;
+  for (size_t i = 0; i < device_relpath_components.size(); ++i) {
+    current_node = current_node->GetChild(device_relpath_components[i]);
+    if (!current_node)
+      return false;
+  }
+  *id = current_node->file_id();
+  return true;
+}
+
 void CreateMTPDeviceAsyncDelegate(
     const std::string& device_location,
     const CreateMTPDeviceAsyncDelegateCallback& callback) {