Chromium Code Reviews Chromium Code Reviews
Issue
1182453004:
Write new Starts/EndsWith and convert FilePath functions to StringPiece. (Closed)
DescriptionWrite 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
Committed: https://crrev.com/89365dc80aa9ea343cea0fe48fdeb702b23726a2
Cr-Commit-Position: refs/heads/master@{#334555}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : save_package #Patch Set 5 : Fix save_package using #
Total comments: 12
Patch Set 6 : #Patch Set 7 : default back #
Total comments: 2
Messages
Total messages: 35 (15 generated)
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
