Fix file name generation in PdfPrinterHandler.

Make it behave more like Save As as implemented in the downloads code.

BUG=375330,782041

Change-Id: I62d373f8f356bc6614ffa2ad293fc05a3d9e1ba3
Reviewed-on: https://chromium-review.googlesource.com/759411
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515276}

chrome/browser/ui/webui/print_preview/pdf_printer_handler.cc

@@ -29,8 +29,10 @@
 #include "chrome/browser/ui/webui/print_preview/sticky_settings.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/cloud_devices/common/printer_description.h"
+#include "components/url_formatter/url_formatter.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
+#include "net/base/filename_util.h"
 #include "printing/print_job_constants.h"
 #include "printing/printing_context.h"
 #include "printing/units.h"
@@ -38,6 +40,8 @@
 
 namespace {
 
+constexpr base::FilePath::CharType kPdfExtension[] = FILE_PATH_LITERAL("pdf");
+
 class PrintingContextDelegate : public printing::PrintingContext::Delegate {
  public:
   // PrintingContext::Delegate methods.
@@ -194,9 +198,21 @@ void PdfPrinterHandler::StartPrint(
   DCHECK(!print_callback_);
   print_callback_ = std::move(callback);
 
+  printing::PrintPreviewDialogController* dialog_controller =
+      printing::PrintPreviewDialogController::GetInstance();
+  content::WebContents* initiator =
+      dialog_controller ? dialog_controller->GetInitiator(preview_web_contents_)
+                        : nullptr;
+  const GURL& initiator_url =
+      initiator ? initiator->GetLastCommittedURL() : GURL::EmptyGURL();
+  bool title_is_url = url_formatter::FormatUrl(initiator_url) == job_title;
+  base::FilePath path = title_is_url ? GetFileNameForURL(initiator_url)
+                                     : GetFileNameForPrintJobTitle(job_title);
+
   base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
   bool prompt_user = !cmdline->HasSwitch(switches::kKioskModePrinting);
-  SelectFile(GetFileNameForPrintJobTitle(job_title), prompt_user);
+
+  SelectFile(path, initiator, prompt_user);
 }
 
 void PdfPrinterHandler::FileSelected(const base::FilePath& path,
@@ -230,16 +246,44 @@ base::FilePath PdfPrinterHandler::GetFileNameForPrintJobTitle(
 
   base::i18n::ReplaceIllegalCharactersInPath(&print_job_title, '_');
   base::FilePath default_filename(print_job_title);
-  return default_filename.ReplaceExtension(FILE_PATH_LITERAL("pdf"));
+  base::FilePath::StringType ext = default_filename.Extension();
+  if (!ext.empty()) {
+    ext = ext.substr(1);
+    if (ext == kPdfExtension)
+      return default_filename;
+  }
+  return default_filename.AddExtension(kPdfExtension);
+}
+
+// static
+base::FilePath PdfPrinterHandler::GetFileNameForURL(const GURL& url) {
+  DCHECK(url.is_valid());
+
+  // TODO(thestig): This code is based on similar code in SavePackage in
+  // content/ that is not exposed via the public content API. Consider looking
+  // for a sane way to share the code.
+  if (url.SchemeIs(url::kDataScheme)) {
+    return base::FilePath::FromUTF8Unsafe("dataurl").ReplaceExtension(
+        kPdfExtension);
+  }
+
+  base::FilePath name =
+      net::GenerateFileName(url, std::string(), std::string(), std::string(),
+                            std::string(), std::string());
+
+  // If host is used as file name, try to decode punycode.
+  if (name.AsUTF8Unsafe() == url.host()) {
+    name = base::FilePath::FromUTF16Unsafe(
+        url_formatter::IDNToUnicode(url.host()));
+  }
+  if (name.AsUTF8Unsafe() == url.host())
+    return name.AddExtension(kPdfExtension);
+  return name.ReplaceExtension(kPdfExtension);
 }
 
 void PdfPrinterHandler::SelectFile(const base::FilePath& default_filename,
+                                   content::WebContents* initiator,
                                    bool prompt_user) {
-  printing::PrintPreviewDialogController* dialog_controller =
-      printing::PrintPreviewDialogController::GetInstance();
-  content::WebContents* initiator =
-      dialog_controller ? dialog_controller->GetInitiator(preview_web_contents_)
-                        : nullptr;
   if (prompt_user) {
     ChromeSelectFilePolicy policy(initiator);
     if (!policy.CanOpenSelectFileDialog()) {

chrome/browser/ui/webui/print_preview/pdf_printer_handler.h

@@ -32,6 +32,7 @@ namespace printing {
 class StickySettings;
 }
 
+class GURL;
 class Profile;
 
 class PdfPrinterHandler : public PrinterHandler,
@@ -70,9 +71,11 @@ class PdfPrinterHandler : public PrinterHandler,
   // Exposed for testing.
   static base::FilePath GetFileNameForPrintJobTitle(
       const base::string16& job_title);
+  static base::FilePath GetFileNameForURL(const GURL& url);
 
  protected:
   virtual void SelectFile(const base::FilePath& default_filename,
+                          content::WebContents* initiator,
                           bool prompt_user);
 
   // The print preview web contents. Protected so unit tests can access it.

chrome/browser/ui/webui/print_preview/pdf_printer_handler_unittest.cc

@@ -6,6 +6,9 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+#define FPL(x) FILE_PATH_LITERAL(x)
 
 using PdfPrinterHandlerTest = testing::Test;
 
@@ -14,24 +17,23 @@ TEST_F(PdfPrinterHandlerTest, GetFileNameForPrintJobTitle) {
     const char* input;
     const base::FilePath::CharType* expected_output;
   } kTestData[] = {
-      {"Foo", FILE_PATH_LITERAL("Foo.pdf")},
-      {"bar", FILE_PATH_LITERAL("bar.pdf")},
-      {"qux.html", FILE_PATH_LITERAL("qux.pdf")},
-      {"Print Me", FILE_PATH_LITERAL("Print Me.pdf")},
-      {"Print Me.html", FILE_PATH_LITERAL("Print Me.pdf")},
-      {"1l!egal_F@L#(N)ame.html", FILE_PATH_LITERAL("1l!egal_F@L#(N)ame.pdf")},
-      // TODO(thestig): Fix for https://crbug.com/782041
-      // Should be "example.com.pdf", like the regular file save dialog.
-      {"example.com", FILE_PATH_LITERAL("example.pdf")},
-      // TODO(thestig): Fix the weird truncation for https://crbug.com/375330
+      {"Foo", FPL("Foo.pdf")},
+      {"bar", FPL("bar.pdf")},
+      {"qux.html", FPL("qux.html.pdf")},
+      {"qux.pdf", FPL("qux.pdf")},
+      {"Print Me", FPL("Print Me.pdf")},
+      {"Print Me.html", FPL("Print Me.html.pdf")},
+      {"1l!egal_F@L#(N)ame.html", FPL("1l!egal_F@L#(N)ame.html.pdf")},
+      {"example.com", FPL("example.com.pdf")},
+      {"data:text/html,foo", FPL("data_text_html,foo.pdf")},
       {"Baz.com Mail - this is e-mail - what. does it mean",
-       FILE_PATH_LITERAL("Baz.com Mail - this is e-mail - what.pdf")},
+       FPL("Baz.com Mail - this is e-mail - what. does it mean.pdf")},
       {"Baz.com Mail - this is email - what. does. it. mean?",
-       FILE_PATH_LITERAL("Baz.com Mail - this is email - what. does. it.pdf")},
+       FPL("Baz.com Mail - this is email - what. does. it. mean_.pdf")},
       {"Baz.com Mail - This is email. What does it mean.",
-       FILE_PATH_LITERAL("Baz.com Mail - This is email.pdf")},
+       FPL("Baz.com Mail - This is email. What does it mean_.pdf")},
       {"Baz.com Mail - this is email what does it mean",
-       FILE_PATH_LITERAL("Baz.pdf")},
+       FPL("Baz.com Mail - this is email what does it mean.pdf")},
   };
 
   for (const auto& data : kTestData) {
@@ -41,3 +43,24 @@ TEST_F(PdfPrinterHandlerTest, GetFileNameForPrintJobTitle) {
     EXPECT_EQ(data.expected_output, path.value());
   }
 }
+
+TEST_F(PdfPrinterHandlerTest, GetFileNameForPrintJobURL) {
+  static const struct {
+    const char* input;
+    const base::FilePath::CharType* expected_output;
+  } kTestData[] = {
+      {"http://example.com", FPL("example.com.pdf")},
+      {"http://example.com/?foo", FPL("example.com.pdf")},
+      {"https://example.com/foo.html", FPL("foo.pdf")},
+      {"https://example.com/bar/qux.txt", FPL("qux.pdf")},
+      {"https://example.com/bar/qux.pdf", FPL("qux.pdf")},
+      {"data:text/html,foo", FPL("dataurl.pdf")},
+  };
+
+  for (const auto& data : kTestData) {
+    SCOPED_TRACE(data.input);
+    base::FilePath path =
+        PdfPrinterHandler::GetFileNameForURL(GURL(data.input));
+    EXPECT_EQ(data.expected_output, path.value());
+  }
+}

chrome/browser/ui/webui/print_preview/pdf_printer_handler_win_unittest.cc

@@ -65,6 +65,7 @@ class FakePdfPrinterHandler : public PdfPrinterHandler {
   // Simplified version of select file to avoid checking preferences and sticky
   // settings in the test
   void SelectFile(const base::FilePath& default_filename,
+                  content::WebContents* /* initiator */,
                   bool prompt_user) override {
     ui::SelectFileDialog::FileTypeInfo file_type_info;
     file_type_info.extensions.resize(1);