Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2821)

Unified Diff: chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc

Issue 1025553006: Fix error handling of CopyFileFromLocal and remove unnecessary PendingRequestDone. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 b14461db62a7e6d33a6b4acaa36a58e7b5616b1b..88999268a34fbcc200f551abde4db8e11f1a10a2 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
@@ -287,11 +287,14 @@ void CloseStorageAndDestroyTaskHelperOnUIThread(
storage_name, read_only);
}
-// Opens |file_path| with |flags|.
-int OpenFileDescriptor(const char* file_path, const int flags) {
+// Opens |file_path| with |flags|. Returns the result as a pair.
+// first is file descriptor.
+// second is errno if it failed.
+std::pair<int, int> OpenFileDescriptor(const char* file_path, const int flags) {
DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
- return open(file_path, flags);
+ int file_descriptor = open(file_path, flags);
+ return std::make_pair(file_descriptor, file_descriptor < 0 ? errno : 0);
}
// Closes |file_descriptor| on file thread.
@@ -624,17 +627,20 @@ void MTPDeviceDelegateImplLinux::CopyFileFromLocal(
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));
+ // Get file info of destination file path.
+ const GetFileInfoSuccessCallback success_callback_wrapper = base::Bind(
+ &MTPDeviceDelegateImplLinux::OnDidGetDestFileInfoToCopyFileFromLocal,
+ weak_ptr_factory_.GetWeakPtr(), error_callback);
+ const ErrorCallback error_callback_wrapper = base::Bind(
+ &MTPDeviceDelegateImplLinux::OnGetDestFileInfoErrorToCopyFileFromLocal,
+ weak_ptr_factory_.GetWeakPtr(), source_file_path, device_file_path,
+ success_callback, error_callback);
+ const base::Closure closure =
+ base::Bind(&MTPDeviceDelegateImplLinux::GetFileInfoInternal,
+ weak_ptr_factory_.GetWeakPtr(), device_file_path,
+ success_callback_wrapper, error_callback_wrapper);
+ EnsureInitAndRunTask(PendingTaskInfo(
+ device_file_path, content::BrowserThread::IO, FROM_HERE, closure));
}
void MTPDeviceDelegateImplLinux::DeleteFile(
@@ -841,7 +847,6 @@ void MTPDeviceDelegateImplLinux::MoveFileLocalInternal(
if (source_file_info.is_directory) {
error_callback.Run(base::File::FILE_ERROR_NOT_A_FILE);
- PendingRequestDone();
return;
}
@@ -877,20 +882,24 @@ void MTPDeviceDelegateImplLinux::MoveFileLocalInternal(
base::Bind(&FakeCopyFileProgressCallback),
success_callback_wrapper, error_callback);
}
-
- PendingRequestDone();
}
-void MTPDeviceDelegateImplLinux::CopyFileFromLocalInternal(
+void MTPDeviceDelegateImplLinux::OnDidOpenFDToCopyFileFromLocal(
const base::FilePath& device_file_path,
const CopyFileFromLocalSuccessCallback& success_callback,
const ErrorCallback& error_callback,
- const int source_file_descriptor) {
+ const std::pair<int, int>& open_fd_result) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+ int source_file_descriptor = open_fd_result.first;
if (source_file_descriptor < 0) {
- error_callback.Run(base::File::FILE_ERROR_INVALID_OPERATION);
- PendingRequestDone();
+ if (open_fd_result.second == ENOENT)
+ error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
+ else if (open_fd_result.second == ENOTDIR)
+ error_callback.Run(base::File::FILE_ERROR_NOT_A_FILE);
+ else
+ error_callback.Run(base::File::FILE_ERROR_FAILED);
+
return;
}
@@ -918,10 +927,8 @@ void MTPDeviceDelegateImplLinux::CopyFileFromLocalInternal(
base::FilePath(), content::BrowserThread::UI, FROM_HERE, closure));
} else {
HandleCopyFileFromLocalError(error_callback, source_file_descriptor,
- base::File::FILE_ERROR_INVALID_OPERATION);
+ base::File::FILE_ERROR_NOT_FOUND);
}
-
- PendingRequestDone();
}
void MTPDeviceDelegateImplLinux::DeleteFileInternal(
@@ -940,8 +947,6 @@ void MTPDeviceDelegateImplLinux::DeleteFileInternal(
else
error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
}
-
- PendingRequestDone();
}
void MTPDeviceDelegateImplLinux::DeleteDirectoryInternal(
@@ -953,14 +958,12 @@ void MTPDeviceDelegateImplLinux::DeleteDirectoryInternal(
if (!file_info.is_directory) {
error_callback.Run(base::File::FILE_ERROR_NOT_A_DIRECTORY);
- PendingRequestDone();
return;
}
uint32 directory_id;
if (!CachedPathToId(file_path, &directory_id)) {
error_callback.Run(base::File::FILE_ERROR_NOT_FOUND);
- PendingRequestDone();
return;
}
@@ -970,7 +973,6 @@ void MTPDeviceDelegateImplLinux::DeleteDirectoryInternal(
file_id_to_node_map_.find(directory_id);
if (it != file_id_to_node_map_.end() && it->second->HasChildren()) {
error_callback.Run(base::File::FILE_ERROR_NOT_EMPTY);
- PendingRequestDone();
return;
}
@@ -989,7 +991,6 @@ void MTPDeviceDelegateImplLinux::DeleteDirectoryInternal(
1 /* max_size */, success_callback_wrapper, error_callback_wrapper);
EnsureInitAndRunTask(PendingTaskInfo(
base::FilePath(), content::BrowserThread::UI, FROM_HERE, closure));
- PendingRequestDone();
}
void MTPDeviceDelegateImplLinux::OnDidReadDirectoryToDeleteDirectory(
@@ -1193,6 +1194,39 @@ void MTPDeviceDelegateImplLinux::OnDidGetFileInfoToCreateSnapshotFile(
WriteDataIntoSnapshotFile(snapshot_file_info);
}
+void MTPDeviceDelegateImplLinux::OnDidGetDestFileInfoToCopyFileFromLocal(
+ 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_INVALID_OPERATION);
+ else
+ error_callback.Run(base::File::FILE_ERROR_FAILED);
+}
+
+void MTPDeviceDelegateImplLinux::OnGetDestFileInfoErrorToCopyFileFromLocal(
+ const base::FilePath& source_file_path,
+ const base::FilePath& device_file_path,
+ const CopyFileFromLocalSuccessCallback& success_callback,
+ const ErrorCallback& error_callback,
+ const base::File::Error error) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ if (error != base::File::FILE_ERROR_NOT_FOUND) {
+ error_callback.Run(error);
+ return;
+ }
+
+ content::BrowserThread::PostTaskAndReplyWithResult(
+ content::BrowserThread::FILE, FROM_HERE,
+ base::Bind(&OpenFileDescriptor, source_file_path.value().c_str(),
+ O_RDONLY),
+ base::Bind(&MTPDeviceDelegateImplLinux::OnDidOpenFDToCopyFileFromLocal,
+ weak_ptr_factory_.GetWeakPtr(), device_file_path,
+ success_callback, error_callback));
+}
+
void MTPDeviceDelegateImplLinux::OnDidReadDirectory(
uint32 dir_id,
const ReadDirectorySuccessCallback& success_callback,

Powered by Google App Engine
This is Rietveld 408576698