Implement DeleteFile and DeleteDirectory.

chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc

 // 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.
 
 #include "chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.h"
 
 #include <fcntl.h>
 #include <algorithm>
 #include <vector>
 
@@ skipping 95 matching lines @@
     const MTPDeviceTaskHelper::ReadDirectorySuccessCallback& success_callback,
     const MTPDeviceTaskHelper::ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   MTPDeviceTaskHelper* task_helper =
       GetDeviceTaskHelperForStorage(storage_name, read_only);
   if (!task_helper)
     return;
   task_helper->ReadDirectory(dir_id, success_callback, error_callback);
 }
 
+// Gets entry ids on |directory_id|.
+//
+// Called on the UI thread to dispatch the request to the
+// MediaTransferProtocolManager.
+//
+// |storage_name| specifies the name of the storage device.
+// |read_only| specifies the mode of the storage device.
+// |directory_id| is the directory to be read.
+// |success_callback| is called when the ReadDirectoryEntryIds request succeeds.
+// |error_callback| is called when the ReadDirectoryEntryIds request fails.
+// |success_callback| and |error_callback| runs on the IO thread.
+void ReadDirectoryEntryIdsOnUIThread(

[Lei Zhang, 2015/03/07 01:17:24]

What if we:
- got rid of this
- make ReadDirectoryOnUIThread() take an optional chunk size. (0 = default)
-- plumb that parameter down to MediaTransferProtocolManagerImpl
-- get rid the newly added ReadDirectoryEntryIds code
- Call ReadDirectoryOnUIThread(chunk_size = 1) instead of
ReadDirectoryEntryIdsOnUIThread()

Would that save us some code?

[yawano, 2015/03/09 03:46:48]

Can I confirm that chunk_size is maximum_length? I think there is no way to
cancel read directory call now.

[Lei Zhang, 2015/03/10 00:48:06]

Actually, my idea only works if we modify
MediaTransferProtocolManagerImpl::OnGotDirectoryEntries() to read at most 1
entry, not a chunk of 1 entry - as it, it would keep reading more directory
entries. Since we can't cancel, we need some way of telling the read directory
call to end early. WDYT?

[yawano, 2015/03/10 01:17:04]

Yes, I agree with implementing max_size. Adding a way to cancel read directory
call is a big task. I'm going to add max_size to ReadDirectory call, and remove
ReadDirectoryEntryIds.

+    const std::string& storage_name,
+    const bool read_only,
+    const uint32 directory_id,
+    const MTPDeviceTaskHelper::ReadDirectoryEntryIdsSuccessCallback&
+        success_callback,
+    const MTPDeviceTaskHelper::ErrorCallback& error_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  MTPDeviceTaskHelper* task_helper =
+      GetDeviceTaskHelperForStorage(storage_name, read_only);
+  if (!task_helper)
+    return;
+  task_helper->ReadDirectoryEntryIds(directory_id, success_callback,
+                                     error_callback);
+}
+
 // Gets the |file_path| details.
 //
 // Called on the UI thread to dispatch the request to the
 // MediaTransferProtocolManager.
 //
 // |storage_name| specifies the name of the storage device.
 // |read_only| specifies the mode of the storage device.
 // |success_callback| is called when the GetFileInfo request succeeds.
 // |error_callback| is called when the GetFileInfo request fails.
 // |success_callback| and |error_callback| runs on the IO thread.
@@ skipping 81 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   MTPDeviceTaskHelper* task_helper =
       GetDeviceTaskHelperForStorage(storage_name, read_only);
   if (!task_helper)
     return;
   task_helper->CopyFileFromLocal(storage_name, source_file_descriptor,
                                  parent_id, file_name, success_callback,
                                  error_callback);
 }
 
+// Deletes |object_id|.
+//
+// Called on the UI thread to dispatch the request to the
+// MediaTransferProtocolManager.
+//
+// |storage_name| specifies the name of the storage device.
+// |read_only| specifies the mode of the storage device.
+// |object_id| is the object to be deleted.
+// |success_callback| is called when the object is deleted successfully.
+// |error_callback| is called when it fails to delete the object.
+// |success_callback| and |error_callback| runs on the IO thread.
+void DeleteObjectOnUIThread(
+    const std::string storage_name,
+    const bool read_only,
+    const uint32 object_id,
+    const MTPDeviceTaskHelper::DeleteObjectSuccessCallback success_callback,
+    const MTPDeviceTaskHelper::ErrorCallback error_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  MTPDeviceTaskHelper* task_helper =
+      GetDeviceTaskHelperForStorage(storage_name, read_only);
+  if (!task_helper)
+    return;
+  task_helper->DeleteObject(object_id, success_callback, error_callback);
+}
+
 // Closes the device storage specified by the |storage_name| and destroys the
 // MTPDeviceTaskHelper object associated with the device storage.
 //
 // Called on the UI thread to dispatch the request to the
 // MediaTransferProtocolManager.
 void CloseStorageAndDestroyTaskHelperOnUIThread(
     const std::string& storage_name,
     const bool read_only) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   MTPDeviceTaskHelper* task_helper =
@@ skipping 260 matching lines @@
                  offset,
                  buf_len,
                  success_callback,
                  error_callback);
   EnsureInitAndRunTask(PendingTaskInfo(device_file_path,
                                        content::BrowserThread::IO,
                                        FROM_HERE,
                                        closure));
 }
 
-bool MTPDeviceDelegateImplLinux::IsReadOnly() {
+bool MTPDeviceDelegateImplLinux::IsReadOnly() const {
   return read_only_;
 }
 
 void MTPDeviceDelegateImplLinux::CopyFileFromLocal(
     const base::FilePath& source_file_path,
     const base::FilePath& device_file_path,
     const CopyFileFromLocalSuccessCallback& success_callback,
     const ErrorCallback& error_callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!source_file_path.empty());
   DCHECK(!device_file_path.empty());
 
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::FILE,
       FROM_HERE,
       base::Bind(&OpenFileDescriptor,
                  source_file_path.value().c_str(),
                  O_RDONLY),
       base::Bind(&MTPDeviceDelegateImplLinux::CopyFileFromLocalInternal,
                  weak_ptr_factory_.GetWeakPtr(),
                  device_file_path,
                  success_callback,
                  error_callback));
 }
 
+void MTPDeviceDelegateImplLinux::DeleteFile(
+    const base::FilePath& file_path,
+    const DeleteFileSuccessCallback& success_callback,
+    const ErrorCallback& error_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!file_path.empty());
+
+  const GetFileInfoSuccessCallback& success_callback_wrapper =
+      base::Bind(&MTPDeviceDelegateImplLinux::DeleteFileInternal,
+                 weak_ptr_factory_.GetWeakPtr(), file_path, success_callback,
+                 error_callback);
+
+  const base::Closure closure =
+      base::Bind(&MTPDeviceDelegateImplLinux::GetFileInfoInternal,
+                 weak_ptr_factory_.GetWeakPtr(), file_path,
+                 success_callback_wrapper, error_callback);
+  EnsureInitAndRunTask(PendingTaskInfo(file_path, content::BrowserThread::IO,
+                                       FROM_HERE, closure));
+}
+
+void MTPDeviceDelegateImplLinux::DeleteDirectory(
+    const base::FilePath& file_path,
+    const DeleteDirectorySuccessCallback& success_callback,
+    const ErrorCallback& error_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!file_path.empty());
+
+  const GetFileInfoSuccessCallback& success_callback_wrapper =
+      base::Bind(&MTPDeviceDelegateImplLinux::DeleteDirectoryInternal,
+                 weak_ptr_factory_.GetWeakPtr(), file_path, success_callback,
+                 error_callback);
+
+  const base::Closure closure =
+      base::Bind(&MTPDeviceDelegateImplLinux::GetFileInfoInternal,
+                 weak_ptr_factory_.GetWeakPtr(), file_path,
+                 success_callback_wrapper, error_callback);
+  EnsureInitAndRunTask(PendingTaskInfo(file_path, content::BrowserThread::IO,
+                                       FROM_HERE, closure));
+}
+
 void MTPDeviceDelegateImplLinux::CancelPendingTasksAndDeleteDelegate() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // To cancel all the pending tasks, destroy the MTPDeviceTaskHelper object.
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
       base::Bind(&CloseStorageAndDestroyTaskHelperOnUIThread,
                  storage_name_,
                  read_only_));
   delete this;
@@ skipping 176 matching lines @@
     EnsureInitAndRunTask(PendingTaskInfo(
         base::FilePath(), content::BrowserThread::UI, FROM_HERE, closure));
   } else {
     HandleCopyFileFromLocalError(error_callback, source_file_descriptor,
                                  base::File::FILE_ERROR_INVALID_OPERATION);
   }
 
   PendingRequestDone();
 }
 
+void MTPDeviceDelegateImplLinux::DeleteFileInternal(
+    const base::FilePath& file_path,
+    const DeleteFileSuccessCallback& success_callback,
+    const ErrorCallback& error_callback,
+    const base::File::Info& file_info) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (file_info.is_directory)
+    error_callback.Run(base::File::FILE_ERROR_NOT_A_FILE);
+  else
+    RunDeleteObjectOnUIThread(file_path, success_callback, error_callback);
+
+  PendingRequestDone();
+}
+
+void MTPDeviceDelegateImplLinux::DeleteDirectoryInternal(
+    const base::FilePath& file_path,
+    const DeleteDirectorySuccessCallback& success_callback,
+    const ErrorCallback& error_callback,
+    const base::File::Info& file_info) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (!file_info.is_directory) {
+    error_callback.Run(base::File::FILE_ERROR_NOT_A_DIRECTORY);
+  } else {
+    // Read directory entry ids to check the directory is empty.
+    uint32 directory_id;
+    if (CachedPathToId(file_path, &directory_id)) {
+      const MTPDeviceTaskHelper::ReadDirectoryEntryIdsSuccessCallback

[Lei Zhang, 2015/03/07 01:17:24]

Before we issue the ReadDirectory call, can we check the cache first? We'll need
to add a HasChildren() method to MTPDeviceDelegateImplLinux::MTPFileNode. If
HasChildren() return true, we know it's not empty, else we stil need to call
ReadDirectory, since the cache could just be empty.

[yawano, 2015/03/09 04:37:49]

Done.

+          success_callback_wrapper =
+              base::Bind(&MTPDeviceDelegateImplLinux::
+                             OnDidReadDirectoryEntryIdsToDeleteDirectory,
+                         weak_ptr_factory_.GetWeakPtr(), directory_id,
+                         success_callback, error_callback);
+      const MTPDeviceTaskHelper::ErrorCallback error_callback_wrapper =
+          base::Bind(&MTPDeviceDelegateImplLinux::HandleDeviceFileError,
+                     weak_ptr_factory_.GetWeakPtr(), error_callback,
+                     directory_id);
+      const base::Closure closure = base::Bind(
+          &ReadDirectoryEntryIdsOnUIThread, storage_name_, read_only_,
+          directory_id, success_callback_wrapper, error_callback_wrapper);
+      EnsureInitAndRunTask(PendingTaskInfo(
+          base::FilePath(), content::BrowserThread::UI, FROM_HERE, closure));
+    } else {
+      error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+    }
+  }
+
+  PendingRequestDone();
+}
+
+void MTPDeviceDelegateImplLinux::OnDidReadDirectoryEntryIdsToDeleteDirectory(
+    const uint32 directory_id,
+    const DeleteDirectorySuccessCallback& success_callback,
+    const ErrorCallback& error_callback,
+    const std::vector<uint32>& file_ids) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (file_ids.size() > 0)
+    error_callback.Run(base::File::FILE_ERROR_NOT_EMPTY);
+  else
+    RunDeleteObjectOnUIThread(directory_id, success_callback, error_callback);
+
+  PendingRequestDone();
+}
+
+void MTPDeviceDelegateImplLinux::RunDeleteObjectOnUIThread(

[Lei Zhang, 2015/03/07 01:17:24]

There's only one caller for this. I would just put get rid of this function and
put the code in DeleteFileInternal().

[yawano, 2015/03/09 04:37:49]

Done.

+    const base::FilePath& file_path,
+    const DeleteObjectSuccessCallback& success_callback,
+    const ErrorCallback& error_callback) {
+  uint32 object_id;
+  if (CachedPathToId(file_path, &object_id))
+    RunDeleteObjectOnUIThread(object_id, success_callback, error_callback);
+  else
+    error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+}
+
+void MTPDeviceDelegateImplLinux::RunDeleteObjectOnUIThread(
+    const uint32 object_id,
+    const DeleteObjectSuccessCallback& success_callback,
+    const ErrorCallback& error_callback) {
+  const MTPDeviceTaskHelper::DeleteObjectSuccessCallback
+      success_callback_wrapper = base::Bind(
+          &MTPDeviceDelegateImplLinux::OnDidDeleteObject,
+          weak_ptr_factory_.GetWeakPtr(), object_id, success_callback);
+
+  const MTPDeviceTaskHelper::ErrorCallback error_callback_wrapper =
+      base::Bind(&MTPDeviceDelegateImplLinux::HandleDeleteFileOrDirectoryError,
+                 weak_ptr_factory_.GetWeakPtr(), error_callback);
+
+  const base::Closure closure =
+      base::Bind(&DeleteObjectOnUIThread, storage_name_, read_only_, object_id,
+                 success_callback_wrapper, error_callback_wrapper);
+
+  EnsureInitAndRunTask(PendingTaskInfo(

[Lei Zhang, 2015/03/07 01:17:24]

You can just call content::BrowserThread::PostTask() directly at this point. The
first argument to PendingTaskInfo() is empty, and initialization must have
already taken place.

[yawano, 2015/03/09 04:37:48]

Done.

+      base::FilePath(), content::BrowserThread::UI, FROM_HERE, closure));
+}
+
 void MTPDeviceDelegateImplLinux::EnsureInitAndRunTask(
     const PendingTaskInfo& task_info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if ((init_state_ == INITIALIZED) && !task_in_progress_) {
     RunTask(task_info);
     return;
   }
 
   // Only *Internal functions have empty paths. Since they are the continuation
   // of the current running task, they get to cut in line.
@@ skipping 286 matching lines @@
   const base::Closure closure = base::Bind(&CloseFileDescriptor,
                                            source_file_descriptor);
 
   content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
                                    closure);
 
   error_callback.Run(error);
   PendingRequestDone();
 }
 
+void MTPDeviceDelegateImplLinux::OnDidDeleteObject(
+    const uint32 object_id,
+    const DeleteObjectSuccessCallback success_callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  EvictCachedPathToId(object_id);
+  success_callback.Run();
+  PendingRequestDone();
+}
+
+void MTPDeviceDelegateImplLinux::HandleDeleteFileOrDirectoryError(
+    const ErrorCallback& error_callback,
+    base::File::Error error) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  error_callback.Run(error);
+  PendingRequestDone();
+}
+
 void MTPDeviceDelegateImplLinux::HandleDeviceFileError(
     const ErrorCallback& error_callback,
     uint32 file_id,
     base::File::Error error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  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);
-    }
-  }
+  EvictCachedPathToId(file_id);
   error_callback.Run(error);
   PendingRequestDone();
 }
 
 base::FilePath MTPDeviceDelegateImplLinux::NextUncachedPathComponent(
     const base::FilePath& path,
     const base::FilePath& cached_path) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(cached_path.empty() || cached_path.IsParent(path));
 
@@ skipping 53 matching lines @@
   const MTPFileNode* current_node = root_node_.get();
   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 MTPDeviceDelegateImplLinux::EvictCachedPathToId(const uint32 id) {
+  FileIdToMTPFileNodeMap::iterator it = file_id_to_node_map_.find(id);
+  if (it != file_id_to_node_map_.end()) {
+    MTPFileNode* parent = it->second->parent();

[Lei Zhang, 2015/03/07 01:17:24]

Assuming we add MTPNode::HasChildren(), I would
DCHECK(!it->second->HasChildren()); here as well.

[yawano, 2015/03/09 04:37:48]

Done. Added also for DeleteChild.

+    if (parent) {
+      const bool ret = parent->DeleteChild(id);
+      DCHECK(ret);
+    }
+  }
+}
+
 void CreateMTPDeviceAsyncDelegate(
     const std::string& device_location,
     const bool read_only,
     const CreateMTPDeviceAsyncDelegateCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   callback.Run(new MTPDeviceDelegateImplLinux(device_location, read_only));
 }