Print Preview: Fix failure to save with long page title

ui/shell_dialogs/select_file_dialog.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/shell_dialogs/select_file_dialog.h"
 
 #include <stddef.h>
+#include <algorithm>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "ui/shell_dialogs/select_file_dialog_factory.h"
 #include "ui/shell_dialogs/select_file_policy.h"
 #include "ui/shell_dialogs/selected_file_info.h"
@@ skipping 73 matching lines @@
 
     // Inform the listener that no file was selected.
     // Post a task rather than calling FileSelectionCanceled directly to ensure
     // that the listener is called asynchronously.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&SelectFileDialog::CancelFileSelection, this, params));
     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);
 }
 
 bool SelectFileDialog::HasMultipleFileTypeChoices() {
   return HasMultipleFileTypeChoicesImpl();
 }
 
 SelectFileDialog::SelectFileDialog(Listener* listener,
                                    ui::SelectFilePolicy* policy)
     : listener_(listener),
       select_file_policy_(policy) {
   DCHECK(listener_);
 }
 
 SelectFileDialog::~SelectFileDialog() {}
 
 void SelectFileDialog::CancelFileSelection(void* params) {
   if (listener_)
     listener_->FileSelectionCanceled(params);
 }
 
 }  // namespace ui