Write new Starts/EndsWith and convert FilePath functions to StringPiece.
When I moved StartsWith and EndsWith to the base namespace it became apparent that the bool parameter was frequently a source of confusion, and that the second argument is almost always a constant that we then convert to a string.
This adds new versions of StartsWith/EndsWith that takes an enum for case sensitivity and string pieces for the string inputs.
The existing functions used a locale-dependent case insensitive comparison that is marked with an old bug number that such comparisons are probably wrong. With this change, I moved the case insensitive comparisons to ASCII ones.
Only callers in base are updated. The rest of the calls go through a compatibility layer.
The compatibility layer keeps the local-dependent compares for the 16-bit string comparisons to avoid breaking things subtly. There are relatively few calls to the 16-bit version, and most use constants for the prefix/suffix (which wouldn't be affected by the locale), so there should be few callers to audit in a later pass to go to pure ASCII comparisons.
The 8-bit versions now always use the ASCII case-insensitive comparisons. This should be the only change in this patch that can affect the behavior of the program. Code doing locale-dependent 8-bit tolower() calls (what the old code ended up doing) is almost certainly wrong. UTF-8 strings will be mangled. The only 8-bit non-UTF-8 strings we have are typically Posix file paths and Posix file paths are case-sensitive. I'm not very concerned about regressions from this change.
Use StringPiece in FilePath for input arguments. This wasn't done before because we had no StringPiece16 until more recently. Constants are frequently passed as input to some of the functions, especially Append, so this will save some string allocations. Unfortunately, this is more likely to affect unit tests that the real browser. This is combined with the StartsWith/EndsWith changes because I started changing FilePath when updating its use of StartsWith.
The FilePath changes should have no observable effect on the product.
BUG=24917
Review URL: https://codereview.chromium.org/1182453004
Cr-Commit-Position: refs/heads/master@{#334555}
CrOS-Libchrome-Original-Commit: 89365dc80aa9ea343cea0fe48fdeb702b23726a2
diff --git a/base/files/file_path.cc b/base/files/file_path.cc
index de8927a..9212353 100644
--- a/base/files/file_path.cc
+++ b/base/files/file_path.cc
@@ -31,7 +31,8 @@
namespace base {
-typedef FilePath::StringType StringType;
+using StringType = FilePath::StringType;
+using StringPieceType = FilePath::StringPieceType;
namespace {
@@ -45,7 +46,7 @@
// otherwise returns npos. This can only be true on Windows, when a pathname
// begins with a letter followed by a colon. On other platforms, this always
// returns npos.
-StringType::size_type FindDriveLetter(const StringType& path) {
+StringPieceType::size_type FindDriveLetter(StringPieceType path) {
#if defined(FILE_PATH_USES_DRIVE_LETTERS)
// This is dependent on an ASCII-based character set, but that's a
// reasonable assumption. iswalpha can be too inclusive here.
@@ -59,26 +60,25 @@
}
#if defined(FILE_PATH_USES_DRIVE_LETTERS)
-bool EqualDriveLetterCaseInsensitive(const StringType& a,
- const StringType& b) {
+bool EqualDriveLetterCaseInsensitive(StringPieceType a, StringPieceType b) {
size_t a_letter_pos = FindDriveLetter(a);
size_t b_letter_pos = FindDriveLetter(b);
if (a_letter_pos == StringType::npos || b_letter_pos == StringType::npos)
return a == b;
- StringType a_letter(a.substr(0, a_letter_pos + 1));
- StringType b_letter(b.substr(0, b_letter_pos + 1));
- if (!StartsWith(a_letter, b_letter, false))
+ StringPieceType a_letter(a.substr(0, a_letter_pos + 1));
+ StringPieceType b_letter(b.substr(0, b_letter_pos + 1));
+ if (!StartsWith(a_letter, b_letter, CompareCase::INSENSITIVE_ASCII))
return false;
- StringType a_rest(a.substr(a_letter_pos + 1));
- StringType b_rest(b.substr(b_letter_pos + 1));
+ StringPieceType a_rest(a.substr(a_letter_pos + 1));
+ StringPieceType b_rest(b.substr(b_letter_pos + 1));
return a_rest == b_rest;
}
#endif // defined(FILE_PATH_USES_DRIVE_LETTERS)
-bool IsPathAbsolute(const StringType& path) {
+bool IsPathAbsolute(StringPieceType path) {
#if defined(FILE_PATH_USES_DRIVE_LETTERS)
StringType::size_type letter = FindDriveLetter(path);
if (letter != StringType::npos) {
@@ -177,7 +177,8 @@
FilePath::FilePath(const FilePath& that) : path_(that.path_) {
}
-FilePath::FilePath(const StringType& path) : path_(path) {
+FilePath::FilePath(StringPieceType path) {
+ path.CopyToString(&path_);
StringType::size_type nul_pos = path_.find(kStringTerminator);
if (nul_pos != StringType::npos)
path_.erase(nul_pos, StringType::npos);
@@ -279,7 +280,7 @@
// never case sensitive.
if ((FindDriveLetter(*parent_comp) != StringType::npos) &&
(FindDriveLetter(*child_comp) != StringType::npos)) {
- if (!StartsWith(*parent_comp, *child_comp, false))
+ if (!StartsWith(*parent_comp, *child_comp, CompareCase::INSENSITIVE_ASCII))
return false;
++parent_comp;
++child_comp;
@@ -404,7 +405,7 @@
return FilePath(path_.substr(0, dot));
}
-FilePath FilePath::InsertBeforeExtension(const StringType& suffix) const {
+FilePath FilePath::InsertBeforeExtension(StringPieceType suffix) const {
if (suffix.empty())
return FilePath(path_);
@@ -413,27 +414,28 @@
StringType ext = Extension();
StringType ret = RemoveExtension().value();
- ret.append(suffix);
+ suffix.AppendToString(&ret);
ret.append(ext);
return FilePath(ret);
}
-FilePath FilePath::InsertBeforeExtensionASCII(const StringPiece& suffix)
+FilePath FilePath::InsertBeforeExtensionASCII(StringPiece suffix)
const {
DCHECK(IsStringASCII(suffix));
#if defined(OS_WIN)
- return InsertBeforeExtension(ASCIIToUTF16(suffix.as_string()));
+ return InsertBeforeExtension(ASCIIToUTF16(suffix));
#elif defined(OS_POSIX)
- return InsertBeforeExtension(suffix.as_string());
+ return InsertBeforeExtension(suffix);
#endif
}
-FilePath FilePath::AddExtension(const StringType& extension) const {
+FilePath FilePath::AddExtension(StringPieceType extension) const {
if (IsEmptyOrSpecialCase(BaseName().value()))
return FilePath();
// If the new extension is "" or ".", then just return the current FilePath.
- if (extension.empty() || extension == StringType(1, kExtensionSeparator))
+ if (extension.empty() ||
+ (extension.size() == 1 && extension[0] == kExtensionSeparator))
return *this;
StringType str = path_;
@@ -441,27 +443,28 @@
*(str.end() - 1) != kExtensionSeparator) {
str.append(1, kExtensionSeparator);
}
- str.append(extension);
+ extension.AppendToString(&str);
return FilePath(str);
}
-FilePath FilePath::ReplaceExtension(const StringType& extension) const {
+FilePath FilePath::ReplaceExtension(StringPieceType extension) const {
if (IsEmptyOrSpecialCase(BaseName().value()))
return FilePath();
FilePath no_ext = RemoveExtension();
// If the new extension is "" or ".", then just remove the current extension.
- if (extension.empty() || extension == StringType(1, kExtensionSeparator))
+ if (extension.empty() ||
+ (extension.size() == 1 && extension[0] == kExtensionSeparator))
return no_ext;
StringType str = no_ext.value();
if (extension[0] != kExtensionSeparator)
str.append(1, kExtensionSeparator);
- str.append(extension);
+ extension.AppendToString(&str);
return FilePath(str);
}
-bool FilePath::MatchesExtension(const StringType& extension) const {
+bool FilePath::MatchesExtension(StringPieceType extension) const {
DCHECK(extension.empty() || extension[0] == kExtensionSeparator);
StringType current_extension = Extension();
@@ -472,17 +475,17 @@
return FilePath::CompareEqualIgnoreCase(extension, current_extension);
}
-FilePath FilePath::Append(const StringType& component) const {
- const StringType* appended = &component;
+FilePath FilePath::Append(StringPieceType component) const {
+ StringPieceType appended = component;
StringType without_nuls;
StringType::size_type nul_pos = component.find(kStringTerminator);
- if (nul_pos != StringType::npos) {
- without_nuls = component.substr(0, nul_pos);
- appended = &without_nuls;
+ if (nul_pos != StringPieceType::npos) {
+ component.substr(0, nul_pos).CopyToString(&without_nuls);
+ appended = StringPieceType(without_nuls);
}
- DCHECK(!IsPathAbsolute(*appended));
+ DCHECK(!IsPathAbsolute(appended));
if (path_.compare(kCurrentDirectory) == 0) {
// Append normally doesn't do any normalization, but as a special case,
@@ -492,7 +495,7 @@
// it's likely in practice to wind up with FilePath objects containing
// only kCurrentDirectory when calling DirName on a single relative path
// component.
- return FilePath(*appended);
+ return FilePath(appended);
}
FilePath new_path(path_);
@@ -501,7 +504,7 @@
// Don't append a separator if the path is empty (indicating the current
// directory) or if the path component is empty (indicating nothing to
// append).
- if (appended->length() > 0 && new_path.path_.length() > 0) {
+ if (appended.length() > 0 && new_path.path_.length() > 0) {
// Don't append a separator if the path still ends with a trailing
// separator after stripping (indicating the root directory).
if (!IsSeparator(new_path.path_[new_path.path_.length() - 1])) {
@@ -512,7 +515,7 @@
}
}
- new_path.path_.append(*appended);
+ appended.AppendToString(&new_path.path_);
return new_path;
}
@@ -520,12 +523,12 @@
return Append(component.value());
}
-FilePath FilePath::AppendASCII(const StringPiece& component) const {
+FilePath FilePath::AppendASCII(StringPiece component) const {
DCHECK(base::IsStringASCII(component));
#if defined(OS_WIN)
- return Append(ASCIIToUTF16(component.as_string()));
+ return Append(ASCIIToUTF16(component));
#elif defined(OS_POSIX)
- return Append(component.as_string());
+ return Append(component);
#endif
}
@@ -680,17 +683,17 @@
}
#if defined(OS_WIN)
-// Windows specific implementation of file string comparisons
+// Windows specific implementation of file string comparisons.
-int FilePath::CompareIgnoreCase(const StringType& string1,
- const StringType& string2) {
+int FilePath::CompareIgnoreCase(StringPieceType string1,
+ StringPieceType string2) {
// Perform character-wise upper case comparison rather than using the
// fully Unicode-aware CompareString(). For details see:
// http://blogs.msdn.com/michkap/archive/2005/10/17/481600.aspx
- StringType::const_iterator i1 = string1.begin();
- StringType::const_iterator i2 = string2.begin();
- StringType::const_iterator string1end = string1.end();
- StringType::const_iterator string2end = string2.end();
+ StringPieceType::const_iterator i1 = string1.begin();
+ StringPieceType::const_iterator i2 = string2.begin();
+ StringPieceType::const_iterator string1end = string1.end();
+ StringPieceType::const_iterator string2end = string2.end();
for ( ; i1 != string1end && i2 != string2end; ++i1, ++i2) {
wchar_t c1 =
(wchar_t)LOWORD(::CharUpperW((LPWSTR)(DWORD_PTR)MAKELONG(*i1, 0)));
@@ -709,7 +712,7 @@
}
#elif defined(OS_MACOSX)
-// Mac OS X specific implementation of file string comparisons
+// Mac OS X specific implementation of file string comparisons.
// cf. http://developer.apple.com/mac/library/technotes/tn/tn1150.html#UnicodeSubtleties
//
@@ -1153,23 +1156,23 @@
return codepoint;
}
-} // anonymous namespace
+} // namespace
// Special UTF-8 version of FastUnicodeCompare. Cf:
// http://developer.apple.com/mac/library/technotes/tn/tn1150.html#StringComparisonAlgorithm
// The input strings must be in the special HFS decomposed form.
-int FilePath::HFSFastUnicodeCompare(const StringType& string1,
- const StringType& string2) {
+int FilePath::HFSFastUnicodeCompare(StringPieceType string1,
+ StringPieceType string2) {
int length1 = string1.length();
int length2 = string2.length();
int index1 = 0;
int index2 = 0;
for (;;) {
- int codepoint1 = HFSReadNextNonIgnorableCodepoint(string1.c_str(),
+ int codepoint1 = HFSReadNextNonIgnorableCodepoint(string1.data(),
length1,
&index1);
- int codepoint2 = HFSReadNextNonIgnorableCodepoint(string2.c_str(),
+ int codepoint2 = HFSReadNextNonIgnorableCodepoint(string2.data(),
length2,
&index2);
if (codepoint1 != codepoint2)
@@ -1182,11 +1185,11 @@
}
}
-StringType FilePath::GetHFSDecomposedForm(const StringType& string) {
+StringType FilePath::GetHFSDecomposedForm(StringPieceType string) {
ScopedCFTypeRef<CFStringRef> cfstring(
CFStringCreateWithBytesNoCopy(
NULL,
- reinterpret_cast<const UInt8*>(string.c_str()),
+ reinterpret_cast<const UInt8*>(string.data()),
string.length(),
kCFStringEncodingUTF8,
false,
@@ -1215,8 +1218,8 @@
return result;
}
-int FilePath::CompareIgnoreCase(const StringType& string1,
- const StringType& string2) {
+int FilePath::CompareIgnoreCase(StringPieceType string1,
+ StringPieceType string2) {
// Quick checks for empty strings - these speed things up a bit and make the
// following code cleaner.
if (string1.empty())
@@ -1233,7 +1236,7 @@
ScopedCFTypeRef<CFStringRef> cfstring1(
CFStringCreateWithBytesNoCopy(
NULL,
- reinterpret_cast<const UInt8*>(string1.c_str()),
+ reinterpret_cast<const UInt8*>(string1.data()),
string1.length(),
kCFStringEncodingUTF8,
false,
@@ -1241,7 +1244,7 @@
ScopedCFTypeRef<CFStringRef> cfstring2(
CFStringCreateWithBytesNoCopy(
NULL,
- reinterpret_cast<const UInt8*>(string2.c_str()),
+ reinterpret_cast<const UInt8*>(string2.data()),
string2.length(),
kCFStringEncodingUTF8,
false,
@@ -1258,9 +1261,9 @@
// Generic (POSIX) implementation of file string comparison.
// TODO(rolandsteiner) check if this is sufficient/correct.
-int FilePath::CompareIgnoreCase(const StringType& string1,
- const StringType& string2) {
- int comparison = strcasecmp(string1.c_str(), string2.c_str());
+int FilePath::CompareIgnoreCase(StringPieceType string1,
+ StringPieceType string2) {
+ int comparison = strcasecmp(string1.data(), string2.data());
if (comparison < 0)
return -1;
if (comparison > 0)
diff --git a/base/files/file_path.h b/base/files/file_path.h
index 0c84af6..25b8391 100644
--- a/base/files/file_path.h
+++ b/base/files/file_path.h
@@ -112,7 +112,7 @@
#include "base/compiler_specific.h"
#include "base/containers/hash_tables.h"
#include "base/strings/string16.h"
-#include "base/strings/string_piece.h" // For implicit conversions.
+#include "base/strings/string_piece.h"
#include "build/build_config.h"
// Windows-style drive letter support and pathname separator characters can be
@@ -144,6 +144,7 @@
typedef std::wstring StringType;
#endif // OS_WIN
+ typedef BasicStringPiece<StringType> StringPieceType;
typedef StringType::value_type CharType;
// Null-terminated array of separators used to separate components in
@@ -166,7 +167,7 @@
FilePath();
FilePath(const FilePath& that);
- explicit FilePath(const StringType& path);
+ explicit FilePath(StringPieceType path);
~FilePath();
FilePath& operator=(const FilePath& that);
@@ -268,25 +269,23 @@
// path == "C:\pics\jojo" suffix == " (1)", returns "C:\pics\jojo (1)"
// path == "C:\pics.old\jojo" suffix == " (1)", returns "C:\pics.old\jojo (1)"
FilePath InsertBeforeExtension(
- const StringType& suffix) const WARN_UNUSED_RESULT;
+ StringPieceType suffix) const WARN_UNUSED_RESULT;
FilePath InsertBeforeExtensionASCII(
- const base::StringPiece& suffix) const WARN_UNUSED_RESULT;
+ StringPiece suffix) const WARN_UNUSED_RESULT;
// Adds |extension| to |file_name|. Returns the current FilePath if
// |extension| is empty. Returns "" if BaseName() == "." or "..".
- FilePath AddExtension(
- const StringType& extension) const WARN_UNUSED_RESULT;
+ FilePath AddExtension(StringPieceType extension) const WARN_UNUSED_RESULT;
// Replaces the extension of |file_name| with |extension|. If |file_name|
// does not have an extension, then |extension| is added. If |extension| is
// empty, then the extension is removed from |file_name|.
// Returns "" if BaseName() == "." or "..".
- FilePath ReplaceExtension(
- const StringType& extension) const WARN_UNUSED_RESULT;
+ FilePath ReplaceExtension(StringPieceType extension) const WARN_UNUSED_RESULT;
// Returns true if the file path matches the specified extension. The test is
// case insensitive. Don't forget the leading period if appropriate.
- bool MatchesExtension(const StringType& extension) const;
+ bool MatchesExtension(StringPieceType extension) const;
// Returns a FilePath by appending a separator and the supplied path
// component to this object's path. Append takes care to avoid adding
@@ -294,7 +293,7 @@
// If this object's path is kCurrentDirectory, a new FilePath corresponding
// only to |component| is returned. |component| must be a relative path;
// it is an error to pass an absolute path.
- FilePath Append(const StringType& component) const WARN_UNUSED_RESULT;
+ FilePath Append(StringPieceType component) const WARN_UNUSED_RESULT;
FilePath Append(const FilePath& component) const WARN_UNUSED_RESULT;
// Although Windows StringType is std::wstring, since the encoding it uses for
@@ -303,8 +302,7 @@
// On Linux, although it can use any 8-bit encoding for paths, we assume that
// ASCII is a valid subset, regardless of the encoding, since many operating
// system paths will always be ASCII.
- FilePath AppendASCII(const base::StringPiece& component)
- const WARN_UNUSED_RESULT;
+ FilePath AppendASCII(StringPiece component) const WARN_UNUSED_RESULT;
// Returns true if this FilePath contains an absolute path. On Windows, an
// absolute path begins with either a drive letter specification followed by
@@ -388,14 +386,14 @@
// 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) {
+ static int CompareIgnoreCase(StringPieceType string1,
+ StringPieceType string2);
+ static bool CompareEqualIgnoreCase(StringPieceType string1,
+ StringPieceType string2) {
return CompareIgnoreCase(string1, string2) == 0;
}
- static bool CompareLessIgnoreCase(const StringType& string1,
- const StringType& string2) {
+ static bool CompareLessIgnoreCase(StringPieceType string1,
+ StringPieceType string2) {
return CompareIgnoreCase(string1, string2) < 0;
}
@@ -405,14 +403,14 @@
// 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);
+ static StringType GetHFSDecomposedForm(StringPieceType 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);
+ static int HFSFastUnicodeCompare(StringPieceType string1,
+ StringPieceType string2);
#endif
#if defined(OS_ANDROID)