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

Unified Diff: chrome/browser/download/download_path_reservation_tracker.cc

Issue 2453633006: [downloads] Move platform specific code out of DownloadTargetDeterminer. (Closed)
Patch Set: . Created 3 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/download/download_path_reservation_tracker.cc
diff --git a/chrome/browser/download/download_path_reservation_tracker.cc b/chrome/browser/download/download_path_reservation_tracker.cc
index cb813a5bdeb1cc35eb96ddf1b6fda19993247dd1..60f0a2267fbcf795a4eb6229da36bc3238a20250 100644
--- a/chrome/browser/download/download_path_reservation_tracker.cc
+++ b/chrome/browser/download/download_path_reservation_tracker.cc
@@ -7,6 +7,7 @@
#include <stddef.h>
#include <map>
+#include <string>
#include "base/bind.h"
#include "base/callback.h"
@@ -24,6 +25,8 @@
#include "chrome/common/features.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/download_item.h"
+#include "net/base/filename_util.h"
+#include "url/gurl.h"
using content::BrowserThread;
using content::DownloadItem;
@@ -35,13 +38,13 @@ typedef std::map<ReservationKey, base::FilePath> ReservationMap;
// The lower bound for file name truncation. If the truncation results in a name
// shorter than this limit, we give up automatic truncation and prompt the user.
-static const size_t kTruncatedNameLengthLowerbound = 5;
+const size_t kTruncatedNameLengthLowerbound = 5;
// The length of the suffix string we append for an intermediate file name.
// In the file name truncation, we keep the margin to append the suffix.
// TODO(kinaba): remove the margin. The user should be able to set maximum
// possible filename.
-static const size_t kIntermediateNameSuffixLength = sizeof(".crdownload") - 1;
+const size_t kIntermediateNameSuffixLength = sizeof(".crdownload") - 1;
// Map of download path reservations. Each reserved path is associated with a
// ReservationKey=DownloadItem*. This object is destroyed in |Revoke()| when
@@ -150,22 +153,104 @@ bool TruncateFileName(base::FilePath* path, size_t limit) {
return true;
}
+// Create a unique filename by appending a uniquifier. Modifies |path| in place
+// if successful and returns true. Otherwise |path| is left unmodified and
+// returns false.
+bool CreateUniqueFilename(int max_path_component_length, base::FilePath* path) {
+ for (int uniquifier = 1;
+ uniquifier <= DownloadPathReservationTracker::kMaxUniqueFiles;
+ ++uniquifier) {
+ // Append uniquifier.
+ std::string suffix(base::StringPrintf(" (%d)", uniquifier));
+ base::FilePath path_to_check(*path);
+ // If the name length limit is available (max_length != -1), and the
+ // the current name exceeds the limit, truncate.
+ if (max_path_component_length != -1) {
+ int limit = max_path_component_length - kIntermediateNameSuffixLength -
+ suffix.size();
+ // If truncation failed, give up uniquification.
+ if (limit <= 0 || !TruncateFileName(&path_to_check, limit))
+ break;
+ }
+ path_to_check = path_to_check.InsertBeforeExtensionASCII(suffix);
+
+ if (!IsPathInUse(path_to_check)) {
+ *path = path_to_check;
+ return true;
+ }
+ }
+ return false;
+}
+
+struct CreateReservationInfo {
+ ReservationKey key;
+ base::FilePath source_path;
+ base::FilePath suggested_path;
+ base::FilePath default_download_path;
+ bool create_target_directory;
+ DownloadPathReservationTracker::FilenameConflictAction conflict_action;
+ DownloadPathReservationTracker::ReservedPathCallback completion_callback;
+};
+
+// Verify that |target_path| can be written to and also resolve any conflicts if
+// necessary by uniquifying the filename.
+PathValidationResult ValidatePathAndResolveConflicts(
+ const CreateReservationInfo& info,
+ base::FilePath* target_path) {
+ // Check writability of the suggested path. If we can't write to it, default
+ // to the user's Documents directory. We'll prompt them in this case. No
+ // further amendments are made to the filename since the user is going to be
+ // prompted.
+ if (!base::PathIsWritable(target_path->DirName())) {
+ DVLOG(1) << "Unable to write to path \"" << target_path->value() << "\"";
+ base::FilePath target_dir;
+ PathService::Get(chrome::DIR_USER_DOCUMENTS, &target_dir);
+ *target_path = target_dir.Append(target_path->BaseName());
+ return PathValidationResult::PATH_NOT_WRITABLE;
+ }
+
+ int max_path_component_length =
+ base::GetMaximumPathComponentLength(target_path->DirName());
+ // Check the limit of file name length if it could be obtained. When the
+ // suggested name exceeds the limit, truncate or prompt the user.
+ if (max_path_component_length != -1) {
+ int limit = max_path_component_length - kIntermediateNameSuffixLength;
+ if (limit <= 0 || !TruncateFileName(target_path, limit))
+ return PathValidationResult::NAME_TOO_LONG;
+ }
+
+ if (!IsPathInUse(*target_path))
+ return PathValidationResult::SUCCESS;
+
+ switch (info.conflict_action) {
+ case DownloadPathReservationTracker::UNIQUIFY:
+ return CreateUniqueFilename(max_path_component_length, target_path)
+ ? PathValidationResult::SUCCESS
+ : PathValidationResult::CONFLICT;
+
+ case DownloadPathReservationTracker::OVERWRITE:
+ return PathValidationResult::SUCCESS;
+
+ case DownloadPathReservationTracker::PROMPT:
+ return PathValidationResult::CONFLICT;
+ }
+ NOTREACHED();
+ return PathValidationResult::SUCCESS;
+}
+
// Called on the FILE thread to reserve a download path. This method:
// - Creates directory |default_download_path| if it doesn't exist.
// - Verifies that the parent directory of |suggested_path| exists and is
// writeable.
// - Truncates the suggested name if it exceeds the filesystem's limit.
// - Uniquifies |suggested_path| if |should_uniquify_path| is true.
-// - Returns true if |reserved_path| has been successfully verified.
-bool CreateReservation(
- ReservationKey key,
- const base::FilePath& suggested_path,
- const base::FilePath& default_download_path,
- bool create_directory,
- DownloadPathReservationTracker::FilenameConflictAction conflict_action,
- base::FilePath* reserved_path) {
+// - Schedules |callback| on the UI thread with the reserved path and a flag
+// indicating whether the returned path has been successfully verified.
+// - Returns the result of creating the path reservation.
+PathValidationResult CreateReservation(const CreateReservationInfo& info,
+ base::FilePath* reserved_path) {
DCHECK_CURRENTLY_ON(BrowserThread::FILE);
- DCHECK(suggested_path.IsAbsolute());
+ DCHECK(info.suggested_path.IsAbsolute());
// Create a reservation map if one doesn't exist. It will be automatically
// deleted when all the reservations are revoked.
@@ -178,14 +263,11 @@ bool CreateReservation(
//
// Revoking and re-acquiring the reservation forces us to re-verify the claims
// we are making about the path.
- g_reservation_map->erase(key);
+ g_reservation_map->erase(info.key);
- base::FilePath target_path(suggested_path.NormalizePathSeparators());
+ base::FilePath target_path(info.suggested_path.NormalizePathSeparators());
base::FilePath target_dir = target_path.DirName();
base::FilePath filename = target_path.BaseName();
- bool is_path_writeable = true;
- bool has_conflicts = false;
- bool name_too_long = false;
// Create target_dir if necessary and appropriate. target_dir may be the last
// directory that the user selected in a FilePicker; if that directory has
@@ -193,79 +275,17 @@ bool CreateReservation(
// create the directory if it is the default Downloads directory or if the
// caller explicitly requested automatic directory creation.
if (!base::DirectoryExists(target_dir) &&
- (create_directory ||
- (!default_download_path.empty() &&
- (default_download_path == target_dir)))) {
+ (info.create_target_directory ||
+ (!info.default_download_path.empty() &&
+ (info.default_download_path == target_dir)))) {
base::CreateDirectory(target_dir);
}
- // Check writability of the suggested path. If we can't write to it, default
- // to the user's "My Documents" directory. We'll prompt them in this case.
- if (!base::PathIsWritable(target_dir)) {
- DVLOG(1) << "Unable to write to directory \"" << target_dir.value() << "\"";
-#if defined(OS_ANDROID)
- // On Android, DIR_USER_DOCUMENTS is in reality a subdirectory
- // of DIR_ANDROID_APP_DATA which isn't accessible by other apps.
- reserved_path->clear();
- (*g_reservation_map)[key] = *reserved_path;
- return false;
-#else
- is_path_writeable = false;
- PathService::Get(chrome::DIR_USER_DOCUMENTS, &target_dir);
- target_path = target_dir.Append(filename);
-#endif // defined(OS_ANDROID)
- }
-
- if (is_path_writeable) {
- // Check the limit of file name length if it could be obtained. When the
- // suggested name exceeds the limit, truncate or prompt the user.
- int max_length = base::GetMaximumPathComponentLength(target_dir);
- if (max_length != -1) {
- int limit = max_length - kIntermediateNameSuffixLength;
- if (limit <= 0 || !TruncateFileName(&target_path, limit))
- name_too_long = true;
- }
-
- // Uniquify the name, if it already exists.
- if (!name_too_long && IsPathInUse(target_path)) {
- has_conflicts = true;
- if (conflict_action == DownloadPathReservationTracker::OVERWRITE) {
- has_conflicts = false;
- }
- // If ...PROMPT, then |has_conflicts| will remain true, |verified| will be
- // false, and CDMD will prompt.
- if (conflict_action == DownloadPathReservationTracker::UNIQUIFY) {
- for (int uniquifier = 1;
- uniquifier <= DownloadPathReservationTracker::kMaxUniqueFiles;
- ++uniquifier) {
- // Append uniquifier.
- std::string suffix(base::StringPrintf(" (%d)", uniquifier));
- base::FilePath path_to_check(target_path);
- // If the name length limit is available (max_length != -1), and the
- // the current name exceeds the limit, truncate.
- if (max_length != -1) {
- int limit =
- max_length - kIntermediateNameSuffixLength - suffix.size();
- // If truncation failed, give up uniquification.
- if (limit <= 0 || !TruncateFileName(&path_to_check, limit))
- break;
- }
- path_to_check = path_to_check.InsertBeforeExtensionASCII(suffix);
-
- if (!IsPathInUse(path_to_check)) {
- target_path = path_to_check;
- has_conflicts = false;
- break;
- }
- }
- }
- }
- }
-
- (*g_reservation_map)[key] = target_path;
- bool verified = (is_path_writeable && !has_conflicts && !name_too_long);
+ PathValidationResult result =
+ ValidatePathAndResolveConflicts(info, &target_path);
+ (*g_reservation_map)[info.key] = target_path;
*reserved_path = target_path;
- return verified;
+ return result;
}
// Called on the FILE thread to update the path of the reservation associated
@@ -301,9 +321,9 @@ void RevokeReservation(ReservationKey key) {
void RunGetReservedPathCallback(
const DownloadPathReservationTracker::ReservedPathCallback& callback,
const base::FilePath* reserved_path,
- bool verified) {
+ PathValidationResult result) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
- callback.Run(*reserved_path, verified);
+ callback.Run(result, *reserved_path);
}
DownloadItemObserver::DownloadItemObserver(DownloadItem* download_item)
@@ -385,18 +405,21 @@ void DownloadPathReservationTracker::GetReservedPath(
// DownloadItemObserver deletes itself.
base::FilePath* reserved_path = new base::FilePath;
+ base::FilePath source_path;
+ if (download_item->GetURL().SchemeIsFile())
+ net::FileURLToFilePath(download_item->GetURL(), &source_path);
+ CreateReservationInfo info = {static_cast<ReservationKey>(download_item),
+ source_path,
+ target_path,
+ default_path,
+ create_directory,
+ conflict_action,
+ callback};
+
BrowserThread::PostTaskAndReplyWithResult(
- BrowserThread::FILE,
- FROM_HERE,
- base::Bind(&CreateReservation,
- download_item,
- target_path,
- default_path,
- create_directory,
- conflict_action,
- reserved_path),
- base::Bind(&RunGetReservedPathCallback,
- callback,
+ BrowserThread::FILE, FROM_HERE,
+ base::Bind(&CreateReservation, info, reserved_path),
+ base::Bind(&RunGetReservedPathCallback, callback,
base::Owned(reserved_path)));
}

Powered by Google App Engine
This is Rietveld 408576698