Do some cleanup of file path name handling.

This started trying to cleanup DownloadManager::GenerateFilename which asserts
if your system locale isn't UTF-8 (I ran into this when mine got messed up).
The solution is to have GetSuggestedFilename return a FilePath rather than
calling FromWStringHack.

The rest of the patch is a result of trying to write GetSuggestedFilename in a
reasonable way. I changed ReplaceIllegalCharacters to work on a
FilePath::StringType.

Some places in the code calling these functions got cleaner, some got messier.
I think overall the ones that got messier are the ones doing sketchy things
with paths and the ones that got cleaner are the ones doing things more
properly.

The only code here that gets called a nontrivial number of times is the
weburlloader, and I think the new code does about the same number of string
conversions overall (though on certain platforms the number will be higher or
lower).

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/271056

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

3 files changed, 98 insertions, 66 deletions

diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index f75184f..49ded14 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -749,6 +749,18 @@ std::wstring FormatViewSourceUrl(const GURL& url,
   return result;
 }
 
+// Converts a UTF-8 string to a FilePath string type.
+//
+// This is inline with the hope that the function will be "free" on non-Windows
+// platforms.
+inline FilePath::StringType UTF8ToFilePathString(const std::string& utf8) {
+#if defined(OS_WIN)
+  return FilePath::StringType(UTF8ToUTF16(utf8));
+#else
+  return utf8;
+#endif
+}
+
 }  // namespace
 
 namespace net {
@@ -805,19 +817,19 @@ std::string GetSpecificHeader(const std::string& headers,
   return GetSpecificHeaderT(headers, name);
 }
 
-std::wstring GetFileNameFromCD(const std::string& header,
-                               const std::string& referrer_charset) {
+std::string GetFileNameFromCD(const std::string& header,
+                              const std::string& referrer_charset) {
   std::string param_value = GetHeaderParamValue(header, "filename");
   if (param_value.empty()) {
     // Some servers use 'name' parameter.
     param_value = GetHeaderParamValue(header, "name");
   }
   if (param_value.empty())
-    return std::wstring();
+    return std::string();
   std::string decoded;
   if (DecodeParamValue(param_value, referrer_charset, &decoded))
-    return UTF8ToWide(decoded);
-  return std::wstring();
+    return decoded;
+  return std::string();
 }
 
 std::wstring GetHeaderParamValue(const std::wstring& field,
@@ -1032,60 +1044,72 @@ std::wstring StripWWW(const std::wstring& text) {
       text.substr(www.length()) : text;
 }
 
-std::wstring GetSuggestedFilename(const GURL& url,
-                                  const std::string& content_disposition,
-                                  const std::string& referrer_charset,
-                                  const std::wstring& default_name) {
+FilePath GetSuggestedFilename(const GURL& url,
+                              const std::string& content_disposition,
+                              const std::string& referrer_charset,
+                              const char* default_name) {
   // TODO(rolandsteiner): as pointed out by darin in the code review, this is
   // hardly ideal. "download" should be translated, or another solution found.
   // (cf. http://code.google.com/p/chromium/issues/detail?id=25289)
-  const wchar_t kFinalFallbackName[] = L"download";
+  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() ? std::wstring(kFinalFallbackName)
-                                : default_name;
+  if (url.SchemeIs("about") || url.SchemeIs("data")) {
+    return FilePath(UTF8ToFilePathString(
+        default_name && default_name[0] ? default_name : kFinalFallbackName));
+  }
 
-  std::wstring filename = GetFileNameFromCD(content_disposition,
-                                            referrer_charset);
+  std::string filename = GetFileNameFromCD(content_disposition,
+                                           referrer_charset);
   if (!filename.empty()) {
     // Remove any path information the server may have sent, take the name
     // only.
-    filename = file_util::GetFilenameFromPath(filename);
+#if defined(OS_WIN)
+    filename = UTF16ToUTF8(FilePath(UTF8ToUTF16(filename)).BaseName().value());
+#else
+    filename = FilePath(filename).BaseName().value();
+#endif
+
     // Next, remove "." from the beginning and end of the file name to avoid
     // tricks with hidden files, "..", and "."
-    TrimString(filename, L".", &filename);
+    TrimString(filename, ".", &filename);
   }
   if (filename.empty()) {
     if (url.is_valid()) {
-      filename = UnescapeAndDecodeUTF8URLComponent(
+      filename = UnescapeURLComponent(
           url.ExtractFileName(),
           UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS);
     }
   }
 
   // Trim '.' once more.
-  TrimString(filename, L".", &filename);
+  TrimString(filename, ".", &filename);
+
   // If there's no filename or it gets trimed to be empty, use
   // the URL hostname or default_name
   if (filename.empty()) {
-    if (!default_name.empty()) {
+    if (default_name && default_name[0]) {
       filename = default_name;
     } else if (url.is_valid()) {
       // Some schemes (e.g. file) do not have a hostname. Even though it's
       // not likely to reach here, let's hardcode the last fallback name.
       // TODO(jungshik) : Decode a 'punycoded' IDN hostname. (bug 1264451)
-      filename = url.host().empty() ? std::wstring(kFinalFallbackName)
-                                    : UTF8ToWide(url.host());
+      filename = url.host().empty() ? std::string(kFinalFallbackName)
+                                    : url.host();
     } else {
       NOTREACHED();
     }
   }
 
-  file_util::ReplaceIllegalCharacters(&filename, '-');
-  return filename;
+#if defined(OS_WIN)
+  FilePath::StringType file_path_string = UTF8ToWide(filename);
+#else
+  std::string& file_path_string = filename;
+#endif
+  file_util::ReplaceIllegalCharactersInPath(&file_path_string, '-');
+  return FilePath(file_path_string);
 }
 
 bool IsPortAllowedByDefault(int port) {
diff --git a/net/base/net_util.h b/net/base/net_util.h
index 302a55f..1ad4ac2 100644
--- a/net/base/net_util.h
+++ b/net/base/net_util.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 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.
 
-#ifndef NET_BASE_NET_UTIL_H__
-#define NET_BASE_NET_UTIL_H__
+#ifndef NET_BASE_NET_UTIL_H_
+#define NET_BASE_NET_UTIL_H_
 
 #include "build/build_config.h"
 
@@ -126,8 +126,8 @@ std::string GetHeaderParamValue(const std::string& field,
 // other caller is a unit test. Need to figure out expose this function only to
 // net_util_unittest.
 //
-std::wstring GetFileNameFromCD(const std::string& header,
-                               const std::string& referrer_charset);
+std::string GetFileNameFromCD(const std::string& header,
+                              const std::string& referrer_charset);
 
 // Converts the given host name to unicode characters, APPENDING them to the
 // the given output string. This can be called for any host name, if the
@@ -194,14 +194,17 @@ std::wstring StripWWW(const std::wstring& 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|.  Note: it's possible for the suggested filename to be empty (e.g.,
+// |url|. If there is no filename or it can't be used, the given default name
+// will be used if specified.
+
+// 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.
-std::wstring GetSuggestedFilename(const GURL& url,
-                                  const std::string& content_disposition,
-                                  const std::string& referrer_charset,
-                                  const std::wstring& default_name);
+FilePath GetSuggestedFilename(const GURL& url,
+                              const std::string& content_disposition,
+                              const std::string& referrer_charset,
+                              const char* default_name);
 
 // Checks the given port against a list of ports which are restricted by
 // default.  Returns true if the port is allowed, false if it is restricted.
@@ -259,4 +262,4 @@ void SetExplicitlyAllowedPorts(const std::wstring& allowed_ports);
 
 }  // namespace net
 
-#endif  // NET_BASE_NET_UTIL_H__
+#endif  // NET_BASE_NET_UTIL_H_
diff --git a/net/base/net_util_unittest.cc b/net/base/net_util_unittest.cc
index eddbd16..f8faedf 100644
--- a/net/base/net_util_unittest.cc
+++ b/net/base/net_util_unittest.cc
@@ -353,7 +353,7 @@ struct SuggestedFilenameCase {
   const char* url;
   const char* content_disp_header;
   const char* referrer_charset;
-  const wchar_t* default_filename;
+  const char* default_filename;
   const wchar_t* expected_filename;
 };
 
@@ -770,8 +770,8 @@ TEST(NetUtilTest, GetFileNameFromCD) {
   };
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     EXPECT_EQ(tests[i].expected,
-              net::GetFileNameFromCD(tests[i].header_field,
-                                     tests[i].referrer_charset));
+              UTF8ToWide(net::GetFileNameFromCD(tests[i].header_field,
+                                                tests[i].referrer_charset)));
   }
 }
 
@@ -853,32 +853,32 @@ 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/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
     // to the hostname. If this behavior is not desirable, we'd better
@@ -886,114 +886,119 @@ 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",
+     "download",
      L"test.html"},
     {"http://www.google.com/",
      "",
      "",
-     L"download",
+     "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=\"..\"",
      "",
-     L"download",
+     "download",
      L"download"},
     {"http://www.google.com/test.html",
      "Content-disposition: attachment; filename=\"..\"",
      "",
-     L"download",
+     "download",
      L"test.html"},
     // Below is a small subset of cases taken from GetFileNameFromCD test above.
     {"http://www.google.com/",
      "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",
+     "download",
      L"\uc608\uc220 \uc608\uc220.jpg"},
     {"http://www.google.com/",
      "Content-disposition: attachment;",
      "",
-     L"\uB2E4\uC6B4\uB85C\uB4DC",
+     "\xEB\x8B\xA4\xEC\x9A\xB4\xEB\xA1\x9C\xEB\x93\x9C",
      L"\uB2E4\uC6B4\uB85C\uB4DC"},
     {"http://www.google.com/",
      "Content-Disposition: attachment; filename=\"=?EUC-JP?Q?=B7=DD=BD="
      "D13=2Epng?=\"",
      "",
-     L"download",
+     "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"},
   };
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
-    std::wstring filename = net::GetSuggestedFilename(
+    FilePath filename = net::GetSuggestedFilename(
         GURL(test_cases[i].url), test_cases[i].content_disp_header,
         test_cases[i].referrer_charset, test_cases[i].default_filename);
-    EXPECT_EQ(std::wstring(test_cases[i].expected_filename), filename);
+#if defined(OS_WIN)
+    EXPECT_EQ(std::wstring(test_cases[i].expected_filename), filename.value())
+#else
+    EXPECT_EQ(WideToUTF8(test_cases[i].expected_filename), filename.value())
+#endif
+      << "Iteration " << i << ": " << test_cases[i].url;
   }
 }