Fix bug 79905: Drag and drop of "DownloadURL" type ignores specified filename for data URLs.

BUG=79905
TEST=Manual test and unittest added for net::GetSuggestedFilename
Review URL: http://codereview.chromium.org/7005011

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

12 files changed, 164 insertions, 87 deletions

diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 22a2d21..b207dba 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -439,7 +439,7 @@ bool CanPasteFromClipboard(const BookmarkNode* node) {
 
 string16 GetNameForURL(const GURL& url) {
   if (url.is_valid()) {
-    return net::GetSuggestedFilename(url, "", "", string16());
+    return net::GetSuggestedFilename(url, "", "", "", string16());
   } else {
     return l10n_util::GetStringUTF16(IDS_APP_UNTITLED_SHORTCUT_FILE_NAME);
   }
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index f68d676..326d831 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -167,6 +167,38 @@ bool IsReservedName(const string16& filename) {
 }
 #endif  // OS_WIN
 
+void GenerateFileNameInternal(const GURL& url,
+                              const std::string& content_disposition,
+                              const std::string& referrer_charset,
+                              const std::string& suggested_name,
+                              const std::string& mime_type,
+                              FilePath* generated_name) {
+
+  string16 default_file_name(
+      l10n_util::GetStringUTF16(IDS_DEFAULT_DOWNLOAD_FILENAME));
+
+  string16 new_name = net::GetSuggestedFilename(GURL(url),
+                                                content_disposition,
+                                                referrer_charset,
+                                                suggested_name,
+                                                default_file_name);
+
+  // TODO(evan): this code is totally wrong -- we should just generate
+  // Unicode filenames and do all this encoding switching at the end.
+  // However, I'm just shuffling wrong code around, at least not adding
+  // to it.
+#if defined(OS_WIN)
+  *generated_name = FilePath(new_name);
+#else
+  *generated_name = FilePath(
+      base::SysWideToNativeMB(UTF16ToWide(new_name)));
+#endif
+
+  DCHECK(!generated_name->empty());
+
+  GenerateSafeFileName(mime_type, generated_name);
+}
+
 }  // namespace
 
 // Download temporary file creation --------------------------------------------
@@ -251,11 +283,17 @@ void GenerateExtension(const FilePath& file_name,
 
 void GenerateFileNameFromInfo(DownloadCreateInfo* info,
                               FilePath* generated_name) {
-  GenerateFileName(GURL(info->url()),
-                   info->content_disposition,
-                   info->referrer_charset,
-                   info->mime_type,
-                   generated_name);
+  GenerateFileNameInternal(GURL(info->url()), info->content_disposition,
+                           info->referrer_charset, std::string(),
+                           info->mime_type, generated_name);
+}
+
+void GenerateFileNameFromSuggestedName(const GURL& url,
+                                       const std::string& suggested_name,
+                                       const std::string& mime_type,
+                                       FilePath* generated_name) {
+  GenerateFileNameInternal(url, std::string(), std::string(),
+                           suggested_name, mime_type, generated_name);
 }
 
 void GenerateFileName(const GURL& url,
@@ -263,28 +301,8 @@ void GenerateFileName(const GURL& url,
                       const std::string& referrer_charset,
                       const std::string& mime_type,
                       FilePath* generated_name) {
-  string16 default_file_name(
-      l10n_util::GetStringUTF16(IDS_DEFAULT_DOWNLOAD_FILENAME));
-
-  string16 new_name = net::GetSuggestedFilename(GURL(url),
-                                                content_disposition,
-                                                referrer_charset,
-                                                default_file_name);
-
-  // TODO(evan): this code is totally wrong -- we should just generate
-  // Unicode filenames and do all this encoding switching at the end.
-  // However, I'm just shuffling wrong code around, at least not adding
-  // to it.
-#if defined(OS_WIN)
-  *generated_name = FilePath(new_name);
-#else
-  *generated_name = FilePath(
-      base::SysWideToNativeMB(UTF16ToWide(new_name)));
-#endif
-
-  DCHECK(!generated_name->empty());
-
-  GenerateSafeFileName(mime_type, generated_name);
+  GenerateFileNameInternal(url, content_disposition, referrer_charset,
+                           std::string(), mime_type, generated_name);
 }
 
 void GenerateSafeFileName(const std::string& mime_type, FilePath* file_name) {
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index 376fc18..707dc07 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -64,7 +64,11 @@ void GenerateExtension(const FilePath& file_name,
 void GenerateFileNameFromInfo(DownloadCreateInfo* info,
                               FilePath* generated_name);
 
-// Create a file name based on the response from the server.
+void GenerateFileNameFromSuggestedName(const GURL& url,
+                                       const std::string& suggested_name,
+                                       const std::string& mime_type,
+                                       FilePath* generated_name);
+
 void GenerateFileName(const GURL& url,
                       const std::string& content_disposition,
                       const std::string& referrer_charset,
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index d18099c..fee6067 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -419,7 +419,7 @@ bool SavePackage::GenerateFileName(const std::string& disposition,
   // TODO(jungshik): Figure out the referrer charset when having one
   // makes sense and pass it to GetSuggestedFilename.
   string16 suggested_name =
-      net::GetSuggestedFilename(url, disposition, "",
+      net::GetSuggestedFilename(url, disposition, "", "",
                                 ASCIIToUTF16(kDefaultSaveName));
 
   // TODO(evan): this code is totally wrong -- we should just generate
diff --git a/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm b/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
index 24ef3b4..b3283b5 100644
--- a/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
+++ b/chrome/browser/ui/cocoa/tab_contents/web_drag_source.mm
@@ -69,7 +69,7 @@ FilePath GetFileNameFromDragData(const WebDropData& drop_data) {
   if (file_name.empty()) {
     // Retrieve the name from the URL.
     string16 suggested_filename =
-        net::GetSuggestedFilename(drop_data.url, "", "", string16());
+        net::GetSuggestedFilename(drop_data.url, "", "", "", string16());
     file_name = FilePathFromFilename(suggested_filename);
   }
 
@@ -402,13 +402,11 @@ void PromiseWriterTask::Run() {
               &mimeType,
               &fileName,
               &downloadURL_)) {
-        std::string contentDisposition =
-            "attachment; filename=" + fileName.value();
-        download_util::GenerateFileName(downloadURL_,
-                                        contentDisposition,
-                                        std::string(),
-                                        UTF16ToUTF8(mimeType),
-                                        &downloadFileName_);
+        download_util::GenerateFileNameFromSuggestedName(
+            downloadURL_,
+            fileName.value(),
+            UTF16ToUTF8(mimeType),
+            &downloadFileName_);
         fileExtension = SysUTF8ToNSString(downloadFileName_.Extension());
       }
     }
diff --git a/chrome/browser/ui/gtk/tab_contents_drag_source.cc b/chrome/browser/ui/gtk/tab_contents_drag_source.cc
index 4ae83bb..1da585e 100644
--- a/chrome/browser/ui/gtk/tab_contents_drag_source.cc
+++ b/chrome/browser/ui/gtk/tab_contents_drag_source.cc
@@ -291,15 +291,12 @@ void TabContentsDragSource::OnDragBegin(GtkWidget* sender,
                                         GdkDragContext* drag_context) {
   if (!download_url_.is_empty()) {
     // Generate the file name based on both mime type and proposed file name.
-    std::string download_mime_type = UTF16ToUTF8(wide_download_mime_type_);
-    std::string content_disposition("attachment; filename=");
-    content_disposition += download_file_name_.value();
     FilePath generated_download_file_name;
-    download_util::GenerateFileName(download_url_,
-                                    content_disposition,
-                                    std::string(),
-                                    download_mime_type,
-                                    &generated_download_file_name);
+    download_util::GenerateFileNameFromSuggestedName(
+        download_url_,
+        download_file_name_.value(),
+        UTF16ToUTF8(wide_download_mime_type_),
+        &generated_download_file_name);
 
     // Pass the file name to the drop target by setting the source window's
     // XdndDirectSave0 property.
diff --git a/chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc b/chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc
index 1a45e4d..71480b2 100644
--- a/chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc
+++ b/chrome/browser/ui/views/tab_contents/tab_contents_drag_win.cc
@@ -196,14 +196,12 @@ void TabContentsDragWin::PrepareDragForDownload(
     return;
 
   // Generate the download filename.
-  std::string content_disposition =
-      "attachment; filename=" + UTF16ToUTF8(file_name.value());
   FilePath generated_file_name;
-  download_util::GenerateFileName(download_url,
-                                  content_disposition,
-                                  std::string(),
-                                  UTF16ToUTF8(mime_type),
-                                  &generated_file_name);
+  download_util::GenerateFileNameFromSuggestedName(
+      download_url,
+      UTF16ToUTF8(file_name.value()),
+      UTF16ToUTF8(mime_type),
+      &generated_file_name);
 
   // Provide the data as file (CF_HDROP). A temporary download file with the
   // Zone.Identifier ADS (Alternate Data Stream) attached will be created.
@@ -232,7 +230,7 @@ void TabContentsDragWin::PrepareDragForFileContents(
   if (file_name.value().empty()) {
     // Retrieve the name from the URL.
     file_name = FilePath(
-        net::GetSuggestedFilename(drop_data.url, "", "", string16()));
+        net::GetSuggestedFilename(drop_data.url, "", "", "", string16()));
     if (file_name.value().size() + drop_data.file_extension.size() + 1 >
         MAX_PATH) {
       file_name = FilePath(file_name.value().substr(
diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 2e77acf..31d7497 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -1248,6 +1248,7 @@ string16 StripWWW(const string16& text) {
 string16 GetSuggestedFilename(const GURL& url,
                               const std::string& content_disposition,
                               const std::string& referrer_charset,
+                              const std::string& suggested_name,
                               const string16& default_name) {
   // TODO: this function to be updated to match the httpbis recommendations.
   // Talk to abarth for the latest news.
@@ -1256,16 +1257,15 @@ string16 GetSuggestedFilename(const GURL& url,
   // needed, the caller should provide localized fallback default_name.
   static const char* kFinalFallbackName = "download";
 
-  // about: and data: URLs don't have file names, but esp. data: URLs may
-  // contain parts that look like ones (i.e., contain a slash).
-  // Therefore we don't attempt to divine a file name out of them.
-  if (url.SchemeIs("about") || url.SchemeIs("data")) {
-    return default_name.empty() ? ASCIIToUTF16(kFinalFallbackName)
-                                : default_name;
-  }
+  std::string filename;
+
+  // Try to extract from content-disposition first.
+  if (!content_disposition.empty())
+    filename = GetFileNameFromCD(content_disposition, referrer_charset);
 
-  std::string filename = GetFileNameFromCD(content_disposition,
-                                           referrer_charset);
+  // Then try to use suggested name.
+  if (filename.empty() && !suggested_name.empty())
+    filename = suggested_name;
 
   if (!filename.empty()) {
     // Replace any path information the server may have sent, by changing
@@ -1277,7 +1277,16 @@ string16 GetSuggestedFilename(const GURL& url,
     // tricks with hidden files, "..", and "."
     TrimString(filename, ".", &filename);
   }
+
   if (filename.empty()) {
+    // about: and data: URLs don't have file names, but esp. data: URLs may
+    // contain parts that look like ones (i.e., contain a slash).
+    // Therefore we don't attempt to divine a file name out of them.
+    if (url.SchemeIs("about") || url.SchemeIs("data")) {
+      return default_name.empty() ? ASCIIToUTF16(kFinalFallbackName)
+                                  : default_name;
+    }
+
     if (url.is_valid()) {
       const std::string unescaped_url_filename = UnescapeURLComponent(
           url.ExtractFileName(),
diff --git a/net/base/net_util.h b/net/base/net_util.h
index 9610932..fef75ba 100644
--- a/net/base/net_util.h
+++ b/net/base/net_util.h
@@ -245,18 +245,19 @@ NET_API std::string GetDirectoryListingEntry(const string16& name,
 // unmodified.
 NET_API string16 StripWWW(const string16& text);
 
-// Gets the filename from the raw Content-Disposition header (as read from the
-// network).  Otherwise uses the last path component name or hostname from
-// |url|. If there is no filename or it can't be used, the given |default_name|,
-// will be used unless it is empty.
-
-// Note: it's possible for the suggested filename to be empty (e.g.,
-// file:///). referrer_charset is used as one of charsets
-// to interpret a raw 8bit string in C-D header (after interpreting
-// as UTF-8 fails). See the comment for GetFilenameFromCD for more details.
+// Gets the filename in the following order:
+// 1) the raw Content-Disposition header (as read from the network).
+//    |referrer_charset| is used as one of charsets to interpret a raw 8bit
+//    string in C-D header (after interpreting as UTF-8 fails).
+//    See the comment for GetFilenameFromCD for more details.
+// 2) the suggested name
+// 3) the last path component name or hostname from |url|
+// 4) the given |default_name|
+// 5) the hard-coded name "download", as the last resort
 NET_API string16 GetSuggestedFilename(const GURL& url,
                                       const std::string& content_disposition,
                                       const std::string& referrer_charset,
+                                      const std::string& suggested_name,
                                       const string16& default_name);
 
 // Checks the given port against a list of ports which are restricted by
diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc
index ac0cdd9..a8dd92e 100644
--- a/net/base/net_util_unittest.cc
+++ b/net/base/net_util_unittest.cc
@@ -386,6 +386,7 @@ struct SuggestedFilenameCase {
   const char* url;
   const char* content_disp_header;
   const char* referrer_charset;
+  const char* suggested_filename;
   const wchar_t* default_filename;
   const wchar_t* expected_filename;
 };
@@ -1048,46 +1049,55 @@ TEST(NetUtilTest, GetSuggestedFilename) {
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=test.html",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=\"test.html\"",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename= \"test.html\"",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename   =   \"test.html\"",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=  ",
      "",
+     "",
      L"",
      L"www.google.com"},
     {"http://www.google.com/path/test.html",
      "Content-disposition: attachment",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/path/test.html",
      "Content-disposition: attachment;",
      "",
+     "",
      L"",
      L"test.html"},
     {"http://www.google.com/",
      "",
      "",
+     "",
      L"",
      L"www.google.com"},
     {"http://www.google.com/test.html",
      "",
      "",
+     "",
      L"",
      L"test.html"},
     // Now that we use googleurl's ExtractFileName, this case falls back
@@ -1096,51 +1106,61 @@ TEST(NetUtilTest, GetSuggestedFilename) {
     {"http://www.google.com/path/",
      "",
      "",
+     "",
      L"",
      L"www.google.com"},
     {"http://www.google.com/path",
      "",
      "",
+     "",
      L"",
      L"path"},
     {"file:///",
      "",
      "",
+     "",
      L"",
      L"download"},
     {"non-standard-scheme:",
      "",
      "",
+     "",
      L"",
      L"download"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename =\"test.html\"",
      "",
+     "",
      L"download",
      L"test.html"},
     {"http://www.google.com/",
      "",
      "",
+     "",
      L"download",
      L"download"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=\"../test.html\"",
      "",
+     "",
      L"",
      L"_test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=\"..\\test.html\"",
      "",
+     "",
      L"",
      L"_test.html"},
     {"http://www.google.com/",
      "Content-disposition: attachment; filename=\"..\"",
      "",
+     "",
      L"download",
      L"download"},
     {"http://www.google.com/test.html",
      "Content-disposition: attachment; filename=\"..\"",
      "",
+     "",
      L"download",
      L"test.html"},
     // Below is a small subset of cases taken from GetFileNameFromCD test above.
@@ -1148,93 +1168,123 @@ TEST(NetUtilTest, GetSuggestedFilename) {
      "Content-Disposition: attachment; filename=\"%EC%98%88%EC%88%A0%20"
      "%EC%98%88%EC%88%A0.jpg\"",
      "",
+     "",
      L"",
      L"\uc608\uc220 \uc608\uc220.jpg"},
     {"http://www.google.com/%EC%98%88%EC%88%A0%20%EC%98%88%EC%88%A0.jpg",
      "",
      "",
+     "",
      L"download",
      L"\uc608\uc220 \uc608\uc220.jpg"},
     {"http://www.google.com/",
      "Content-disposition: attachment;",
      "",
+     "",
      L"\uB2E4\uC6B4\uB85C\uB4DC",
      L"\uB2E4\uC6B4\uB85C\uB4DC"},
     {"http://www.google.com/",
      "Content-Disposition: attachment; filename=\"=?EUC-JP?Q?=B7=DD=BD="
      "D13=2Epng?=\"",
      "",
+     "",
      L"download",
      L"\u82b8\u88533.png"},
     {"http://www.example.com/images?id=3",
      "Content-Disposition: attachment; filename=caf\xc3\xa9.png",
      "iso-8859-1",
+     "",
      L"",
      L"caf\u00e9.png"},
     {"http://www.example.com/images?id=3",
      "Content-Disposition: attachment; filename=caf\xe5.png",
      "windows-1253",
+     "",
      L"",
      L"caf\u03b5.png"},
     {"http://www.example.com/file?id=3",
      "Content-Disposition: attachment; name=\xcf\xc2\xd4\xd8.zip",
      "GBK",
+     "",
      L"",
      L"\u4e0b\u8f7d.zip"},
     // Invalid C-D header. Extracts filename from url.
     {"http://www.google.com/test.html",
      "Content-Disposition: attachment; filename==?iiso88591?Q?caf=EG?=",
      "",
+     "",
      L"",
      L"test.html"},
     // about: and data: URLs
     {"about:chrome",
      "",
      "",
+     "",
      L"",
      L"download"},
     {"data:,looks/like/a.path",
      "",
      "",
+     "",
      L"",
      L"download"},
     {"data:text/plain;base64,VG8gYmUgb3Igbm90IHRvIGJlLg=",
      "",
      "",
+     "",
      L"",
      L"download"},
     {"data:,looks/like/a.path",
      "",
      "",
+     "",
      L"default_filename_is_given",
      L"default_filename_is_given"},
     {"data:,looks/like/a.path",
      "",
      "",
+     "",
      L"\u65e5\u672c\u8a9e",  // Japanese Kanji.
      L"\u65e5\u672c\u8a9e"},
     // Dotfiles. Ensures preceeding period(s) stripped.
     {"http://www.google.com/.test.html",
-    "",
-    "",
-    L"",
-    L"test.html"},
+     "",
+     "",
+     "",
+     L"",
+     L"test.html"},
     {"http://www.google.com/.test",
-    "",
-    "",
-    L"",
-    L"test"},
+     "",
+     "",
+     "",
+     L"",
+     L"test"},
     {"http://www.google.com/..test",
-    "",
-    "",
-    L"",
-    L"test"},
+     "",
+     "",
+     "",
+     L"",
+     L"test"},
     // The filename encoding is specified by the referrer charset.
     {"http://example.com/V%FDvojov%E1%20psychologie.doc",
      "",
      "iso-8859-1",
+     "",
      L"",
      L"V\u00fdvojov\u00e1 psychologie.doc"},
+    {"http://www.google.com/test",
+     "",
+     "",
+     "suggested",
+     L"",
+     L"suggested"},
+    // The content-disposition has higher precedence over the suggested name.
+    {"http://www.google.com/test",
+     "Content-disposition: attachment; filename=test.html",
+     "",
+     "suggested",
+     L"",
+     L"test.html"},
     // The filename encoding doesn't match the referrer charset, the
     // system charset, or UTF-8.
     // TODO(jshin): we need to handle this case.
@@ -1242,6 +1292,7 @@ TEST(NetUtilTest, GetSuggestedFilename) {
     {"http://example.com/V%FDvojov%E1%20psychologie.doc",
      "",
      "utf-8",
+     "",
      L"",
      L"V\u00fdvojov\u00e1 psychologie.doc",
     },
@@ -1251,7 +1302,8 @@ TEST(NetUtilTest, GetSuggestedFilename) {
     std::wstring default_name = test_cases[i].default_filename;
     string16 filename = GetSuggestedFilename(
         GURL(test_cases[i].url), test_cases[i].content_disp_header,
-        test_cases[i].referrer_charset, WideToUTF16(default_name));
+        test_cases[i].referrer_charset, test_cases[i].suggested_filename,
+        WideToUTF16(default_name));
     EXPECT_EQ(std::wstring(test_cases[i].expected_filename),
               UTF16ToWide(filename))
       << "Iteration " << i << ": " << test_cases[i].url;
diff --git a/ui/base/dragdrop/os_exchange_data_provider_win.cc b/ui/base/dragdrop/os_exchange_data_provider_win.cc
index 6712d7d..3a4b407 100644
--- a/ui/base/dragdrop/os_exchange_data_provider_win.cc
+++ b/ui/base/dragdrop/os_exchange_data_provider_win.cc
@@ -848,7 +848,7 @@ static void CreateValidFileNameFromTitle(const GURL& url,
                                          string16* validated) {
   if (title.empty()) {
     if (url.is_valid()) {
-      *validated = net::GetSuggestedFilename(url, "", "", string16());
+      *validated = net::GetSuggestedFilename(url, "", "", "", string16());
     } else {
       // Nothing else can be done, just use a default.
       *validated =
diff --git a/webkit/glue/weburlloader_impl.cc b/webkit/glue/weburlloader_impl.cc
index 9105157..0a874d0 100644
--- a/webkit/glue/weburlloader_impl.cc
+++ b/webkit/glue/weburlloader_impl.cc
@@ -259,7 +259,7 @@ void PopulateURLResponse(
   std::string value;
   if (headers->EnumerateHeader(NULL, "content-disposition", &value)) {
     response->setSuggestedFileName(
-        net::GetSuggestedFilename(url, value, "", string16()));
+        net::GetSuggestedFilename(url, value, "", "", string16()));
   }
 
   Time time_val;