Print Preview: Fix failure to save with long page title

ui/shell_dialogs/select_file_dialog.cc

Index: ui/shell_dialogs/select_file_dialog.cc
diff --git a/ui/shell_dialogs/select_file_dialog.cc b/ui/shell_dialogs/select_file_dialog.cc
index 1aa7a0e26164079e0563943180353d03cdb6c861..42e3bf7b2c57e6c3b904467332c79025ec213ca0 100644
--- a/ui/shell_dialogs/select_file_dialog.cc
+++ b/ui/shell_dialogs/select_file_dialog.cc
@@ -5,6 +5,7 @@
 #include "ui/shell_dialogs/select_file_dialog.h"
 
 #include <stddef.h>
+#include <algorithm>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ -98,8 +99,35 @@ void SelectFileDialog::SelectFile(
     return;
   }
 
+  // Shorten filename if necessary.

[Lei Zhang, 2017/04/13 01:34:51]

This is getting long-ish. Can we split it off into its own function?

[rbpotter, 2017/04/13 19:21:02]

Done.

+  base::FilePath filename = default_path.BaseName();
+  base::FilePath path = default_path;
+  const size_t kMaxNameLength = 255;
+  if (filename.value().length() > kMaxNameLength) {
+    base::FilePath::StringType extension = filename.FinalExtension();
+    filename = filename.RemoveFinalExtension();
+    // 1 for . plus 12 for longest known extension.
+    size_t max_extension_length = 13;
+    if (kMaxNameLength > filename.value().length()) {

[Lei Zhang, 2017/04/13 01:34:51]

This reads more naturally if you flip the equation around. It's easier to think
of "if the file name length is less than the maximum" than "if the maximum is
greater than the file name length".

[rbpotter, 2017/04/13 19:21:02]

Done.

+      max_extension_length = std::max(
+          max_extension_length, kMaxNameLength - filename.value().length());
+    }
+    size_t extension_length = extension.length();
+    if (extension.length() >= max_extension_length) {
+      // Take the last max_extension_length - 1 characters (save 1 character
+      // for the '.').
+      extension =

[Lei Zhang, 2017/04/13 01:34:50]

This is a bit weird. If the code does not go through this path, then |extension|
has a leading dot. If we go through here, then it does not. Looking at
FilePath::AddExtension(), it looks like AddExtension() can tolerate both cases,
but it may be good to be consistent.

[rbpotter, 2017/04/13 19:21:02]

Done.

+          extension.substr(extension.length() - max_extension_length + 1,
+                           max_extension_length - 1);
+      extension_length = max_extension_length;
+    }
+    path = default_path.DirName().Append(
+        filename.value().substr(0, kMaxNameLength - extension_length));
+    path = path.AddExtension(extension);
+  }
+
   // Call the platform specific implementation of the file selection dialog.
-  SelectFileImpl(type, title, default_path, file_types, file_type_index,
+  SelectFileImpl(type, title, path, file_types, file_type_index,
                  default_extension, owning_window, params);
 }