Append a number to make app shortcut name unique.

- Move DownloadManager's unique path helpers into download utils;
- Use those functions to make shortcut file name unqiue.

BUG=642
TEST=Verify fixes for issue 642.

Review URL: http://codereview.chromium.org/1276003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42769 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 69 insertions, 38 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 0075352..7e4740e 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -79,34 +79,6 @@ const int kUpdateTimeMs = 1000;
 // more negative.
 const int kUninitializedHandle = 0;
 
-// Appends the passed the number between parenthesis the path before the
-// extension.
-void AppendNumberToPath(FilePath* path, int number) {
-  file_util::InsertBeforeExtension(path,
-      StringPrintf(FILE_PATH_LITERAL(" (%d)"), number));
-}
-
-// Attempts to find a number that can be appended to that path to make it
-// unique. If |path| does not exist, 0 is returned.  If it fails to find such
-// a number, -1 is returned.
-int GetUniquePathNumber(const FilePath& path) {
-  const int kMaxAttempts = 100;
-
-  if (!file_util::PathExists(path))
-    return 0;
-
-  FilePath new_path;
-  for (int count = 1; count <= kMaxAttempts; ++count) {
-    new_path = FilePath(path);
-    AppendNumberToPath(&new_path, count);
-
-    if (!file_util::PathExists(new_path))
-      return count;
-  }
-
-  return -1;
-}
-
 // Used to sort download items based on descending start time.
 bool CompareStartTime(DownloadItem* first, DownloadItem* second) {
   return first->start_time() > second->start_time();
@@ -324,7 +296,7 @@ FilePath DownloadItem::GetFileName() const {
     return file_name_;
   if (path_uniquifier_ > 0) {
     FilePath name(original_name_);
-    AppendNumberToPath(&name, path_uniquifier_);
+    download_util::AppendNumberToPath(&name, path_uniquifier_);
     return name;
   }
   return original_name_;
@@ -699,8 +671,10 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
 
   // Do not add the path uniquifier if we are saving to a specific path as in
   // the drag-out case.
-  if (info->save_info.file_path.empty())
-    info->path_uniquifier = GetUniquePathNumber(info->suggested_path);
+  if (info->save_info.file_path.empty()) {
+    info->path_uniquifier = download_util::GetUniquePathNumber(
+        info->suggested_path);
+  }
 
   // If the download is deemed dangerous, we'll use a temporary name for it.
   if (info->is_dangerous) {
@@ -720,7 +694,8 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
   } else {
     // We know the final path, build it if necessary.
     if (info->path_uniquifier > 0) {
-      AppendNumberToPath(&(info->suggested_path), info->path_uniquifier);
+      download_util::AppendNumberToPath(&(info->suggested_path),
+                                        info->path_uniquifier);
       // Setting path_uniquifier to 0 to make sure we don't try to unique it
       // later on.
       info->path_uniquifier = 0;
@@ -1016,9 +991,9 @@ void DownloadManager::ProceedWithFinishedDangerousDownload(
     // have the same path.  This is because we uniquify the name on download
     // start, and at that time the first file does not exists yet, so the second
     // file gets the same name.
-    uniquifier = GetUniquePathNumber(new_path);
+    uniquifier = download_util::GetUniquePathNumber(new_path);
     if (uniquifier > 0)
-      AppendNumberToPath(&new_path, uniquifier);
+      download_util::AppendNumberToPath(&new_path, uniquifier);
     success = file_util::Move(path, new_path);
   } else {
     NOTREACHED();
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index 5d0c49c..720d203 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -495,4 +495,32 @@ void UpdateAppIconDownloadProgress(int download_count,
 }
 #endif
 
+// Appends the passed the number between parenthesis the path before the
+// extension.
+void AppendNumberToPath(FilePath* path, int number) {
+  file_util::InsertBeforeExtension(path,
+      StringPrintf(FILE_PATH_LITERAL(" (%d)"), number));
+}
+
+// Attempts to find a number that can be appended to that path to make it
+// unique. If |path| does not exist, 0 is returned.  If it fails to find such
+// a number, -1 is returned.
+int GetUniquePathNumber(const FilePath& path) {
+  const int kMaxAttempts = 100;
+
+  if (!file_util::PathExists(path))
+    return 0;
+
+  FilePath new_path;
+  for (int count = 1; count <= kMaxAttempts; ++count) {
+    new_path = FilePath(path);
+    AppendNumberToPath(&new_path, count);
+
+    if (!file_util::PathExists(new_path))
+      return count;
+  }
+
+  return -1;
+}
+
 }  // namespace download_util
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index d692a46..dee3600 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -166,6 +166,15 @@ void UpdateAppIconDownloadProgress(int download_count,
                                    bool progress_known,
                                    float progress);
 
+// Appends the passed the number between parenthesis the path before the
+// extension.
+void AppendNumberToPath(FilePath* path, int number);
+
+// Attempts to find a number that can be appended to that path to make it
+// unique. If |path| does not exist, 0 is returned.  If it fails to find such
+// a number, -1 is returned.
+int GetUniquePathNumber(const FilePath& path);
+
 }  // namespace download_util
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_
diff --git a/chrome/browser/web_applications/web_app.cc b/chrome/browser/web_applications/web_app.cc
index 9eebef4..b5714b2 100644
--- a/chrome/browser/web_applications/web_app.cc
+++ b/chrome/browser/web_applications/web_app.cc
@@ -21,6 +21,7 @@
 #include "base/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_constants.h"
@@ -378,10 +379,21 @@ bool CreateShortcutTask::CreateShortcut() {
       web_app::GenerateApplicationNameFromURL(shortcut_info_.url).c_str(),
       profile_path_);
 
+  FilePath shortcut_to_pin;
+
   bool success = true;
   for (size_t i = 0; i < shortcut_paths.size(); ++i) {
     FilePath shortcut_file = shortcut_paths[i].Append(file_name).
         ReplaceExtension(FILE_PATH_LITERAL(".lnk"));
+
+    int unique_number = download_util::GetUniquePathNumber(shortcut_file);
+    if (unique_number == -1) {
+      success = false;
+      continue;
+    } else if (unique_number > 0) {
+      download_util::AppendNumberToPath(&shortcut_file, unique_number);
+    }
+
     success &= file_util::CreateShortcutLink(chrome_exe.c_str(),
         shortcut_file.value().c_str(),
         chrome_folder.c_str(),
@@ -390,13 +402,20 @@ bool CreateShortcutTask::CreateShortcut() {
         icon_file.value().c_str(),
         0,
         app_id.c_str());
+
+    // Any shortcut would work for the pinning. We use the first one.
+    if (success && pin_to_taskbar && shortcut_to_pin.empty())
+      shortcut_to_pin = shortcut_file;
   }
 
   if (success && pin_to_taskbar) {
-    // Any shortcut would work for the pinning. We use the first one.
-    FilePath shortcut_file = shortcut_paths[0].Append(file_name).
-        ReplaceExtension(FILE_PATH_LITERAL(".lnk"));
-    success &= file_util::TaskbarPinShortcutLink(shortcut_file.value().c_str());
+    if (!shortcut_to_pin.empty()) {
+      success &= file_util::TaskbarPinShortcutLink(
+          shortcut_to_pin.value().c_str());
+    } else {
+      NOTREACHED();
+      success = false;
+    }
   }
 
   return success;