Skip to content

GitLab

T
tangled
Commit f6cb1e07 authored by rolandsteiner@chromium.org's avatar rolandsteiner@chromium.org
Browse files
Options

Revert 30168 - Commit patch set from http://codereview.chromium.org/149796 

(see discussion and history there)

BUG=10876
TEST=FilePathTest.MatchesExtension.CompareIgnoreCase


TBR=rolandsteiner@chromium.org
Review URL: http://codereview.chromium.org/337042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30170 0039d316-1c4b-4281-b951-d872f2087c98
parent 47f782c2
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 813 deletions
base/file_path.cc
View file @ f6cb1e07
This diff is collapsed.
base/file_path.h
View file @ f6cb1e07
......@@ -294,41 +294,6 @@ class FilePath {
// TODO(port): remove these functions.
static FilePath FromWStringHack(const std::wstring& wstring);
// Compare two strings in the same way the file system does.
// Note that these always ignore case, even on file systems that are case-
// sensitive. If case-sensitive comparison is ever needed, add corresponding
// methods here.
// The methods are written as a static method so that they can also be used
// on parts of a file path, e.g., just the extension.
// CompareIgnoreCase() returns -1, 0 or 1 for less-than, equal-to and
// greater-than respectively.
static int CompareIgnoreCase(const StringType& string1,
const StringType& string2);
static bool CompareEqualIgnoreCase(const StringType& string1,
const StringType& string2) {
return CompareIgnoreCase(string1, string2) == 0;
}
static bool CompareLessIgnoreCase(const StringType& string1,
const StringType& string2) {
return CompareIgnoreCase(string1, string2) < 0;
}
#if defined(OS_MACOSX)
// Returns the string in the special canonical decomposed form as defined for
// HFS, which is close to, but not quite, decomposition form D. See
// http://developer.apple.com/mac/library/technotes/tn/tn1150.html#UnicodeSubtleties
// for further comments.
// Returns the epmty string if the conversion failed.
static StringType GetHFSDecomposedForm(const FilePath::StringType& string);
// Special UTF-8 version of FastUnicodeCompare. Cf:
// http://developer.apple.com/mac/library/technotes/tn/tn1150.html#StringComparisonAlgorithm
// IMPORTANT: The input strings must be in the special HFS decomposed form!
// (cf. above GetHFSDecomposedForm method)
static int HFSFastUnicodeCompare(const StringType& string1,
const StringType& string2);
#endif
// Older Chromium code assumes that paths are always wstrings.
// This function produces a wstring from a FilePath, and is useful to smooth
// porting that old code to the FilePath API.
......
base/file_path_unittest.cc
View file @ f6cb1e07
......@@ -32,11 +32,6 @@ struct BinaryBooleanTestData {
bool expected;
};
struct BinaryIntTestData {
const FilePath::CharType* inputs[2];
int expected;
};
// file_util winds up using autoreleased objects on the Mac, so this needs
// to be a PlatformTest
class FilePathTest : public PlatformTest {
......@@ -946,38 +941,24 @@ TEST_F(FilePathTest, ReplaceExtension) {
TEST_F(FilePathTest, MatchesExtension) {
const struct BinaryBooleanTestData cases[] = {
{ { FPL("foo"), FPL("") }, true},
{ { FPL("foo"), FPL(".") }, false},
{ { FPL("foo."), FPL("") }, false},
{ { FPL("foo."), FPL(".") }, true},
{ { FPL("foo.txt"), FPL(".dll") }, false},
{ { FPL("foo.txt"), FPL(".txt") }, true},
{ { FPL("foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("foo.txt.dll"), FPL(".dll") }, true},
{ { FPL("foo.TXT"), FPL(".txt") }, true},
{ { FPL("foo.txt"), FPL(".TXT") }, true},
{ { FPL("foo.tXt"), FPL(".txt") }, true},
{ { FPL("foo.txt"), FPL(".tXt") }, true},
{ { FPL("foo.tXt"), FPL(".TXT") }, true},
{ { FPL("foo.tXt"), FPL(".tXt") }, true},
{ { FPL("foo"), FPL("") }, true},
{ { FPL("foo"), FPL(".") }, false},
{ { FPL("foo."), FPL("") }, false},
{ { FPL("foo."), FPL(".") }, true},
{ { FPL("foo.txt"), FPL(".dll") }, false},
{ { FPL("foo.txt"), FPL(".txt") }, true},
{ { FPL("foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("foo.txt.dll"), FPL(".dll") }, true},
#if defined(FILE_PATH_USES_DRIVE_LETTERS)
{ { FPL("c:/foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("c:/foo.txt"), FPL(".txt") }, true},
{ { FPL("c:/foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("c:/foo.txt"), FPL(".txt") }, true},
#endif // FILE_PATH_USES_DRIVE_LETTERS
#if defined(FILE_PATH_USES_WIN_SEPARATORS)
{ { FPL("c:\\bar\\foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("c:\\bar\\foo.txt"), FPL(".txt") }, true},
{ { FPL("c:\\bar\\foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("c:\\bar\\foo.txt"), FPL(".txt") }, true},
#endif // FILE_PATH_USES_DRIVE_LETTERS
{ { FPL("/bar/foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("/bar/foo.txt"), FPL(".txt") }, true},
#if defined(OS_WIN) || defined(OS_MACOSX)
// Umlauts A, O, U: direct comparison, and upper case vs. lower case
{ { FPL("foo.\u00E4\u00F6\u00FC"), FPL(".\u00E4\u00F6\u00FC") }, true},
{ { FPL("foo.\u00C4\u00D6\u00DC"), FPL(".\u00E4\u00F6\u00FC") }, true},
// C with circumflex: direct comparison, and upper case vs. lower case
{ { FPL("foo.\u0109"), FPL(".\u0109") }, true},
{ { FPL("foo.\u0108"), FPL(".\u0109") }, true},
#endif
{ { FPL("/bar/foo.txt.dll"), FPL(".txt") }, false},
{ { FPL("/bar/foo.txt"), FPL(".txt") }, true},
};
for (size_t i = 0; i < arraysize(cases); ++i) {
......@@ -989,83 +970,6 @@ TEST_F(FilePathTest, MatchesExtension) {
}
}
TEST_F(FilePathTest, CompareIgnoreCase) {
const struct BinaryIntTestData cases[] = {
{ { FPL("foo"), FPL("foo") }, 0},
{ { FPL("FOO"), FPL("foo") }, 0},
{ { FPL("foo.ext"), FPL("foo.ext") }, 0},
{ { FPL("FOO.EXT"), FPL("foo.ext") }, 0},
{ { FPL("Foo.Ext"), FPL("foo.ext") }, 0},
{ { FPL("foO"), FPL("foo") }, 0},
{ { FPL("foo"), FPL("foO") }, 0},
{ { FPL("fOo"), FPL("foo") }, 0},
{ { FPL("foo"), FPL("fOo") }, 0},
{ { FPL("bar"), FPL("foo") }, -1},
{ { FPL("foo"), FPL("bar") }, 1},
{ { FPL("BAR"), FPL("foo") }, -1},
{ { FPL("FOO"), FPL("bar") }, 1},
{ { FPL("bar"), FPL("FOO") }, -1},
{ { FPL("foo"), FPL("BAR") }, 1},
{ { FPL("BAR"), FPL("FOO") }, -1},
{ { FPL("FOO"), FPL("BAR") }, 1},
// German "Eszett" (lower case and the new-fangled upper case)
// Note that uc(<lowercase eszett>) => "SS", NOT <uppercase eszett>!
// However, neither Windows nor Mac OSX converts these.
// (or even have glyphs for <uppercase eszett>)
{ { FPL("\u00DF"), FPL("\u00DF") }, 0},
{ { FPL("\u1E9E"), FPL("\u1E9E") }, 0},
{ { FPL("\u00DF"), FPL("\u1E9E") }, -1},
{ { FPL("SS"), FPL("\u00DF") }, -1},
{ { FPL("SS"), FPL("\u1E9E") }, -1},
#if defined(OS_WIN) || defined(OS_MACOSX)
// Umlauts A, O, U: direct comparison, and upper case vs. lower case
{ { FPL("\u00E4\u00F6\u00FC"), FPL("\u00E4\u00F6\u00FC") }, 0},
{ { FPL("\u00C4\u00D6\u00DC"), FPL("\u00E4\u00F6\u00FC") }, 0},
// C with circumflex: direct comparison, and upper case vs. lower case
{ { FPL("\u0109"), FPL("\u0109") }, 0},
{ { FPL("\u0108"), FPL("\u0109") }, 0},
// Cyrillic letter SHA: direct comparison, and upper case vs. lower case
{ { FPL("\u0428"), FPL("\u0428") }, 0},
{ { FPL("\u0428"), FPL("\u0448") }, 0},
// Greek letter DELTA: direct comparison, and upper case vs. lower case
{ { FPL("\u0394"), FPL("\u0394") }, 0},
{ { FPL("\u0394"), FPL("\u03B4") }, 0},
// Japanese full-width A: direct comparison, and upper case vs. lower case
// Note that full-width and standard characters are considered different.
{ { FPL("\uFF21"), FPL("\uFF21") }, 0},
{ { FPL("\uFF21"), FPL("\uFF41") }, 0},
{ { FPL("A"), FPL("\uFF21") }, -1},
{ { FPL("A"), FPL("\uFF41") }, -1},
{ { FPL("a"), FPL("\uFF21") }, -1},
{ { FPL("a"), FPL("\uFF41") }, -1},
#endif
#if defined(OS_MACOSX)
// Codepoints > 0x1000
// Georgian letter DON: direct comparison, and upper case vs. lower case
{ { FPL("\u10A3"), FPL("\u10A3") }, 0},
{ { FPL("\u10A3"), FPL("\u10D3") }, 0},
// Combining characters vs. pre-composed characters, upper and lower case
{ { FPL("k\u0301u\u032Do\u0304\u0301n"), FPL("\u1E31\u1E77\u1E53n") }, 0},
{ { FPL("k\u0301u\u032Do\u0304\u0301n"), FPL("kuon") }, 1},
{ { FPL("kuon"), FPL("k\u0301u\u032Do\u0304\u0301n") }, -1},
{ { FPL("K\u0301U\u032DO\u0304\u0301N"), FPL("KUON") }, 1},
{ { FPL("KUON"), FPL("K\u0301U\u032DO\u0304\u0301N") }, -1},
{ { FPL("k\u0301u\u032Do\u0304\u0301n"), FPL("KUON") }, 1},
{ { FPL("K\u0301U\u032DO\u0304\u0301N"), FPL("\u1E31\u1E77\u1E53n") }, 0},
{ { FPL("k\u0301u\u032Do\u0304\u0301n"), FPL("\u1E30\u1E76\u1E52n") }, 0},
{ { FPL("k\u0301u\u032Do\u0304\u0302n"), FPL("\u1E30\u1E76\u1E52n") }, 1},
#endif
};
for (size_t i = 0; i < arraysize(cases); ++i) {
FilePath::StringType s1(cases[i].inputs[0]);
FilePath::StringType s2(cases[i].inputs[1]);
int result = FilePath::CompareIgnoreCase(s1, s2);
EXPECT_EQ(cases[i].expected, result) <<
"i: " << i << ", s1: " << s1 << ", s2: " << s2;
}
}
TEST_F(FilePathTest, ReferencesParent) {
const struct UnaryBooleanTestData cases[] = {
{ FPL("."), false },
......@@ -1096,4 +1000,3 @@ TEST_F(FilePathTest, ReferencesParent) {
"i: " << i << ", input: " << input.value();
}
}
chrome/browser/download/download_manager.cc
View file @ f6cb1e07
......@@ -526,8 +526,8 @@ bool DownloadManager::Init(Profile* profile) {
std::vector<std::wstring> extensions;
SplitString(extensions_to_open, L':', &extensions);
for (size_t i = 0; i < extensions.size(); ++i) {
if (!extensions[i].empty() && !IsExecutableFile(
FilePath::FromWStringHack(extensions[i])))
if (!extensions[i].empty() && !IsExecutable(
FilePath::FromWStringHack(extensions[i]).value()))
auto_open_.insert(FilePath::FromWStringHack(extensions[i]).value());
}
......@@ -857,13 +857,19 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
if (it != dangerous_finished_.end())
dangerous_finished_.erase(it);
// Open the download if the user or user prefs indicate it should be.
FilePath::StringType extension = download->full_path().Extension();
// Drop the leading period. (The auto-open list is period-less.)
if (extension.size() > 0)
extension = extension.substr(1);
// Handle chrome extensions explicitly and skip the shell execute.
if (IsExtensionInstall(download)) {
OpenChromeExtension(download->full_path(), download->url(),
download->referrer_url());
download->set_auto_opened(true);
} else if (download->open_when_complete() ||
ShouldOpenFileBasedOnExtension(download->full_path())) {
ShouldOpenFileExtension(extension)) {
OpenDownloadInShell(download, NULL);
download->set_auto_opened(true);
}
......@@ -872,7 +878,6 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
// state to complete but did not notify).
download->UpdateObservers();
}
// Called on the file thread. Renames the downloaded file to its original name.
void DownloadManager::ProceedWithFinishedDangerousDownload(
int64 download_handle,
......@@ -999,7 +1004,11 @@ void DownloadManager::OnPauseDownloadRequest(ResourceDispatcherHost* rdh,
bool DownloadManager::IsDangerous(const FilePath& file_name) {
// TODO(jcampan): Improve me.
return IsExecutableFile(file_name);
FilePath::StringType extension = file_name.Extension();
// Drop the leading period.
if (extension.size() > 0)
extension = extension.substr(1);
return IsExecutable(extension);
}
void DownloadManager::RenameDownload(DownloadItem* download,
......@@ -1146,7 +1155,7 @@ void DownloadManager::GenerateExtension(
return;
}
if (IsExecutableExtension(extension) && !IsExecutableMimeType(mime_type)) {
if (IsExecutable(extension) && !IsExecutableMimeType(mime_type)) {
// We want to be careful about executable extensions. The worry here is
// that a trusted web site could be tricked into dropping an executable file
// on the user's filesystem.
......@@ -1174,7 +1183,7 @@ void DownloadManager::GenerateExtension(
if (net::GetPreferredExtensionForMimeType(mime_type, &append_extension)) {
if (append_extension != FILE_PATH_LITERAL("txt") &&
append_extension != extension &&
!IsExecutableExtension(append_extension) &&
!IsExecutable(append_extension) &&
!(append_extension == FILE_PATH_LITERAL("gz") &&
extension == FILE_PATH_LITERAL("tgz")) &&
(append_extension != FILE_PATH_LITERAL("tar") ||
......@@ -1273,32 +1282,21 @@ void DownloadManager::OpenDownloadInShell(const DownloadItem* download,
#endif
}
void DownloadManager::OpenFilesBasedOnExtension(
const FilePath& path, bool open) {
FilePath::StringType extension = path.Extension();
if (extension.empty())
return;
DCHECK(extension[0] == FilePath::kExtensionSeparator);
extension.erase(0, 1);
if (open && !IsExecutableExtension(extension))
void DownloadManager::OpenFilesOfExtension(
const FilePath::StringType& extension, bool open) {
if (open && !IsExecutable(extension))
auto_open_.insert(extension);
else
auto_open_.erase(extension);
SaveAutoOpens();
}
bool DownloadManager::ShouldOpenFileBasedOnExtension(
const FilePath& path) const {
bool DownloadManager::ShouldOpenFileExtension(
const FilePath::StringType& extension) {
// Special-case Chrome extensions as always-open.
FilePath::StringType extension = path.Extension();
if (extension.empty())
return false;
if (IsExecutableExtension(extension))
return false;
DCHECK(extension[0] == FilePath::kExtensionSeparator);
extension.erase(0, 1);
if (auto_open_.find(extension) != auto_open_.end() ||
Extension::IsExtension(path))
if (!IsExecutable(extension) &&
(auto_open_.find(extension) != auto_open_.end() ||
Extension::IsExtension(FilePath(extension))))
return true;
return false;
}
......@@ -1335,14 +1333,7 @@ bool DownloadManager::IsExecutableMimeType(const std::string& mime_type) {
return net::MatchesMimeType("application/*", mime_type);
}
bool DownloadManager::IsExecutableFile(const FilePath& path) const {
return IsExecutableExtension(path.Extension());
}
bool DownloadManager::IsExecutableExtension(
const FilePath::StringType& extension) const {
if (extension.empty())
return false;
bool DownloadManager::IsExecutable(const FilePath::StringType& extension) {
if (!IsStringASCII(extension))
return false;
#if defined(OS_WIN)
......@@ -1352,10 +1343,6 @@ bool DownloadManager::IsExecutableExtension(
#endif
StringToLowerASCII(&ascii_extension);
// Strip out leading dot if it's still there
if (ascii_extension[0] == FilePath::kExtensionSeparator)
ascii_extension.erase(0, 1);
return exe_types_.find(ascii_extension) != exe_types_.end();
}
......@@ -1372,7 +1359,7 @@ void DownloadManager::SaveAutoOpens() {
PrefService* prefs = profile_->GetPrefs();
if (prefs) {
FilePath::StringType extensions;
for (AutoOpenSet::iterator it = auto_open_.begin();
for (std::set<FilePath::StringType>::iterator it = auto_open_.begin();
it != auto_open_.end(); ++it) {
extensions += *it + FILE_PATH_LITERAL(":");
}
......
chrome/browser/download/download_manager.h
View file @ f6cb1e07
......@@ -419,19 +419,16 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
// Registers this file extension for automatic opening upon download
// completion if 'open' is true, or prevents the extension from automatic
// opening if 'open' is false.
void OpenFilesBasedOnExtension(const FilePath& path, bool open);
void OpenFilesOfExtension(const FilePath::StringType& extension, bool open);
// Tests if a file type should be opened automatically.
bool ShouldOpenFileBasedOnExtension(const FilePath& path) const;
bool ShouldOpenFileExtension(const FilePath::StringType& extension);
// Tests if we think the server means for this mime_type to be executable.
static bool IsExecutableMimeType(const std::string& mime_type);
// Tests if a file is considered executable, based on its type.
bool IsExecutableFile(const FilePath& path) const;
// Tests if a file type is considered executable.
bool IsExecutableExtension(const FilePath::StringType& extension) const;
bool IsExecutable(const FilePath::StringType& extension);
// Resets the automatic open preference.
void ResetAutoOpenFiles();
......@@ -608,14 +605,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
FilePath last_download_path_;
// Set of file extensions to open at download completion.
struct AutoOpenCompareFunctor {
inline bool operator()(const FilePath::StringType& a,
const FilePath::StringType& b) const {
return FilePath::CompareLessIgnoreCase(a, b);
}
};
typedef std::set<FilePath::StringType, AutoOpenCompareFunctor> AutoOpenSet;
AutoOpenSet auto_open_;
std::set<FilePath::StringType> auto_open_;
// Set of file extensions that are executables and shouldn't be auto opened.
std::set<std::string> exe_types_;
......
chrome/browser/download/download_shelf.cc
View file @ f6cb1e07
......@@ -32,8 +32,9 @@ bool DownloadShelfContextMenu::ItemIsChecked(int id) const {
return download_->open_when_complete();
}
case ALWAYS_OPEN_TYPE: {
return download_->manager()->ShouldOpenFileBasedOnExtension(
download_->full_path());
const FilePath::StringType extension =
file_util::GetFileExtensionFromPath(download_->full_path());
return download_->manager()->ShouldOpenFileExtension(extension);
}
case TOGGLE_PAUSE: {
return download_->is_paused();
......@@ -95,8 +96,10 @@ void DownloadShelfContextMenu::ExecuteItemCommand(int id) {
download_util::OpenDownload(download_);
break;
case ALWAYS_OPEN_TYPE: {
download_->manager()->OpenFilesBasedOnExtension(
download_->full_path(), !ItemIsChecked(ALWAYS_OPEN_TYPE));
const FilePath::StringType extension =
file_util::GetFileExtensionFromPath(download_->full_path());
download_->manager()->OpenFilesOfExtension(
extension, !ItemIsChecked(ALWAYS_OPEN_TYPE));
break;
}
case CANCEL:
......
chrome/browser/download/download_util.cc
View file @ f6cb1e07
......@@ -53,7 +53,9 @@ bool CanOpenDownload(DownloadItem* download) {
if (!download->original_name().value().empty())
file_to_use = download->original_name();
return !download->manager()->IsExecutableFile(file_to_use);
const FilePath::StringType extension =
file_util::GetFileExtensionFromPath(file_to_use);
return !download->manager()->IsExecutable(extension);
}
void OpenDownload(DownloadItem* download) {
......
chrome/browser/net/url_fixer_upper_unittest.cc
View file @ f6cb1e07
......@@ -196,8 +196,12 @@ static bool IsMatchingFileURL(const std::string& url,
FilePath derived_path;
net::FileURLToFilePath(GURL(url), &derived_path);
return FilePath::CompareEqualIgnoreCase(derived_path.value(),
full_file_path.value());
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 {
......
chrome/browser/shell_integration_win.cc
View file @ f6cb1e07
......@@ -115,7 +115,11 @@ ShellIntegration::DefaultBrowserState ShellIntegration::IsDefaultBrowser() {
std::wstring short_path;
GetShortPathName(command_line.program().c_str(),
WriteInto(&short_path, MAX_PATH), MAX_PATH);
if (!FilePath::CompareEqualIgnoreCase(short_path, short_app_path))
if ((short_path.size() != short_app_path.size()) ||
(!std::equal(short_path.begin(),
short_path.end(),
short_app_path.begin(),
CaseInsensitiveCompare<wchar_t>())))
return NOT_DEFAULT_BROWSER;
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment