Truncate the download file name if it exceeds the filesystem limit.

chrome/browser/download/download_path_reservation_tracker.cc

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 eab019053fa5920c5716aa998640c7d395f2df66..bd4cf41b15ba306e4d9489a1e7fff97ef9ae3f87 100644
--- a/chrome/browser/download/download_path_reservation_tracker.cc
+++ b/chrome/browser/download/download_path_reservation_tracker.cc
@@ -12,6 +12,8 @@
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
+#include "base/string_util.h"
+#include "base/sys_info.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
@@ -90,6 +92,22 @@ bool IsPathInUse(const FilePath& path) {
   return false;
 }
 
+// Truncate the given name so that its becomes .size() <= limit. Returns an
+// empty string if it is impossible due to unknown file-name encoding.
+FilePath::StringType TruncateFileName(const FilePath::StringType& name,
+                                      size_t limit) {
+  FilePath::StringType truncated;
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
+  TruncateUTF8ToByteSize(name, limit, &truncated);
+#elif defined(OS_WIN)
+  // TODO(kinaba): Implement for Windows, where StringType == string16.
+#else
+  // We cannot generally assume that the file name encoding is in UTF-8 (see
+  // the comment for FilePath::AsUTF8Unsafe), hence no safe way to truncate.

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'm inclined to think that we'll end up with a better user experience if we
power through and do it anyway, but that might be worth checking with people
wiser in the ways of character encodings than I.

+#endif
+  return truncated;
+}
+
 // 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
@@ -139,6 +157,33 @@ void CreateReservation(
     target_path = dir.Append(filename);
   }
 
+  // Check the limit of file name length if it could be obtained. When the
+  // suggested name exceeds the limit, truncate or prompt the user.
+  bool name_too_long = false;
+  if (is_path_writeable) {
+    int max_length = base::SysInfo::GetMaximumPathComponentLength(dir);
+    if (max_length > 0) {
+      // The extension should not be truncated, so we split it first.
+      FilePath::StringType ext = filename.Extension();
+      FilePath::StringType name = filename.RemoveExtension().value();
+
+      // Reserve several characters we may append later.
+      const int kExtraChars = sizeof(" (100).crdownload") - 1;

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'd rather find some way to export the amount of space we need to reserve for
the intermediate name from the system that's creating the intermediate name
(chrome_download_manager_delegate.h).  But that's a nit that we can resolve in
later reviews.

+      int limit = max_length - ext.size() - kExtraChars;
+      if (static_cast<int>(name.size()) > limit) {
+        name_too_long = true;
+        if (limit > 0) {

[Randy Smith (Not in Mondays), 2013/02/07 19:12:03]

I'd be inclined to reserve a few more characters here, though I'll admit I'm not
sure how many; five?  It should be pulled out into a separate constant, as I
could imagine people might want to play with the value for maintainability.

[kinaba, 2013/02/08 10:46:57]

Pulled out the constant (tentatively 5).
IMHO we can be more aggressive like, say, 100, since all modern filesystem has
max_length>=255 (ecryptfs's 143 is notably low) but not quite sure either...

+          // Truncate the name to fit in |limit|.
+          FilePath::StringType truncated = TruncateFileName(name, limit);
+          if (!truncated.empty()) {
+            target_path = dir.Append(truncated + ext);
+            name_too_long = false;
+          }
+        }
+      }
+    }
+  }
+
   if (is_path_writeable && should_uniquify && IsPathInUse(target_path)) {
     has_conflicts = true;
     for (int uniquifier = 1;
@@ -154,9 +199,9 @@ void CreateReservation(
     }
   }
   reservations[download_id] = target_path;
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, target_path, (is_path_writeable && !has_conflicts)));
+  bool verified = (is_path_writeable && !has_conflicts && !name_too_long);
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, target_path, verified));
 }
 
 // Called on the FILE thread to update the path of the reservation associated