From 89365dc80aa9ea343cea0fe48fdeb702b23726a2 Mon Sep 17 00:00:00 2001 From: brettw Date: Mon, 15 Jun 2015 22:52:47 -0700 Subject: [PATCH] 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} --- base/files/file_path.cc | 119 ++++++++++++++++--------------- base/files/file_path.h | 40 +++++------ base/strings/string_util.cc | 122 ++++++++++++++++++++++++-------- base/strings/string_util.h | 62 +++++++++++----- base/sys_info.cc | 2 +- base/version.cc | 18 ++--- base/win/win_util.cc | 4 +- content/browser/download/save_package.h | 7 +- 8 files changed, 236 insertions(+), 138 deletions(-) diff --git a/base/files/file_path.cc b/base/files/file_path.cc index de8927ac5b27..92123533aaa5 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 @@ const FilePath::CharType kStringTerminator = FILE_PATH_LITERAL('\0'); // 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 @@ StringType::size_type FindDriveLetter(const StringType& path) { } #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() { 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 @@ bool FilePath::AppendRelativePath(const FilePath& child, // 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 @@ FilePath FilePath::RemoveFinalExtension() const { 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 @@ FilePath FilePath::InsertBeforeExtension(const StringType& suffix) const { 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 @@ FilePath FilePath::AddExtension(const StringType& extension) const { *(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 @@ bool FilePath::MatchesExtension(const StringType& extension) const { 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 @@ FilePath FilePath::Append(const StringType& component) const { // 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 @@ FilePath FilePath::Append(const StringType& component) const { // 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 @@ FilePath FilePath::Append(const StringType& component) const { } } - new_path.path_.append(*appended); + appended.AppendToString(&new_path.path_); return new_path; } @@ -520,12 +523,12 @@ FilePath FilePath::Append(const FilePath& component) const { 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 @@ bool FilePath::ReadFromPickle(PickleIterator* iter) { } #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 @@ int FilePath::CompareIgnoreCase(const StringType& string1, } #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 @@ inline int HFSReadNextNonIgnorableCodepoint(const char* string, 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 @@ int FilePath::HFSFastUnicodeCompare(const StringType& string1, } } -StringType FilePath::GetHFSDecomposedForm(const StringType& string) { +StringType FilePath::GetHFSDecomposedForm(StringPieceType string) { ScopedCFTypeRef cfstring( CFStringCreateWithBytesNoCopy( NULL, - reinterpret_cast(string.c_str()), + reinterpret_cast(string.data()), string.length(), kCFStringEncodingUTF8, false, @@ -1215,8 +1218,8 @@ StringType FilePath::GetHFSDecomposedForm(const StringType& string) { 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 @@ int FilePath::CompareIgnoreCase(const StringType& string1, ScopedCFTypeRef cfstring1( CFStringCreateWithBytesNoCopy( NULL, - reinterpret_cast(string1.c_str()), + reinterpret_cast(string1.data()), string1.length(), kCFStringEncodingUTF8, false, @@ -1241,7 +1244,7 @@ int FilePath::CompareIgnoreCase(const StringType& string1, ScopedCFTypeRef cfstring2( CFStringCreateWithBytesNoCopy( NULL, - reinterpret_cast(string2.c_str()), + reinterpret_cast(string2.data()), string2.length(), kCFStringEncodingUTF8, false, @@ -1258,9 +1261,9 @@ int FilePath::CompareIgnoreCase(const StringType& string1, // 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 0c84af6c8551..25b8391c0109 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 @@ class BASE_EXPORT FilePath { typedef std::wstring StringType; #endif // OS_WIN + typedef BasicStringPiece StringPieceType; typedef StringType::value_type CharType; // Null-terminated array of separators used to separate components in @@ -166,7 +167,7 @@ class BASE_EXPORT FilePath { FilePath(); FilePath(const FilePath& that); - explicit FilePath(const StringType& path); + explicit FilePath(StringPieceType path); ~FilePath(); FilePath& operator=(const FilePath& that); @@ -268,25 +269,23 @@ class BASE_EXPORT FilePath { // 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 @@ class BASE_EXPORT FilePath { // 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 @@ class BASE_EXPORT FilePath { // 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 @@ class BASE_EXPORT FilePath { // 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 @@ class BASE_EXPORT FilePath { // 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) diff --git a/base/strings/string_util.cc b/base/strings/string_util.cc index 738d32e0455e..9bbe1cb569a9 100644 --- a/base/strings/string_util.cc +++ b/base/strings/string_util.cc @@ -507,48 +507,114 @@ bool EqualsASCII(const string16& a, const StringPiece& b) { return std::equal(b.begin(), b.end(), a.begin()); } -bool StartsWithASCII(const std::string& str, - const std::string& search, - bool case_sensitive) { - if (case_sensitive) - return str.compare(0, search.length(), search) == 0; - else - return base::strncasecmp(str.c_str(), search.c_str(), search.length()) == 0; +template +bool StartsWithT(BasicStringPiece str, + BasicStringPiece search_for, + CompareCase case_sensitivity) { + if (search_for.size() > str.size()) + return false; + + BasicStringPiece source = str.substr(0, search_for.size()); + + switch (case_sensitivity) { + case CompareCase::SENSITIVE: + return source == search_for; + + case CompareCase::INSENSITIVE_ASCII: + return std::equal( + search_for.begin(), search_for.end(), + source.begin(), + base::CaseInsensitiveCompareASCII()); + + default: + NOTREACHED(); + return false; + } +} + +bool StartsWith(StringPiece str, + StringPiece search_for, + CompareCase case_sensitivity) { + return StartsWithT(str, search_for, case_sensitivity); +} + +bool StartsWith(StringPiece16 str, + StringPiece16 search_for, + CompareCase case_sensitivity) { + return StartsWithT(str, search_for, case_sensitivity); } bool StartsWith(const string16& str, const string16& search, bool case_sensitive) { - if (case_sensitive) { - return str.compare(0, search.length(), search) == 0; + if (!case_sensitive) { + // This function was originally written using the current locale functions + // for case-insensitive comparisons. Emulate this behavior until callers + // can be converted either to use the case-insensitive ASCII one (most + // callers) or ICU functions in base_i18n. + if (search.size() > str.size()) + return false; + return std::equal(search.begin(), search.end(), str.begin(), + CaseInsensitiveCompare()); } - if (search.size() > str.size()) - return false; - return std::equal(search.begin(), search.end(), str.begin(), - CaseInsensitiveCompare()); + return StartsWith(StringPiece16(str), StringPiece16(search), + CompareCase::SENSITIVE); } -template -bool EndsWithT(const STR& str, const STR& search, bool case_sensitive) { - size_t str_length = str.length(); - size_t search_length = search.length(); - if (search_length > str_length) +template +bool EndsWithT(BasicStringPiece str, + BasicStringPiece search_for, + CompareCase case_sensitivity) { + if (search_for.size() > str.size()) return false; - if (case_sensitive) - return str.compare(str_length - search_length, search_length, search) == 0; - return std::equal(search.begin(), search.end(), - str.begin() + (str_length - search_length), - base::CaseInsensitiveCompare()); + + BasicStringPiece source = str.substr(str.size() - search_for.size(), + search_for.size()); + + switch (case_sensitivity) { + case CompareCase::SENSITIVE: + return source == search_for; + + case CompareCase::INSENSITIVE_ASCII: + return std::equal( + source.begin(), source.end(), + search_for.begin(), + base::CaseInsensitiveCompareASCII()); + + default: + NOTREACHED(); + return false; + } } -bool EndsWith(const std::string& str, const std::string& search, - bool case_sensitive) { - return EndsWithT(str, search, case_sensitive); +bool EndsWith(StringPiece str, + StringPiece search_for, + CompareCase case_sensitivity) { + return EndsWithT(str, search_for, case_sensitivity); +} + +bool EndsWith(StringPiece16 str, + StringPiece16 search_for, + CompareCase case_sensitivity) { + return EndsWithT(str, search_for, case_sensitivity); } -bool EndsWith(const string16& str, const string16& search, +bool EndsWith(const string16& str, + const string16& search, bool case_sensitive) { - return EndsWithT(str, search, case_sensitive); + if (!case_sensitive) { + // This function was originally written using the current locale functions + // for case-insensitive comparisons. Emulate this behavior until callers + // can be converted either to use the case-insensitive ASCII one (most + // callers) or ICU functions in base_i18n. + if (search.size() > str.size()) + return false; + return std::equal(search.begin(), search.end(), + str.begin() + (str.size() - search.size()), + CaseInsensitiveCompare()); + } + return EndsWith(StringPiece16(str), StringPiece16(search), + CompareCase::SENSITIVE); } } // namespace base diff --git a/base/strings/string_util.h b/base/strings/string_util.h index 9628c67cbea9..6f3b766fc138 100644 --- a/base/strings/string_util.h +++ b/base/strings/string_util.h @@ -329,24 +329,52 @@ BASE_EXPORT bool LowerCaseEqualsASCII(const char16* a_begin, // strings are not ASCII. BASE_EXPORT bool EqualsASCII(const string16& a, const StringPiece& b); -// Returns true if str starts with search, or false otherwise. -// TODO(brettw) the case sensitive flag makes callsites difficult to read. -// Consider splitting this out in two variants (few callers want -// case-insensitive compares) or use an enum that makes this more explicit. -BASE_EXPORT bool StartsWithASCII(const std::string& str, - const std::string& search, - bool case_sensitive); -BASE_EXPORT bool StartsWith(const base::string16& str, - const base::string16& search, - bool case_sensitive); +// Indicates case sensitivity of comparisons. Only ASCII case insensitivity +// is supported. Full Unicode case-insensitive conversions would need to go in +// base/i18n so it can use ICU. +// +// If you need to do Unicode-aware case-insensitive StartsWith/EndsWith, it's +// best to just call base::i18n::ToLower() on the arguements, and then use the +// results to a case-sensitive comparison. +enum class CompareCase { + SENSITIVE, + INSENSITIVE_ASCII, +}; -// Returns true if str ends with search, or false otherwise. -// TODO(brettw) case sensitive flag confusion, see StartsWith above. -BASE_EXPORT bool EndsWith(const std::string& str, - const std::string& search, - bool case_sensitive); -BASE_EXPORT bool EndsWith(const base::string16& str, - const base::string16& search, +BASE_EXPORT bool StartsWith(StringPiece str, + StringPiece search_for, + CompareCase case_sensitivity); +BASE_EXPORT bool StartsWith(StringPiece16 str, + StringPiece16 search_for, + CompareCase case_sensitivity); +BASE_EXPORT bool EndsWith(StringPiece str, + StringPiece search_for, + CompareCase case_sensitivity); +BASE_EXPORT bool EndsWith(StringPiece16 str, + StringPiece16 search_for, + CompareCase case_sensitivity); + +// DEPRECATED. Returns true if str starts/ends with search, or false otherwise. +// TODO(brettw) remove in favor of the "enum" versions above. +inline bool StartsWithASCII(const std::string& str, + const std::string& search, + bool case_sensitive) { + return StartsWith(StringPiece(str), StringPiece(search), + case_sensitive ? CompareCase::SENSITIVE + : CompareCase::INSENSITIVE_ASCII); +} +BASE_EXPORT bool StartsWith(const string16& str, + const string16& search, + bool case_sensitive); +inline bool EndsWith(const std::string& str, + const std::string& search, + bool case_sensitive) { + return EndsWith(StringPiece(str), StringPiece(search), + case_sensitive ? CompareCase::SENSITIVE + : CompareCase::INSENSITIVE_ASCII); +} +BASE_EXPORT bool EndsWith(const string16& str, + const string16& search, bool case_sensitive); } // namespace base diff --git a/base/sys_info.cc b/base/sys_info.cc index 8640dc14edcf..6e283be1f95e 100644 --- a/base/sys_info.cc +++ b/base/sys_info.cc @@ -41,7 +41,7 @@ bool SysInfo::IsLowEndDevice() { // Low End Device Mode will be enabled if this client is assigned to // one of those EnabledXXX groups. - if (StartsWithASCII(group_name, "Enabled", true)) + if (StartsWith(group_name, "Enabled", CompareCase::SENSITIVE)) return true; return g_lazy_low_end_device.Get().value(); diff --git a/base/version.cc b/base/version.cc index ede8a4586eb9..228dcb8aef20 100644 --- a/base/version.cc +++ b/base/version.cc @@ -24,15 +24,17 @@ namespace { // parsed successfully, false otherwise. bool ParseVersionNumbers(const std::string& version_str, std::vector* parsed) { - std::vector numbers; - SplitString(version_str, '.', &numbers); + std::vector numbers = + SplitStringPiece(version_str, ".", KEEP_WHITESPACE, SPLIT_WANT_ALL); if (numbers.empty()) return false; - for (std::vector::const_iterator it = numbers.begin(); - it != numbers.end(); ++it) { - if (StartsWithASCII(*it, "+", false)) + for (auto it = numbers.begin(); it != numbers.end(); ++it) { + if (StartsWith(*it, "+", CompareCase::SENSITIVE)) return false; + + // TODO(brettw) when we have a StringPiece version of StringToUint, delete + // this string conversion. unsigned int num; if (!StringToUint(*it, &num)) return false; @@ -98,8 +100,8 @@ bool Version::IsValid() const { // static bool Version::IsValidWildcardString(const std::string& wildcard_string) { std::string version_string = wildcard_string; - if (EndsWith(wildcard_string.c_str(), ".*", false)) - version_string = wildcard_string.substr(0, wildcard_string.size() - 2); + if (EndsWith(version_string, ".*", CompareCase::SENSITIVE)) + version_string.resize(version_string.size() - 2); Version version(version_string); return version.IsValid(); @@ -117,7 +119,7 @@ int Version::CompareToWildcardString(const std::string& wildcard_string) const { DCHECK(Version::IsValidWildcardString(wildcard_string)); // Default behavior if the string doesn't end with a wildcard. - if (!EndsWith(wildcard_string.c_str(), ".*", false)) { + if (!EndsWith(wildcard_string, ".*", CompareCase::SENSITIVE)) { Version version(wildcard_string); DCHECK(version.IsValid()); return CompareTo(version); diff --git a/base/win/win_util.cc b/base/win/win_util.cc index cf93444b6e2a..e33e70b5f2eb 100644 --- a/base/win/win_util.cc +++ b/base/win/win_util.cc @@ -149,8 +149,8 @@ bool IsKeyboardPresentOnSlate() { if (status == CR_SUCCESS) { // To reduce the scope of the hack we only look for ACPI and HID\\VID // prefixes in the keyboard device ids. - if (StartsWith(device_id, L"ACPI", false) || - StartsWith(device_id, L"HID\\VID", false)) { + if (StartsWith(device_id, L"ACPI", CompareCase::INSENSITIVE_ASCII) || + StartsWith(device_id, L"HID\\VID", CompareCase::INSENSITIVE_ASCII)) { keyboard_count++; } } diff --git a/content/browser/download/save_package.h b/content/browser/download/save_package.h index 3507b1760b4c..071a5b7adead 100644 --- a/content/browser/download/save_package.h +++ b/content/browser/download/save_package.h @@ -301,9 +301,10 @@ class CONTENT_EXPORT SavePackage // Number of all need to be saved resources. size_t all_save_items_count_; - typedef std::set FileNameSet; + using FileNameSet = + std::set; // This set is used to eliminate duplicated file names in saving directory. FileNameSet file_name_set_; -- 2.11.4.GIT