summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/download/save_package.cc7
-rw-r--r--chrome/browser/download/save_package.h4
-rw-r--r--chrome/browser/download/save_package_unittest.cc189
-rw-r--r--chrome/chrome.xcodeproj/project.pbxproj2
-rw-r--r--chrome/test/unit/unit_tests.scons1
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',
generated by cgit v1.1 at 2025-01-22 20:50:40 +0000