This CL makes Chrome on par with Firefox in terms of 'GetSuggestedFilename' for file download via context-menu.

For a download initiated with a click on a link in a web page, a webkit-side change is necessary, which will be done later.

Add a field (referrer_charset) to URLRequestContext and DownloadCreateInfo. It's set to the character encoding of a document where the download request originates from when it's known (download initiated via "save as" in the context menu). 

If it's not known (a download initiated by clicking on a download link or typing a url directly to the omnibox), it's initialized to the default character encoding in the user's preference. I guess this is marginally better than leaving it empty (in that case, step 2b below will be skipped and step 2c will be taken) because a user has a better control over how raw 8bit characters in C-D are interpreted (especially on Windows where a reboot is required to change the OS default codepage). 

This is later passed to GetSuggestedFilename and used as one of fallback encodings (1. UTF-8, 2. origin_charset, 3. default OS codepage). With this change, we support the following:

1. RFC 2047
2. Raw-8bit-characters : a. UTF-8, b. origin_charset, c. default os codepage. 
3. %-escaped UTF-8. 

In this CL, for #3, I didn't add a fallback similar to one used for #2. If necessary, it can be added easily. New entries are added to 3 existing tests. What's previously not covered (raw 8bit Content-Disposition header) is now covered in all 3 tests. 

BUG=1148
TEST=net unit test: NetUtilTest.GetFileNameFromCD                    
                    NetUtilTest.GetSuggestedFilename         
     unittest : DownloadManagerTest.TestDownloadFilename

Review URL: http://codereview.chromium.org/83002

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

14 files changed, 63 insertions, 10 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 5a5649c..0fc444f 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -1085,8 +1085,10 @@ int DownloadManager::RemoveAllDownloads() {
 // download.
 void DownloadManager::DownloadUrl(const GURL& url,
                                   const GURL& referrer,
+                                  const std::string& referrer_charset,
                                   WebContents* web_contents) {
   DCHECK(web_contents);
+  request_context_->set_referrer_charset(referrer_charset);
   file_manager_->DownloadUrl(url,
                              referrer,
                              web_contents->process()->pid(),
@@ -1179,6 +1181,7 @@ void DownloadManager::GenerateFilename(DownloadCreateInfo* info,
   *generated_name = FilePath::FromWStringHack(
       net::GetSuggestedFilename(GURL(info->url),
                                 info->content_disposition,
+                                info->referrer_charset,
                                 L"download"));
   DCHECK(!generated_name->empty());
 
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 5cb22a4..50f4fa3 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -356,6 +356,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // Download the object at the URL. Used in cases such as "Save Link As..."
   void DownloadUrl(const GURL& url,
                    const GURL& referrer,
+                   const std::string& referrer_encoding,
                    WebContents* web_contents);
 
   // Allow objects to observe the download creation process.
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index f1fea31..01f4839 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -18,11 +18,13 @@ class DownloadManagerTest : public testing::Test {
   void GetGeneratedFilename(const std::string& content_disposition,
                             const std::string& url,
                             const std::string& mime_type,
+                            const std::string& referrer_charset,
                             std::wstring* generated_name_string) {
     DownloadCreateInfo info;
     info.content_disposition = content_disposition;
     info.url = GURL(url);
     info.mime_type = mime_type;
+    info.referrer_charset = referrer_charset;
     FilePath generated_name;
     download_manager_->GenerateFilename(&info, &generated_name);
     *generated_name_string = generated_name.ToWStringHack();
@@ -335,7 +337,6 @@ const struct {
    "application/x-tar",
    L"bar.bogus.tar"},
 
-  // TODO(darin): Add some raw 8-bit Content-Disposition tests.
 };
 
 }  // namespace
@@ -343,14 +344,35 @@ const struct {
 // Tests to ensure that the file names we generate from hints from the server
 // (content-disposition, URL name, etc) don't cause security holes.
 TEST_F(DownloadManagerTest, TestDownloadFilename) {
+  std::wstring file_name;
   for (int i = 0; i < arraysize(kGeneratedFiles); ++i) {
-    std::wstring file_name;
     GetGeneratedFilename(kGeneratedFiles[i].disposition,
                          kGeneratedFiles[i].url,
                          kGeneratedFiles[i].mime_type,
+                         "",
+                         &file_name);
+    EXPECT_EQ(kGeneratedFiles[i].expected_name, file_name);
+    GetGeneratedFilename(kGeneratedFiles[i].disposition,
+                         kGeneratedFiles[i].url,
+                         kGeneratedFiles[i].mime_type,
+                         "GBK",
                          &file_name);
     EXPECT_EQ(kGeneratedFiles[i].expected_name, file_name);
   }
+
+  // A couple of cases with raw 8bit characters in C-D.
+  GetGeneratedFilename("attachment; filename=caf\xc3\xa9.png",
+                       "http://www.example.com/images?id=3",
+                       "image/png",
+                       "iso-8859-1",
+                       &file_name);
+  EXPECT_EQ(L"caf\u00e9.png", file_name);
+  GetGeneratedFilename("attachment; filename=caf\xe5.png",
+                       "http://www.example.com/images?id=3",
+                       "image/png",
+                       "windows-1253",
+                       &file_name);
+  EXPECT_EQ(L"caf\u03b5.png", file_name);
 }
 
 namespace {
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 3fc1738..19d7df6 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -296,8 +296,10 @@ bool SavePackage::GenerateFilename(const std::string& disposition,
                                    const GURL& url,
                                    bool need_html_ext,
                                    FilePath::StringType* generated_name) {
+  // TODO(jungshik): Figure out the referrer charset when having one
+  // makes sense and pass it to GetSuggestedFilename.
   FilePath file_path = FilePath::FromWStringHack(
-      net::GetSuggestedFilename(url, disposition, kDefaultSaveName));
+      net::GetSuggestedFilename(url, disposition, "", kDefaultSaveName));
 
   DCHECK(!file_path.empty());
   FilePath::StringType pure_file_name =
diff --git a/chrome/browser/history/download_types.h b/chrome/browser/history/download_types.h
index 477a249..25e039a 100644
--- a/chrome/browser/history/download_types.h
+++ b/chrome/browser/history/download_types.h
@@ -69,6 +69,9 @@ struct DownloadCreateInfo {
   bool is_dangerous;
   // The original name for a dangerous download.
   FilePath original_name;
+  // The charset of the referring page where the download request comes from.
+  // It's used to construct a suggested filename.
+  std::string referrer_charset;
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_TYPES_H_
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index ac5c3c3..b15db26 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -198,6 +198,20 @@ ChromeURLRequestContext::ChromeURLRequestContext(Profile* profile)
   accept_charset_ = net::HttpUtil::GenerateAcceptCharsetHeader(
       WideToASCII(prefs_->GetString(prefs::kDefaultCharset)));
 
+  // At this point, we don't know the charset of the referring page
+  // where a url request originates from. This is used to get a suggested
+  // filename from Content-Disposition header made of raw 8bit characters.
+  // Down the road, it can be overriden if it becomes known (for instance,
+  // when download request is made through the context menu in a web page).
+  // At the moment, it'll remain 'undeterministic' when a user
+  // types a URL in the omnibar or click on a download link in a page.
+  // For the latter, we need a change on the webkit-side.
+  // We initialize it to the default charset here and a user will
+  // have an *arguably* better default charset for interpreting a raw 8bit
+  // C-D header field.  It means the native OS codepage fallback in
+  // net_util::GetSuggestedFilename is unlikely to be taken.
+  referrer_charset_ = accept_charset_;
+
   cookie_policy_.SetType(net::CookiePolicy::FromInt(
       prefs_->GetInteger(prefs::kCookieBehavior)));
 
diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc
index 9824d97..b100df3 100644
--- a/chrome/browser/renderer_host/download_resource_handler.cc
+++ b/chrome/browser/renderer_host/download_resource_handler.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "net/base/io_buffer.h"
+#include "net/url_request/url_request_context.h"
 
 DownloadResourceHandler::DownloadResourceHandler(ResourceDispatcherHost* rdh,
                                                  int render_process_host_id,
@@ -64,6 +65,7 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id,
   info->mime_type = response->response_head.mime_type;
   info->save_as = save_as_;
   info->is_dangerous = false;
+  info->referrer_charset = request_->context()->referrer_charset();
   download_manager_->file_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(download_manager_,
                         &DownloadFileManager::StartDownload,
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index 6d45a73..c77d8d5 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -375,7 +375,8 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) {
                                                   params_.image_url);
       DownloadManager* dlm =
           source_web_contents_->profile()->GetDownloadManager();
-      dlm->DownloadUrl(url, referrer, source_web_contents_);
+      dlm->DownloadUrl(url, referrer, params_.frame_charset,
+                       source_web_contents_);
       break;
     }
 
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index a835423..37b370c 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1091,7 +1091,7 @@ void TabContents::OnSavePage() {
     DownloadManager* dlm = profile()->GetDownloadManager();
     const GURL& current_page_url = GetURL();
     if (dlm && current_page_url.is_valid())
-      dlm->DownloadUrl(current_page_url, GURL(), AsWC(this));
+      dlm->DownloadUrl(current_page_url, GURL(), "", AsWC(this));
     return;
   }
 
diff --git a/chrome/browser/tab_contents/tab_contents_view_win.cc b/chrome/browser/tab_contents/tab_contents_view_win.cc
index 5f47164..6fee9c3 100644
--- a/chrome/browser/tab_contents/tab_contents_view_win.cc
+++ b/chrome/browser/tab_contents/tab_contents_view_win.cc
@@ -133,7 +133,7 @@ void TabContentsViewWin::StartDragging(const WebDropData& drop_data) {
     if (file_name.value().empty()) {
       // Retrieve the name from the URL.
       file_name = FilePath::FromWStringHack(
-          net::GetSuggestedFilename(drop_data.url, L"", L""));
+          net::GetSuggestedFilename(drop_data.url, "", "", L""));
     }
     file_name = file_name.ReplaceExtension(drop_data.file_extension);
     data->SetFileContents(file_name.value(), drop_data.file_contents);
diff --git a/chrome/common/os_exchange_data.cc b/chrome/common/os_exchange_data.cc
index 2b131f2..fbedcb8 100644
--- a/chrome/common/os_exchange_data.cc
+++ b/chrome/common/os_exchange_data.cc
@@ -668,7 +668,7 @@ static void CreateValidFileNameFromTitle(const GURL& url,
   if (title.empty()) {
     if (url.is_valid()) {
       *validated = net::GetSuggestedFilename(
-          url, std::wstring(), std::wstring());
+          url, std::string(), std::string(), std::wstring());
     } else {
       // Nothing else can be done, just use a default.
       *validated = l10n_util::GetString(IDS_UNTITLED_SHORTCUT_FILE_NAME);
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index c689365..e187b7d 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -889,6 +889,7 @@ struct ParamTraits<ContextMenuParams> {
     WriteParam(m, p.spellcheck_enabled);
     WriteParam(m, p.edit_flags);
     WriteParam(m, p.security_info);
+    WriteParam(m, p.frame_charset);
   }
   static bool Read(const Message* m, void** iter, param_type* p) {
     return
@@ -905,7 +906,8 @@ struct ParamTraits<ContextMenuParams> {
       ReadParam(m, iter, &p->dictionary_suggestions) &&
       ReadParam(m, iter, &p->spellcheck_enabled) &&
       ReadParam(m, iter, &p->edit_flags) &&
-      ReadParam(m, iter, &p->security_info);
+      ReadParam(m, iter, &p->security_info) &&
+      ReadParam(m, iter, &p->frame_charset);
   }
   static void Log(const param_type& p, std::wstring* l) {
     l->append(L"<ContextMenuParams>");
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index a8a8c41..1236886 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1994,7 +1994,8 @@ void RenderView::ShowContextMenu(WebView* webview,
                                  const std::wstring& selection_text,
                                  const std::wstring& misspelled_word,
                                  int edit_flags,
-                                 const std::string& security_info) {
+                                 const std::string& security_info,
+                                 const std::string& frame_charset) {
   ContextMenuParams params;
   params.node = node;
   params.x = x;
@@ -2010,6 +2011,7 @@ void RenderView::ShowContextMenu(WebView* webview,
       webview->GetFocusedFrame()->SpellCheckEnabled();
   params.edit_flags = edit_flags;
   params.security_info = security_info;
+  params.frame_charset = frame_charset;
   Send(new ViewHostMsg_ContextMenu(routing_id_, params));
 }
 
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index b3d0f10..fa88cb1 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -266,7 +266,8 @@ class RenderView : public RenderWidget,
                                const std::wstring& selection_text,
                                const std::wstring& misspelled_word,
                                int edit_flags,
-                               const std::string& security_info);
+                               const std::string& security_info,
+                               const std::string& frame_charset);
   virtual void StartDragging(WebView* webview,
                              const WebKit::WebDragData& drag_data);