Bug 160630 - Fix some minor problems in the StringImpl header
Summary: Fix some minor problems in the StringImpl header
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-08-05 23:34 PDT by Darin Adler
Modified: 2017-11-24 18:45 PST (History)
7 users (show)

See Also:

Attachments
Patch (56.54 KB, patch)
2016-08-05 23:50 PDT, Darin Adler 		bfulgham: review+
bfulgham: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2016-08-05 23:34:58 PDT 
Fix some minor problems in the StringImpl header
Comment 1 Darin Adler 2016-08-05 23:50:36 PDT 
Created attachment 285483 [details]
Patch
Comment 2 Darin Adler 2016-08-05 23:51:33 PDT 
Brady, notice the false positives from check-webkit-style here.
Comment 3 WebKit Commit Bot 2016-08-05 23:53:12 PDT 
Attachment 285483 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/text/StringImpl.h:670:  'simplifyWhiteSpace' is incorrectly named. It should be named 'protector' or 'protectedCodeUnitMatchFunction'.  [readability/naming/protected] [4]
ERROR: Source/WTF/wtf/text/StringImpl.h:672:  'stripLeadingAndTrailingCharacters' is incorrectly named. It should be named 'protector' or 'protectedCodeUnitMatchFunction'.  [readability/naming/protected] [4]
ERROR: Source/WTF/wtf/text/StringImpl.h:673:  'removeCharacters' is incorrectly named. It should be named 'protector' or 'protectedCodeUnitMatchFunction'.  [readability/naming/protected] [4]
Total errors found: 3 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Brent Fulgham 2016-08-22 11:13:30 PDT 
Windows error:

C:\cygwin\home\buildbot\WebKit\Source\WebKit\win\WebCoreSupport\WebChromeClient.cpp)
C:\cygwin\home\buildbot\WebKit\Source\WebKit\win\WebView.cpp(3758): error C2065: 'TextCaseSensitive': undeclared identifier [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebKit\WebKit.vcxproj]
C:\cygwin\home\buildbot\WebKit\Source\WebKit\win\WebView.cpp(3758): error C2065: 'TextCaseInsensitive': undeclared identifier [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebKit\WebKit.vcxproj]
C:\cygwin\home\buildbot\WebKit\Source\WebKit\win\WebView.cpp(3758): error C2660: 'WebCore::Page::markAllMatchesForText': function does not take 3 arguments [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebKit\WebKit.vcxproj]
  WebEditorClient.cpp

It looks like WebChromeClient.cpp and WebView.cpp need to know that 'TextCaseSensitivity' was moved to JavascriptCore/yarr.
Comment 5 Brent Fulgham 2016-08-22 11:14:05 PDT 
The GTK build failures looks like the device ran out of space, not due to a problem with this patch.
Comment 6 Brent Fulgham 2016-08-22 11:16:22 PDT 
Comment on attachment 285483 [details]
Patch

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

The patch breaks the Windows build. r- to correct that problem.

> Source/JavaScriptCore/ChangeLog:15
> +        the only place it is used.

It looks like you need to make similar RegularExpression -> JSC::Yarr::RegularExpression changes in the Windows code, too.
Comment 7 Brent Fulgham 2016-08-22 11:17:03 PDT 
Comment on attachment 285483 [details]
Patch

Actually, r+ but please fix the JSC::Yarr namespacing in the Windows code before landing.
Comment 8 Darin Adler 2017-11-24 18:44:53 PST 
Committed r225144: <https://trac.webkit.org/changeset/225144>
Comment 9 Radar WebKit Bug Importer 2017-11-24 18:45:28 PST 
<rdar://problem/35685297>