Port save_package_unittest to posix.

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

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

5 files changed, 104 insertions, 99 deletions

diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 32e2243..6f1fcf8 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -52,9 +52,6 @@ using base::Time;
 
 namespace {
 
-const FilePath::CharType kLeftParen = FILE_PATH_LITERAL('(');
-const FilePath::CharType kRightParen = FILE_PATH_LITERAL(')');
-
 // Default name which will be used when we can not get proper name from
 // resource URL.
 const wchar_t kDefaultSaveName[] = L"saved_resource";
@@ -180,8 +177,8 @@ SavePackage::SavePackage(WebContents* web_content,
 
 // This is for testing use. Set |finished_| as true because we don't want
 // method Cancel to be be called in destructor in test mode.
-SavePackage::SavePackage(const FilePath::CharType* file_full_path,
-                         const FilePath::CharType* directory_full_path)
+SavePackage::SavePackage(const FilePath& file_full_path,
+                         const FilePath& directory_full_path)
     : download_(NULL),
       saved_main_file_path_(file_full_path),
       saved_main_directory_path_(directory_full_path),
diff --git a/chrome/browser/download/save_package.h b/chrome/browser/download/save_package.h
index 2035b61..c68eb86 100644
--- a/chrome/browser/download/save_package.h
+++ b/chrome/browser/download/save_package.h
@@ -204,8 +204,8 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
 
  private:
   // For testing only.
-  SavePackage(const FilePath::CharType* file_full_path,
-              const FilePath::CharType* directory_full_path);
+  SavePackage(const FilePath& file_full_path,
+              const FilePath& directory_full_path);
 
   void Stop();
   void CheckFinish();
diff --git a/chrome/browser/download/save_package_unittest.cc b/chrome/browser/download/save_package_unittest.cc
index a96c0fc..7837523 100644
--- a/chrome/browser/download/save_package_unittest.cc
+++ b/chrome/browser/download/save_package_unittest.cc
@@ -2,37 +2,76 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
+#include "base/file_path.h"
 #include "base/logging.h"
+#include "base/path_service.h"
+#include "base/string_util.h"
 #include "chrome/browser/download/save_package.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-//    0         1         2         3         4         5         6
-//    0123456789012345678901234567890123456789012345678901234567890123456789
-static const wchar_t* const kLongFilePath =
-    L"C:\\EFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
-    L"89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
-    L"6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
-    L"456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789a.htm";
+#define FPL FILE_PATH_LITERAL
+
+namespace {
+
+// This constant copied from save_package.cc.
+#if defined(OS_WIN)
+const uint32 kMaxFilePathLength = MAX_PATH - 1;
+#elif defined(OS_POSIX)
+const uint32 kMaxFilePathLength = PATH_MAX - 1;
+#endif
+
+// Used to make long filenames.
+std::string long_file_name(
+    "EFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
+    "89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
+    "6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
+    "456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789a");
+
+bool HasOrdinalNumber(const FilePath::StringType& filename) {
+  FilePath::StringType::size_type r_paren_index = filename.rfind(FPL(')'));
+  FilePath::StringType::size_type l_paren_index = filename.rfind(FPL('('));
+  if (l_paren_index >= r_paren_index)
+    return false;
+
+  for (FilePath::StringType::size_type i = l_paren_index + 1;
+       i != r_paren_index; ++i) {
+    if (!IsAsciiDigit(filename[i]))
+      return false;
+  }
+
+  return true;
+}
 
-static const wchar_t* const kLongDirPath =
-    L"C:\\EFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
-    L"89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
-    L"6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
-    L"456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789a_files";
+}  // namespace
 
 class SavePackageTest : public testing::Test {
  public:
   SavePackageTest() {
-    save_package_success_ = new SavePackage(L"c:\\testfile.htm",
-                                            L"c:\\testfile_files");
-    save_package_fail_ = new SavePackage(kLongFilePath, kLongDirPath);
+    FilePath test_dir;
+    PathService::Get(base::DIR_TEMP, &test_dir);
+
+    save_package_success_ = new SavePackage(
+        test_dir.AppendASCII("testfile.htm"),
+        test_dir.AppendASCII("testfile_files"));
+
+    // We need to construct a path that is *almost* kMaxFilePathLength long
+    long_file_name.resize(kMaxFilePathLength + long_file_name.length());
+    while (long_file_name.length() < kMaxFilePathLength)
+      long_file_name += long_file_name;
+    long_file_name.resize(kMaxFilePathLength - 9 - test_dir.value().length());
+
+    save_package_fail_ = new SavePackage(
+        test_dir.AppendASCII(long_file_name + ".htm"),
+        test_dir.AppendASCII(long_file_name + "_files"));
   }
 
   bool GetGeneratedFilename(bool need_success_generate_filename,
                             const std::string& disposition,
-                            const std::wstring& url,
+                            const std::string& url,
                             bool need_htm_ext,
-                            std::wstring* generated_name) {
+                            FilePath::StringType* generated_name) {
     SavePackage* save_package;
     if (need_success_generate_filename)
       save_package = save_package_success_.get();
@@ -42,110 +81,52 @@ class SavePackageTest : public testing::Test {
                                           generated_name);
   }
 
- protected:
-  DISALLOW_EVIL_CONSTRUCTORS(SavePackageTest);
-
  private:
   // SavePackage for successfully generating file name.
   scoped_refptr<SavePackage> save_package_success_;
   // SavePackage for failed generating file name.
   scoped_refptr<SavePackage> save_package_fail_;
+
+  DISALLOW_COPY_AND_ASSIGN(SavePackageTest);
 };
 
 static const struct {
   const char* disposition;
-  const wchar_t* url;
-  const wchar_t* expected_name;
+  const char* url;
+  const FilePath::CharType* expected_name;
   bool need_htm_ext;
 } kGeneratedFiles[] = {
   // We mainly focus on testing duplicated names here, since retrieving file
   // name from disposition and url has been tested in DownloadManagerTest.
 
   // No useful information in disposition or URL, use default.
-  {"1.html", L"http://www.savepage.com/", L"saved_resource.htm", true},
+  {"1.html", "http://www.savepage.com/", FPL("saved_resource.htm"), true},
 
   // No duplicate occurs.
-  {"filename=1.css", L"http://www.savepage.com", L"1.css", false},
+  {"filename=1.css", "http://www.savepage.com", FPL("1.css"), false},
 
   // No duplicate occurs.
-  {"filename=1.js", L"http://www.savepage.com", L"1.js", false},
+  {"filename=1.js", "http://www.savepage.com", FPL("1.js"), false},
 
   // Append numbers for duplicated names.
-  {"filename=1.css", L"http://www.savepage.com", L"1(1).css", false},
+  {"filename=1.css", "http://www.savepage.com", FPL("1(1).css"), false},
 
   // No duplicate occurs.
-  {"filename=1(1).js", L"http://www.savepage.com", L"1(1).js", false},
+  {"filename=1(1).js", "http://www.savepage.com", FPL("1(1).js"), false},
 
   // Append numbers for duplicated names.
-  {"filename=1.css", L"http://www.savepage.com", L"1(2).css", false},
+  {"filename=1.css", "http://www.savepage.com", FPL("1(2).css"), false},
 
   // Change number for duplicated names.
-  {"filename=1(1).css", L"http://www.savepage.com", L"1(3).css", false},
+  {"filename=1(1).css", "http://www.savepage.com", FPL("1(3).css"), false},
 
   // No duplicate occurs.
-  {"filename=1(11).css", L"http://www.savepage.com", L"1(11).css", false},
-
-  // test length of file path is great than MAX_PATH(260 characters).
-  {"",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  //                      0         1         2         3         4
-  //                      01234567890123456789012345678901234567890123456789
-  L"http://www.google.com/ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmn"
-  L"opqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijkl"
-  L"mnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghij"
-  L"klmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefgh"
-  L"test.css",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  L"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
-  L"89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
-  L"6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
-  L"456789ABCDEFGHIJKLMNOPQRSTU.css",
-  false},
-
-  // test duplicated file name, its length of file path is great than
-  // MAX_PATH(260 characters).
-  {"",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  //                      0         1         2         3         4
-  //                      01234567890123456789012345678901234567890123456789
-  L"http://www.google.com/ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmn"
-  L"opqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijkl"
-  L"mnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghij"
-  L"klmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefgh"
-  L"test.css",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  L"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
-  L"89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
-  L"6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
-  L"456789ABCDEFGHIJKLMNO(1).css",
-  false},
-
-  {"",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  //                      0         1         2         3         4
-  //                      01234567890123456789012345678901234567890123456789
-  L"http://www.google.com/ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmn"
-  L"opqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijkl"
-  L"mnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghij"
-  L"klmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefgh"
-  L"test.css",
-  // 0         1         2         3         4         5         6
-  // 0123456789012345678901234567890123456789012345678901234567890123456789
-  L"ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz01234567"
-  L"89ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz012345"
-  L"6789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnopqrstuvwxyz0123"
-  L"456789ABCDEFGHIJKLMNO(2).css",
-  false}
+  {"filename=1(11).css", "http://www.savepage.com", FPL("1(11).css"), false},
 };
 
 TEST_F(SavePackageTest, TestSuccessfullyGenerateSavePackageFilename) {
-  for (int i = 0; i < arraysize(kGeneratedFiles); ++i) {
-    std::wstring file_name;
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kGeneratedFiles); ++i) {
+    FilePath::StringType file_name;
     bool ok = GetGeneratedFilename(true,
                                    kGeneratedFiles[i].disposition,
                                    kGeneratedFiles[i].url,
@@ -157,8 +138,8 @@ TEST_F(SavePackageTest, TestSuccessfullyGenerateSavePackageFilename) {
 }
 
 TEST_F(SavePackageTest, TestUnSuccessfullyGenerateSavePackageFilename) {
-  for (int i = 0; i < arraysize(kGeneratedFiles); ++i) {
-    std::wstring file_name;
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kGeneratedFiles); ++i) {
+    FilePath::StringType file_name;
     bool ok = GetGeneratedFilename(false,
                                    kGeneratedFiles[i].disposition,
                                    kGeneratedFiles[i].url,
@@ -168,3 +149,29 @@ TEST_F(SavePackageTest, TestUnSuccessfullyGenerateSavePackageFilename) {
   }
 }
 
+TEST_F(SavePackageTest, TestLongSavePackageFilename) {
+  const std::string base_url("http://www.google.com/");
+  const std::string long_file = long_file_name + ".css";
+  const std::string url = base_url + long_file;
+
+  FilePath::StringType filename;
+  // Test that the filename is successfully shortened to fit.
+  ASSERT_TRUE(GetGeneratedFilename(true, "", url, false, &filename));
+  EXPECT_TRUE(filename.length() < long_file.length());
+  EXPECT_FALSE(HasOrdinalNumber(filename));
+
+  // Test that the filename is successfully shortened to fit, and gets an
+  // an ordinal appended.
+  ASSERT_TRUE(GetGeneratedFilename(true, "", url, false, &filename));
+  EXPECT_TRUE(filename.length() < long_file.length());
+  EXPECT_TRUE(HasOrdinalNumber(filename));
+
+  // Test that the filename is successfully shortened to fit, and gets a
+  // different ordinal appended.
+  FilePath::StringType filename2;
+  ASSERT_TRUE(GetGeneratedFilename(true, "", url, false, &filename2));
+  EXPECT_TRUE(filename2.length() < long_file.length());
+  EXPECT_TRUE(HasOrdinalNumber(filename2));
+  EXPECT_NE(filename, filename2);
+}
+
diff --git a/chrome/chrome.xcodeproj/project.pbxproj b/chrome/chrome.xcodeproj/project.pbxproj
index 3d5b11b..8ab2f18 100644
--- a/chrome/chrome.xcodeproj/project.pbxproj
+++ b/chrome/chrome.xcodeproj/project.pbxproj
@@ -77,6 +77,7 @@
 /* Begin PBXBuildFile section */
 		0D71821EDDA2629232DE3AD9 /* safe_browsing_blocking_page.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D7BFADB0E9D49DE009A6919 /* safe_browsing_blocking_page.cc */; };
 		0EE123B79B750A2FCEFB4569 /* history_backend_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D7BF9F00E9D48F7009A6919 /* history_backend_unittest.cc */; };
+		0FF05474D5345CABC5C85B5F /* save_package_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D7BF9DC0E9D48CE009A6919 /* save_package_unittest.cc */; };
 		1647A33CB5B4B14087BFF5C8 /* dns_global.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D7BFA680E9D4981009A6919 /* dns_global.cc */; };
 		1C284EB767D0E3D302AC675C /* tab_restore_service.cc in Sources */ = {isa = PBXBuildFile; fileRef = B020A11D500D7519E54F2957 /* tab_restore_service.cc */; };
 		2760C4346D6AB3AD94E9CF05 /* url_fixer_upper.cc in Sources */ = {isa = PBXBuildFile; fileRef = B5D16EF40F2145C600861FAC /* url_fixer_upper.cc */; };
@@ -5597,6 +5598,7 @@
 				4D7BFCF30E9D4E07009A6919 /* run_all_unittests.cc in Sources */,
 				E48FB9870EC4EBA10052B72B /* safe_browsing_database_unittest.cc in Sources */,
 				4D7BFB7F0E9D4C63009A6919 /* safe_browsing_util_unittest.cc in Sources */,
+				0FF05474D5345CABC5C85B5F /* save_package_unittest.cc in Sources */,
 				E4F324980EE5D7DE002533CE /* snippet_unittest.cc in Sources */,
 				7442556F908264C52BEBFE4D /* starred_url_database_unittest.cc in Sources */,
 				8268589C0F326CCC009F6555 /* template_url_parser_unittest.cc in Sources */,
diff --git a/chrome/test/unit/unit_tests.scons b/chrome/test/unit/unit_tests.scons
index 301fc24..c8464ca 100644
--- a/chrome/test/unit/unit_tests.scons
+++ b/chrome/test/unit/unit_tests.scons
@@ -394,7 +394,6 @@ if not env.Bit('windows'):
       '$CHROME_DIR/browser/browser_commands_unittest.cc',
       '$CHROME_DIR/browser/download/download_manager_unittest.cc',
       '$CHROME_DIR/browser/download/download_request_manager_unittest.cc',
-      '$CHROME_DIR/browser/download/save_package_unittest.cc',
       '$CHROME_DIR/browser/extensions/user_script_master_unittest.cc',
       '$CHROME_DIR/browser/importer/firefox_importer_unittest.cc',
       '$CHROME_DIR/browser/importer/importer_unittest.cc',