Fix strings in url_fixer_upper.

* change URLs from wstring to string
* change filenames from wstring to FilePath
* url_fixer_upper{_unittest}.cc building on posix

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

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

1 files changed, 140 insertions, 128 deletions

diff --git a/chrome/browser/net/url_fixer_upper_unittest.cc b/chrome/browser/net/url_fixer_upper_unittest.cc
index 024060c..1e6dbc7 100644
--- a/chrome/browser/net/url_fixer_upper_unittest.cc
+++ b/chrome/browser/net/url_fixer_upper_unittest.cc
@@ -3,9 +3,9 @@
 // found in the LICENSE file.
 
 #include <stdlib.h>
-#include <windows.h>
 
 #include "base/basictypes.h"
+#include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
@@ -26,8 +26,8 @@ std::ostream& operator<<(std::ostream& os, const url_parse::Component& part) {
 }
 
 struct segment_case {
-  const std::wstring input;
-  const std::wstring result;
+  const std::string input;
+  const std::string result;
   const url_parse::Component scheme;
   const url_parse::Component username;
   const url_parse::Component password;
@@ -39,7 +39,7 @@ struct segment_case {
 };
 
 static const segment_case segment_cases[] = {
-  { L"http://www.google.com/", L"http",
+  { "http://www.google.com/", "http",
     url_parse::Component(0, 4), // scheme
     url_parse::Component(), // username
     url_parse::Component(), // password
@@ -49,7 +49,7 @@ static const segment_case segment_cases[] = {
     url_parse::Component(), // query
     url_parse::Component(), // ref
   },
-  { L"aBoUt:vErSiOn", L"about",
+  { "aBoUt:vErSiOn", "about",
     url_parse::Component(0, 5), // scheme
     url_parse::Component(), // username
     url_parse::Component(), // password
@@ -59,7 +59,7 @@ static const segment_case segment_cases[] = {
     url_parse::Component(), // query
     url_parse::Component(), // ref
   },
-  { L"    www.google.com:124?foo#", L"http",
+  { "    www.google.com:124?foo#", "http",
     url_parse::Component(), // scheme
     url_parse::Component(), // username
     url_parse::Component(), // password
@@ -69,7 +69,7 @@ static const segment_case segment_cases[] = {
     url_parse::Component(23, 3), // query
     url_parse::Component(27, 0), // ref
   },
-  { L"user@www.google.com", L"http",
+  { "user@www.google.com", "http",
     url_parse::Component(), // scheme
     url_parse::Component(0, 4), // username
     url_parse::Component(), // password
@@ -79,7 +79,7 @@ static const segment_case segment_cases[] = {
     url_parse::Component(), // query
     url_parse::Component(), // ref
   },
-  { L"ftp:/user:P:a$$Wd@..ftp.google.com...::23///pub?foo#bar", L"ftp",
+  { "ftp:/user:P:a$$Wd@..ftp.google.com...::23///pub?foo#bar", "ftp",
     url_parse::Component(0, 3), // scheme
     url_parse::Component(5, 4), // username
     url_parse::Component(10, 7), // password
@@ -92,10 +92,10 @@ static const segment_case segment_cases[] = {
 };
 
 TEST(URLFixerUpperTest, SegmentURL) {
-  std::wstring result;
+  std::string result;
   url_parse::Parsed parts;
 
-  for (int i = 0; i < arraysize(segment_cases); ++i) {
+  for (size_t i = 0; i < arraysize(segment_cases); ++i) {
     segment_case value = segment_cases[i];
     result = URLFixerUpper::SegmentURL(value.input, &parts);
     EXPECT_EQ(value.result, result);
@@ -115,76 +115,74 @@ TEST(URLFixerUpperTest, SegmentURL) {
 //    full_path = "c:\foo\bar.txt"
 //    dir = "c:\foo"
 //    file_name = "bar.txt"
-static bool MakeTempFile(const std::wstring& dir,
-                         const std::wstring& file_name,
-                         std::wstring* full_path) {
-  *full_path = dir + L"\\" + file_name;
-
-  HANDLE hfile = CreateFile(full_path->c_str(), GENERIC_READ | GENERIC_WRITE,
-                            0, NULL, CREATE_ALWAYS, 0, NULL);
-  if (hfile == NULL || hfile == INVALID_HANDLE_VALUE)
-    return false;
-  CloseHandle(hfile);
-  return true;
+static bool MakeTempFile(const FilePath& dir,
+                         const FilePath& file_name,
+                         FilePath* full_path) {
+  *full_path = dir.Append(file_name);
+  return file_util::WriteFile(full_path->ToWStringHack(), NULL, 0) == 0;
 }
 
 // Returns true if the given URL is a file: URL that matches the given file
-static bool IsMatchingFileURL(const std::wstring& url,
-                              const std::wstring& full_file_path) {
+static bool IsMatchingFileURL(const std::string& url,
+                              const FilePath& full_file_path) {
   if (url.length() <= 8)
     return false;
-  if (std::wstring(L"file:///") != url.substr(0, 8))
+  if (std::string("file:///") != url.substr(0, 8))
     return false; // no file:/// prefix
-  if (url.find('\\') != std::wstring::npos)
+  if (url.find('\\') != std::string::npos)
     return false; // contains backslashes
 
-  std::wstring derived_path;
+  FilePath derived_path;
   net::FileURLToFilePath(GURL(url), &derived_path);
-  return (derived_path.length() == full_file_path.length()) &&
-      std::equal(derived_path.begin(), derived_path.end(),
-                 full_file_path.begin(), CaseInsensitiveCompare<wchar_t>());
+
+  FilePath::StringType derived_path_str = derived_path.value();
+  return (derived_path_str.length() == full_file_path.value().length()) &&
+      std::equal(derived_path_str.begin(),
+                 derived_path_str.end(),
+                 full_file_path.value().begin(),
+                 CaseInsensitiveCompare<FilePath::CharType>());
 }
 
 struct fixup_case {
-  const std::wstring input;
-  const std::wstring desired_tld;
-  const std::wstring output;
+  const std::string input;
+  const std::string desired_tld;
+  const std::string output;
 } fixup_cases[] = {
-  {L"www.google.com", L"", L"http://www.google.com/"},
-  {L" www.google.com     ", L"", L"http://www.google.com/"},
-  {L" foo.com/asdf  bar", L"", L"http://foo.com/asdf  bar"},
-  {L"..www.google.com..", L"", L"http://www.google.com./"},
-  {L"http://......", L"", L"http://....../"},
-  {L"http://host.com:ninety-two/", L"", L"http://host.com/"},
-  {L"http://host.com:ninety-two?foo", L"", L"http://host.com/?foo"},
-  {L"google.com:123", L"", L"http://google.com:123/"},
-  {L"about:", L"", L"about:"},
-  {L"about:version", L"", L"about:version"},
-  {L"www:123", L"", L"http://www:123/"},
-  {L"   www:123", L"", L"http://www:123/"},
-  {L"www.google.com?foo", L"", L"http://www.google.com/?foo"},
-  {L"www.google.com#foo", L"", L"http://www.google.com/#foo"},
-  {L"www.google.com?", L"", L"http://www.google.com/?"},
-  {L"www.google.com#", L"", L"http://www.google.com/#"},
-  {L"www.google.com:123?foo#bar", L"", L"http://www.google.com:123/?foo#bar"},
-  {L"user@www.google.com", L"", L"http://user@www.google.com/"},
-  {L"\x6C34.com", L"", L"http://\x6C34.com/" },
+  {"www.google.com", "", "http://www.google.com/"},
+  {" www.google.com     ", "", "http://www.google.com/"},
+  {" foo.com/asdf  bar", "", "http://foo.com/asdf  bar"},
+  {"..www.google.com..", "", "http://www.google.com./"},
+  {"http://......", "", "http://....../"},
+  {"http://host.com:ninety-two/", "", "http://host.com/"},
+  {"http://host.com:ninety-two?foo", "", "http://host.com/?foo"},
+  {"google.com:123", "", "http://google.com:123/"},
+  {"about:", "", "about:"},
+  {"about:version", "", "about:version"},
+  {"www:123", "", "http://www:123/"},
+  {"   www:123", "", "http://www:123/"},
+  {"www.google.com?foo", "", "http://www.google.com/?foo"},
+  {"www.google.com#foo", "", "http://www.google.com/#foo"},
+  {"www.google.com?", "", "http://www.google.com/?"},
+  {"www.google.com#", "", "http://www.google.com/#"},
+  {"www.google.com:123?foo#bar", "", "http://www.google.com:123/?foo#bar"},
+  {"user@www.google.com", "", "http://user@www.google.com/"},
+  {"\xE6\xB0\xB4.com" , "", "http://\xE6\xB0\xB4.com/"},
   // It would be better if this next case got treated as http, but I don't see
   // a clean way to guess this isn't the new-and-exciting "user" scheme.
-  {L"user:passwd@www.google.com:8080/", L"", L"user:passwd@www.google.com:8080/"},
-  //{L"file:///c:/foo/bar%20baz.txt", L"", L"file:///C:/foo/bar%20baz.txt"},
-  {L"ftp.google.com", L"", L"ftp://ftp.google.com/"},
-  {L"    ftp.google.com", L"", L"ftp://ftp.google.com/"},
-  {L"FTP.GooGle.com", L"", L"ftp://FTP.GooGle.com/"},
-  {L"ftpblah.google.com", L"", L"http://ftpblah.google.com/"},
-  {L"ftp", L"", L"http://ftp/"},
-  {L"google.ftp.com", L"", L"http://google.ftp.com/"},
+  {"user:passwd@www.google.com:8080/", "", "user:passwd@www.google.com:8080/"},
+  //{"file:///c:/foo/bar%20baz.txt", "", "file:///C:/foo/bar%20baz.txt"},
+  {"ftp.google.com", "", "ftp://ftp.google.com/"},
+  {"    ftp.google.com", "", "ftp://ftp.google.com/"},
+  {"FTP.GooGle.com", "", "ftp://FTP.GooGle.com/"},
+  {"ftpblah.google.com", "", "http://ftpblah.google.com/"},
+  {"ftp", "", "http://ftp/"},
+  {"google.ftp.com", "", "http://google.ftp.com/"},
 };
 
 TEST(URLFixerUpperTest, FixupURL) {
-  std::wstring output;
+  std::string output;
 
-  for (int i = 0; i < arraysize(fixup_cases); ++i) {
+  for (size_t i = 0; i < arraysize(fixup_cases); ++i) {
     fixup_case value = fixup_cases[i];
     output = URLFixerUpper::FixupURL(value.input, value.desired_tld);
     EXPECT_EQ(value.output, output);
@@ -192,25 +190,25 @@ TEST(URLFixerUpperTest, FixupURL) {
 
   // Check the TLD-appending functionality
   fixup_case tld_cases[] = {
-    {L"google", L"com", L"http://www.google.com/"},
-    {L"google.", L"com", L"http://www.google.com/"},
-    {L"google..", L"com", L"http://www.google.com/"},
-    {L".google", L"com", L"http://www.google.com/"},
-    {L"www.google", L"com", L"http://www.google.com/"},
-    {L"google.com", L"com", L"http://google.com/"},
-    {L"http://google", L"com", L"http://www.google.com/"},
-    {L"..google..", L"com", L"http://www.google.com/"},
-    {L"http://www.google", L"com", L"http://www.google.com/"},
-    {L"google/foo", L"com", L"http://www.google.com/foo"},
-    {L"google.com/foo", L"com", L"http://google.com/foo"},
-    {L"google/?foo=.com", L"com", L"http://www.google.com/?foo=.com"},
-    {L"www.google/?foo=www.", L"com", L"http://www.google.com/?foo=www."},
-    {L"google.com/?foo=.com", L"com", L"http://google.com/?foo=.com"},
-    {L"http://www.google.com", L"com", L"http://www.google.com/"},
-    {L"google:123", L"com", L"http://www.google.com:123/"},
-    {L"http://google:123", L"com", L"http://www.google.com:123/"},
+    {"google", "com", "http://www.google.com/"},
+    {"google.", "com", "http://www.google.com/"},
+    {"google..", "com", "http://www.google.com/"},
+    {".google", "com", "http://www.google.com/"},
+    {"www.google", "com", "http://www.google.com/"},
+    {"google.com", "com", "http://google.com/"},
+    {"http://google", "com", "http://www.google.com/"},
+    {"..google..", "com", "http://www.google.com/"},
+    {"http://www.google", "com", "http://www.google.com/"},
+    {"google/foo", "com", "http://www.google.com/foo"},
+    {"google.com/foo", "com", "http://google.com/foo"},
+    {"google/?foo=.com", "com", "http://www.google.com/?foo=.com"},
+    {"www.google/?foo=www.", "com", "http://www.google.com/?foo=www."},
+    {"google.com/?foo=.com", "com", "http://google.com/?foo=.com"},
+    {"http://www.google.com", "com", "http://www.google.com/"},
+    {"google:123", "com", "http://www.google.com:123/"},
+    {"http://google:123", "com", "http://www.google.com:123/"},
   };
-  for (int i = 0; i < arraysize(tld_cases); ++i) {
+  for (size_t i = 0; i < arraysize(tld_cases); ++i) {
     fixup_case value = tld_cases[i];
     output = URLFixerUpper::FixupURL(value.input, value.desired_tld);
     EXPECT_EQ(value.output, output);
@@ -222,113 +220,127 @@ TEST(URLFixerUpperTest, FixupURL) {
 // has to exist.
 TEST(URLFixerUpperTest, FixupFile) {
   // this "original" filename is the one we tweak to get all the variations
-  std::wstring dir;
-  std::wstring original;
+  FilePath dir;
+  FilePath original;
   ASSERT_TRUE(PathService::Get(chrome::DIR_APP, &dir));
-  ASSERT_TRUE(MakeTempFile(dir, L"url fixer upper existing file.txt",
+  ASSERT_TRUE(MakeTempFile(dir,
+                           FilePath(FILE_PATH_LITERAL("url fixer upper existing file.txt")),
                            &original));
 
   // reference path
-  std::wstring golden =
-      UTF8ToWide(net::FilePathToFileURL(original).spec());
+  std::string golden = net::FilePathToFileURL(original).spec();
 
   // c:\foo\bar.txt -> file:///c:/foo/bar.txt (basic)
-  std::wstring fixedup = URLFixerUpper::FixupURL(original, L"");
+#if defined(OS_WIN)
+  std::string fixedup = URLFixerUpper::FixupURL(WideToUTF8(original.value()), "");
+#elif defined(OS_POSIX)
+  std::string fixedup = URLFixerUpper::FixupURL(original.value(), "");
+#endif
   EXPECT_EQ(golden, fixedup);
 
+  // TODO(port): Make some equivalent tests for posix.
+#if defined(OS_WIN)
   // c|/foo\bar.txt -> file:///c:/foo/bar.txt (pipe allowed instead of colon)
-  std::wstring cur(original);
+  std::string cur(WideToUTF8(original.value()));
   EXPECT_EQ(':', cur[1]);
   cur[1] = '|';
-  fixedup = URLFixerUpper::FixupURL(cur, L"");
+  fixedup = URLFixerUpper::FixupURL(cur, "");
   EXPECT_EQ(golden, fixedup);
 
   fixup_case file_cases[] = {
     // File URLs go through GURL, which tries to escape intelligently.
-    {L"c:\\This%20is a non-existent file.txt", L"", L"file:///C:/This%2520is%20a%20non-existent%20file.txt"},
+    {"c:\\This%20is a non-existent file.txt", "", "file:///C:/This%2520is%20a%20non-existent%20file.txt"},
 
     // \\foo\bar.txt -> file://foo/bar.txt
     // UNC paths, this file won't exist, but since there are no escapes, it
     // should be returned just converted to a file: URL.
-    {L"\\\\SomeNonexistentHost\\foo\\bar.txt", L"", L"file://somenonexistenthost/foo/bar.txt"},
-    {L"//SomeNonexistentHost\\foo/bar.txt", L"", L"file://somenonexistenthost/foo/bar.txt"},
-    {L"file:///C:/foo/bar", L"", L"file:///C:/foo/bar"},
+    {"\\\\SomeNonexistentHost\\foo\\bar.txt", "", "file://somenonexistenthost/foo/bar.txt"},
+    {"//SomeNonexistentHost\\foo/bar.txt", "", "file://somenonexistenthost/foo/bar.txt"},
+    {"file:///C:/foo/bar", "", "file:///C:/foo/bar"},
 
     // These are fixups we don't do, but could consider:
     //
-    //   {L"file://C:/foo/bar", L"", L"file:///C:/foo/bar"},
-    //   {L"file:c:", L"", L"file:///c:/"},
-    //   {L"file:c:WINDOWS", L"", L"file:///c:/WINDOWS"},
-    //   {L"file:c|Program Files", L"", L"file:///c:/Program Files"},
-    //   {L"file:///foo:/bar", L"", L"file://foo/bar"},
-    //   {L"file:/file", L"", L"file://file/"},
-    //   {L"file:////////c:\\foo", L"", L"file:///c:/foo"},
-    //   {L"file://server/folder/file", L"", L"file://server/folder/file"},
-    //   {L"file:/\\/server\\folder/file", L"", L"file://server/folder/file"},
+    //   {"file://C:/foo/bar", "", "file:///C:/foo/bar"},
+    //   {"file:c:", "", "file:///c:/"},
+    //   {"file:c:WINDOWS", "", "file:///c:/WINDOWS"},
+    //   {"file:c|Program Files", "", "file:///c:/Program Files"},
+    //   {"file:///foo:/bar", "", "file://foo/bar"},
+    //   {"file:/file", "", "file://file/"},
+    //   {"file:////////c:\\foo", "", "file:///c:/foo"},
+    //   {"file://server/folder/file", "", "file://server/folder/file"},
+    //   {"file:/\\/server\\folder/file", "", "file://server/folder/file"},
   };
-  for (int i = 0; i < arraysize(file_cases); i++) {
+  for (size_t i = 0; i < arraysize(file_cases); i++) {
     fixedup = URLFixerUpper::FixupURL(file_cases[i].input,
                                       file_cases[i].desired_tld);
     EXPECT_EQ(file_cases[i].output, fixedup);
   }
+#endif
 
-  EXPECT_TRUE(DeleteFile(original.c_str()));
+  EXPECT_TRUE(file_util::Delete(original, false));
 }
 
 TEST(URLFixerUpperTest, FixupRelativeFile) {
-  std::wstring full_path, dir;
-  std::wstring file_part(L"url_fixer_upper_existing_file.txt");
+  FilePath full_path, dir;
+  FilePath file_part(FILE_PATH_LITERAL("url_fixer_upper_existing_file.txt"));
   ASSERT_TRUE(PathService::Get(chrome::DIR_APP, &dir));
   ASSERT_TRUE(MakeTempFile(dir, file_part, &full_path));
 
   // make sure we pass through good URLs
-  std::wstring fixedup;
-    for (int i = 0; i < arraysize(fixup_cases); ++i) {
+  std::string fixedup;
+  for (size_t i = 0; i < arraysize(fixup_cases); ++i) {
     fixup_case value = fixup_cases[i];
-    fixedup = URLFixerUpper::FixupRelativeFile(dir, value.input);
+#if defined(OS_WIN)
+    FilePath input(UTF8ToWide(value.input));
+#elif defined(OS_POSIX)
+    FilePath input(value.input);
+#endif
+    fixedup = URLFixerUpper::FixupRelativeFile(dir, input);
     EXPECT_EQ(value.output, fixedup);
   }
 
   // make sure the existing file got fixed-up to a file URL, and that there
   // are no backslashes
   fixedup = URLFixerUpper::FixupRelativeFile(dir, file_part);
-  EXPECT_PRED2(IsMatchingFileURL, fixedup, full_path);
-  EXPECT_TRUE(DeleteFile(full_path.c_str()));
+  EXPECT_TRUE(IsMatchingFileURL(fixedup, full_path));
+  EXPECT_TRUE(file_util::Delete(full_path, false));
 
   // create a filename we know doesn't exist and make sure it doesn't get
   // fixed up to a file URL
-  std::wstring nonexistent_file(L"url_fixer_upper_nonexistent_file.txt");
+  FilePath nonexistent_file(FILE_PATH_LITERAL("url_fixer_upper_nonexistent_file.txt"));
   fixedup = URLFixerUpper::FixupRelativeFile(dir, nonexistent_file);
-  EXPECT_NE(std::wstring(L"file:///"), fixedup.substr(0, 8));
+  EXPECT_NE(std::string("file:///"), fixedup.substr(0, 8));
   EXPECT_FALSE(IsMatchingFileURL(fixedup, nonexistent_file));
 
   // make a subdir to make sure relative paths with directories work, also
   // test spaces: "app_dir\url fixer-upper dir\url fixer-upper existing file.txt"
-  std::wstring sub_dir(L"url fixer-upper dir");
-  std::wstring sub_file(L"url fixer-upper existing file.txt");
-  std::wstring new_dir = dir + L"\\" + sub_dir;
-  CreateDirectory(new_dir.c_str(), NULL);
+  FilePath sub_dir(FILE_PATH_LITERAL("url fixer-upper dir"));
+  FilePath sub_file(FILE_PATH_LITERAL("url fixer-upper existing file.txt"));
+  FilePath new_dir = dir.Append(sub_dir);
+  file_util::CreateDirectory(new_dir);
   ASSERT_TRUE(MakeTempFile(new_dir, sub_file, &full_path));
 
   // test file in the subdir
-  std::wstring relative_file = sub_dir + L"\\" + sub_file;
+  FilePath relative_file = sub_dir.Append(sub_file);
   fixedup = URLFixerUpper::FixupRelativeFile(dir, relative_file);
-  EXPECT_PRED2(IsMatchingFileURL, fixedup, full_path);
+  EXPECT_TRUE(IsMatchingFileURL(fixedup, full_path));
 
-  // test file in the subdir with different slashes and escaping
-  relative_file = sub_dir + L"/" + sub_file;
-  ReplaceSubstringsAfterOffset(&relative_file, 0, L" ", L"%20");
-  fixedup = URLFixerUpper::FixupRelativeFile(dir, relative_file);
-  EXPECT_PRED2(IsMatchingFileURL, fixedup, full_path);
+  // test file in the subdir with different slashes and escaping.
+  FilePath::StringType relative_file_str = sub_dir.value() +
+      FILE_PATH_LITERAL("/") + sub_file.value();
+  ReplaceSubstringsAfterOffset(&relative_file_str, 0,
+      FILE_PATH_LITERAL(" "), FILE_PATH_LITERAL("%20"));
+  fixedup = URLFixerUpper::FixupRelativeFile(dir, FilePath(relative_file_str));
+  EXPECT_TRUE(IsMatchingFileURL(fixedup, full_path));
 
   // test relative directories and duplicate slashes
   // (should resolve to the same file as above)
-  relative_file = sub_dir + L"\\../" + sub_dir  + L"\\\\\\.\\" + sub_file;
-  fixedup = URLFixerUpper::FixupRelativeFile(dir, relative_file);
-  EXPECT_PRED2(IsMatchingFileURL, fixedup, full_path);
+  relative_file_str = sub_dir.value() + FILE_PATH_LITERAL("/../") +
+      sub_dir.value() + FILE_PATH_LITERAL("///./") + sub_file.value();
+  fixedup = URLFixerUpper::FixupRelativeFile(dir, FilePath(relative_file_str));
+  EXPECT_TRUE(IsMatchingFileURL(fixedup, full_path));
 
   // done with the subdir
-  EXPECT_TRUE(DeleteFile(full_path.c_str()));
-  EXPECT_TRUE(RemoveDirectory(new_dir.c_str()));
+  EXPECT_TRUE(file_util::Delete(full_path, false));
+  EXPECT_TRUE(file_util::Delete(new_dir, true));
 }
-