Bug 153732

Summary: Cut down on calls to String::lower; mostly replace with convertToASCIILowercase
Product: WebKit Reporter: Darin Adler <darin>
Component: WebCore Misc.Assignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, buildbot, commit-queue, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews102 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Archive of layout-test-results from ews115 for mac-yosemite
none
Patch dino: review+

Description Darin Adler 2016-01-31 17:02:35 PST
Cut down on calls to String::lower; mostly replace with convertToASCIILowercase
Comment 1 Darin Adler 2016-01-31 18:32:29 PST
Created attachment 270359 [details]
Patch
Comment 2 Darin Adler 2016-01-31 18:33:55 PST
This first batch covers things I ran into going alphabetically through the source code. I stopped before the "html" directory, and there are a lot of call sites for String::lower in there and beyond.
Comment 3 WebKit Commit Bot 2016-01-31 18:34:21 PST
Attachment 270359 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:887:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1513:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:1116:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/navigatorcontentutils/NavigatorContentUtils.cpp:70:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:85:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:103:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1371:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 7 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Darin Adler 2016-01-31 18:35:29 PST
Created attachment 270360 [details]
Patch
Comment 5 WebKit Commit Bot 2016-01-31 18:37:06 PST
Attachment 270360 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:887:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1513:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:1116:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/navigatorcontentutils/NavigatorContentUtils.cpp:70:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:85:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:103:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1371:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 7 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Build Bot 2016-01-31 19:39:34 PST
Comment on attachment 270360 [details]
Patch

Attachment 270360 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/765998

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/time.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/search_input.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/range.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/datetime.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/required_attribute.html
fast/canvas/webgl/antialiasing-enabled.html
animations/restart-after-scroll.html
animations/restart-after-scroll-nested.html
animations/dynamic-stylesheet-loading.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/pattern_attribute.html
Comment 7 Build Bot 2016-01-31 19:39:37 PST
Created attachment 270362 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-01-31 19:43:17 PST
Comment on attachment 270360 [details]
Patch

Attachment 270360 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/766001

New failing tests:
animations/dynamic-stylesheet-loading.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/time.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/range.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/datetime.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/pattern_attribute.html
fast/canvas/webgl/antialiasing-enabled.html
animations/restart-after-scroll.html
animations/restart-after-scroll-nested.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/search_input.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/required_attribute.html
Comment 9 Build Bot 2016-01-31 19:43:20 PST
Created attachment 270364 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 10 Build Bot 2016-01-31 19:47:08 PST
Comment on attachment 270360 [details]
Patch

Attachment 270360 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/766005

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/time.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/search_input.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/range.html
fast/canvas/webgl/antialiasing-enabled.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/pattern_attribute.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/datetime.html
animations/restart-after-scroll-nested.html
animations/dynamic-stylesheet-loading.html
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/required_attribute.html
Comment 11 Build Bot 2016-01-31 19:47:11 PST
Created attachment 270365 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 12 Darin Adler 2016-01-31 20:11:28 PST
Created attachment 270366 [details]
Patch
Comment 13 WebKit Commit Bot 2016-01-31 20:14:20 PST
Attachment 270366 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/cf/MediaPlayerPrivateAVFoundationCF.cpp:887:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1513:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:1116:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/navigatorcontentutils/NavigatorContentUtils.cpp:70:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:85:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm:103:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1666:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1371:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 8 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Darin Adler 2016-01-31 20:15:42 PST
Style checker doesn’t seem to understand the style we use for lambdas. I am supposed to file a bug against check-webkit-style about that?
Comment 15 Darin Adler 2016-01-31 20:34:38 PST
OK, let me say that again. The style checker failures are all due to bug 137309.
Comment 16 Darin Adler 2016-01-31 20:36:20 PST
Which is a duplicate of bug 125616.
Comment 17 Darin Adler 2016-01-31 20:40:22 PST
OK, looks like EWS is passing now so it’s time for me to find a reviewer.
Comment 18 Darin Adler 2016-01-31 21:08:16 PST
Comment on attachment 270366 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270366&action=review

> Source/WebCore/css/CSSParserValues.h:37
> +// 1. The convertToASCIILowercaseInPlace clobbers the data we point to.

Probably should have used the word "function" here.
Comment 19 Dean Jackson 2016-01-31 21:37:20 PST
Comment on attachment 270366 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=270366&action=review

> Source/WebCore/css/CSSParser.cpp:1202
> +        // Don't try to parse initial/inherit/unset/revert shorthands; return an error so it the caller will use the full CSS parser.

return an error so the caller... (remove "it")

> Source/WebCore/css/CSSParserValues.h:39
> +// But it sure would be nice if it was a StringView.

I feel like this sentence needs to end with "!"
Comment 20 Darin Adler 2016-01-31 21:46:25 PST
Committed r195951: <http://trac.webkit.org/changeset/195951>