[go: nahoru, domu]

GetUniquePath{,Number} cleanups.

GetUniquePathNumber previously took an optional suffix that was poorly
documented and confusing to use. This change modifies the one caller
that used this suffix (in chrome/browser/downgrade) so that it is no
longer needed and removes it. The documentation for GetUniquePathNumber
has also been revised so that it is now accurate and comprehensible.

GetUniquePath previously returned the original path in case a unique
name could not be found. This was surely a bug, and is now fixed. The
documentation for this function has also been revised for clarity and
accuracy.

Furthermore, the callers of GetUniquePathNumber that did nothing more
with the result than format it into a new FilePath have been converted
to using GetUniquePath, thereby removing some duplication and
simplifying code.

R=eladalon@chromium.org

BUG: 785333
Change-Id: I7d75e615f58c1bc5f95b1edb7a744a3fc389df61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761596
Auto-Submit: Greg Thompson <grt@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Elad Alon <eladalon@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689770}
diff --git a/base/files/file_util.cc b/base/files/file_util.cc
index f11f456..1b8de96 100644
--- a/base/files/file_util.cc
+++ b/base/files/file_util.cc
@@ -25,17 +25,6 @@
 namespace base {
 
 #if !defined(OS_NACL_NONSFI)
-namespace {
-
-// The maximum number of 'uniquified' files we will try to create.
-// This is used when the filename we're trying to download is already in use,
-// so we create a new unique filename by appending " (nnn)" before the
-// extension, where 1 <= nnn <= kMaxUniqueFiles.
-// Also used by code that cleans up said files.
-static const int kMaxUniqueFiles = 100;
-
-}  // namespace
-
 int64_t ComputeDirectorySize(const FilePath& root_path) {
   int64_t running_size = 0;
   FileEnumerator file_iter(root_path, true, FileEnumerator::FILES);
@@ -280,33 +269,15 @@
   return true;
 }
 
-int GetUniquePathNumber(const FilePath& path,
-                        FilePath::StringPieceType suffix) {
-  // Storage for use by is_unique to reduce heap churn when looping.
-  FilePath::StringType path_with_suffix;
-
-  // A function that returns true if |candidate| is unique with and without an
-  // optional suffix.
-  const auto is_unique = [suffix,
-                          &path_with_suffix](const base::FilePath& candidate) {
-    if (!PathExists(candidate)) {
-      if (suffix.empty())
-        return true;
-      path_with_suffix = candidate.value();
-      suffix.AppendToString(&path_with_suffix);
-      if (!PathExists(FilePath(path_with_suffix)))
-        return true;
-    }
-    return false;
-  };
-
-  if (is_unique(path))
+int GetUniquePathNumber(const FilePath& path) {
+  DCHECK(!path.empty());
+  if (!PathExists(path))
     return 0;
 
   std::string number;
   for (int count = 1; count <= kMaxUniqueFiles; ++count) {
     StringAppendF(&number, " (%d)", count);
-    if (is_unique(path.InsertBeforeExtensionASCII(number)))
+    if (!PathExists(path.InsertBeforeExtensionASCII(number)))
       return count;
     number.clear();
   }
@@ -315,13 +286,11 @@
 }
 
 FilePath GetUniquePath(const FilePath& path) {
-  FilePath unique_path = path;
-  int uniquifier = GetUniquePathNumber(path);
-  if (uniquifier > 0) {
-    unique_path = unique_path.InsertBeforeExtensionASCII(
-        StringPrintf(" (%d)", uniquifier));
-  }
-  return unique_path;
+  DCHECK(!path.empty());
+  const int uniquifier = GetUniquePathNumber(path);
+  if (uniquifier > 0)
+    return path.InsertBeforeExtensionASCII(StringPrintf(" (%d)", uniquifier));
+  return uniquifier == 0 ? path : base::FilePath();
 }
 #endif  // !defined(OS_NACL_NONSFI)