Bug 153419 – WTF should have a similar function as equalLettersIgnoringASCIICase to match beginning of strings

• Component: Web Template Framework
• Status: RESOLVED FIXED

[youenn fablet]

startWithLettersIgnoringASCIICase would be useful for checking headers in https://fetch.spec.whatwg.org/#terminology-headers

[youenn fablet]

Created attachment 269774 [details]
Patch

[Darin Adler]

Comment on attachment 269774 [details]
Patch

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

Name should be startsWithLettersIgnoringASCIICase, plural.

> Source/WTF/wtf/text/StringCommon.h:562
> +template<typename StringClass> bool equalLettersIgnoringASCIICaseCommonWithLength(const StringClass& string, const char* lowercaseLetters, unsigned length)

I am not sure this is a good name for this helper. This checks if a prefix of string is equal assuming we already know length is <= string.length(), so it’s not a general purpose function so needs a more specific name.

> Source/WTF/wtf/text/StringCommon.h:582
> +    return equalLettersIgnoringASCIICaseCommonWithLength(string, lowercaseLetters, length);

Because the function this calls is not marked inline, this is likely adding a level function call overhead to every call of equalLettersIgnoringASCIICase. We don’t want to do that! Need to mark the function this is calling inline.

> Source/WTF/wtf/text/StringImpl.h:1200
> +template<unsigned length> inline bool startWithLettersIgnoringASCIICase(const StringImpl& string, const char (&lowercaseLetters)[length])
> +{
> +    if (length == 1)
> +        return true;
> +    if (string.length() < (length - 1))
> +        return false;
> +    return equalLettersIgnoringASCIICaseCommonWithLength(string, lowercaseLetters, length - 1);
> +}

I think this should work the same way as equalLettersIgnoringASCIICaseCommon; don’t actually use the length, chose code size over speed (for now). Call a version that just takes a pointer and does strlen.

[youenn fablet]

Created attachment 269863 [details]
Updated according review

[youenn fablet]

Created attachment 269876 [details]
Fully aligning with equalLetters implementation

[youenn fablet]

(In reply to comment #2)
> Comment on attachment 269774 [details]
> Patch

Thanks for the review.
I fully aligned with equalLettersIgnoringASCIICase for consistency.
I changed the new routine to hasPrefixWithLettersIgnoringASCIICaseCommon

[Darin Adler]

Comment on attachment 269876 [details]
Fully aligning with equalLetters implementation

Excellent.

We will want to follow this patch up by using this everywhere that the member startsWith(x, false) is currently used with a constant string that includes the legal characters.

WebCore/css/CSSParser.cpp:    if (!string.startsWith("translate", false))
WebCore/css/CSSFontFaceSrcValue.cpp:        if (!m_resource.startsWith("data:", false) && m_resource.endsWith(".eot", false))
WebCore/css/CSSParser.cpp:    if (!string.startsWith("translate", false))
WebCore/dom/DOMImplementation.cpp:    return feature.startsWith("http://www.w3.org/tr/svg11/feature#", false)
WebCore/dom/DOMImplementation.cpp:    if (feature.startsWith("http://www.w3.org/TR/SVG", false)
WebCore/dom/DOMImplementation.cpp:        || feature.startsWith("org.w3c.dom.svg", false)
WebCore/dom/DOMImplementation.cpp:        || feature.startsWith("org.w3c.svg", false)) {
WebCore/html/HTMLObjectElement.cpp:        if (equalLettersIgnoringASCIICase(metaElement.name(), "generator") && metaElement.content().startsWith("Mac OS X Server Web Services Server", false))
WebCore/html/HTMLObjectElement.cpp:    if (MIMETypeRegistry::isJavaAppletMIMEType(serviceType()) && fastGetAttribute(classidAttr).startsWith("java:", false))
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("+//Silmaril//dtd html Pro v0r11 19970101//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//AdvaSoft Ltd//DTD HTML 3.0 asWedit + extensions//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//AS//DTD HTML 3.0 asWedit + extensions//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0 Level 1//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0 Level 2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0 Strict Level 1//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0 Strict Level 2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0 Strict//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 2.1E//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 3.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 3.2 Final//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 3.2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML 3//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Level 0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Level 1//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Level 2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Level 3//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Strict Level 0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Strict Level 1//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Strict Level 2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Strict Level 3//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML Strict//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//IETF//DTD HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Metrius//DTD Metrius Presentational//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 2.0 HTML Strict//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 2.0 HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 2.0 Tables//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 3.0 HTML Strict//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 3.0 HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Microsoft//DTD Internet Explorer 3.0 Tables//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Netscape Comm. Corp.//DTD HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Netscape Comm. Corp.//DTD Strict HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//O'Reilly and Associates//DTD HTML 2.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//O'Reilly and Associates//DTD HTML Extended 1.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//O'Reilly and Associates//DTD HTML Extended Relaxed 1.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//SoftQuad Software//DTD HoTMetaL PRO 6.0::19990601::extensions to HTML 4.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//SoftQuad//DTD HoTMetaL PRO 4.0::19971010::extensions to HTML 4.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Spyglass//DTD HTML 2.0 Extended//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//SQ//DTD HTML 2.0 HoTMetaL + extensions//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Sun Microsystems Corp.//DTD HotJava HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//Sun Microsystems Corp.//DTD HotJava Strict HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 3 1995-03-24//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 3.2 Draft//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 3.2 Final//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 3.2//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 3.2S Draft//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 4.0 Frameset//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML 4.0 Transitional//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML Experimental 19960712//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD HTML Experimental 970421//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD W3 HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3O//DTD W3 HTML 3.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//WebTechs//DTD Mozilla HTML 2.0//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//WebTechs//DTD Mozilla HTML//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || (systemId.isEmpty() && publicId.startsWith("-//W3C//DTD HTML 4.01 Frameset//", false))
WebCore/html/parser/HTMLConstructionSite.cpp:        || (systemId.isEmpty() && publicId.startsWith("-//W3C//DTD HTML 4.01 Transitional//", false))) {
WebCore/html/parser/HTMLConstructionSite.cpp:    if (publicId.startsWith("-//W3C//DTD XHTML 1.0 Frameset//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || publicId.startsWith("-//W3C//DTD XHTML 1.0 Transitional//", false)
WebCore/html/parser/HTMLConstructionSite.cpp:        || (!systemId.isEmpty() && publicId.startsWith("-//W3C//DTD HTML 4.01 Frameset//", false))
WebCore/html/parser/HTMLConstructionSite.cpp:        || (!systemId.isEmpty() && publicId.startsWith("-//W3C//DTD HTML 4.01 Transitional//", false))) {
WebCore/page/SecurityOrigin.cpp:    if (!urlString.startsWith("feed", false))
WebCore/page/SecurityOrigin.cpp:    return urlString.startsWith("feed://", false) 
WebCore/page/SecurityOrigin.cpp:        || urlString.startsWith("feed:http:", false) || urlString.startsWith("feed:https:", false)
WebCore/page/SecurityOrigin.cpp:        || urlString.startsWith("feeds:http:", false) || urlString.startsWith("feeds:https:", false)
WebCore/page/SecurityOrigin.cpp:        || urlString.startsWith("feedsearch:http:", false) || urlString.startsWith("feedsearch:https:", false);
WebCore/platform/MIMETypeRegistry.cpp:    return mimeType.startsWith("application/x-java-applet", false)
WebCore/platform/MIMETypeRegistry.cpp:        || mimeType.startsWith("application/x-java-bean", false)
WebCore/platform/MIMETypeRegistry.cpp:        || mimeType.startsWith("application/x-java-vm", false);
WebCore/platform/MIMETypeRegistry.cpp:    if (mimeType.startsWith("text/", false))
WebCore/platform/graphics/MediaPlayer.cpp:            && (parameters.type.startsWith("video/webm", false) || parameters.type.startsWith("video/x-flv", false)))
WebCore/platform/network/curl/ResourceHandleManager.cpp:    if (key.startsWith("x-", /* caseSensitive */ false))
WebCore/platform/network/curl/ResourceHandleManager.cpp:        } else if (header.startsWith("HTTP", false)) {
WebCore/xml/XMLHttpRequest.cpp:    if (name.startsWith("proxy-", false))
WebCore/xml/XMLHttpRequest.cpp:    if (name.startsWith("sec-", false))
WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:    if (urlString.startsWith("file:///", false) && urlString.endsWith("/etc/catalog", false))
WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:    if (urlString.startsWith("http://www.w3.org/TR/xhtml", false))
WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:    if (urlString.startsWith("http://www.w3.org/Graphics/SVG", false))

WebKit/win/Plugins/PluginDatabaseWin.cpp:            if ((!filename.startsWith("np", false) || !filename.endsWith("dll", false)) &&
WebKit2/NetworkProcess/cache/NetworkCache.cpp:    if (mimeType.startsWith("video/", /*caseSensitive*/ false))
WebKit2/NetworkProcess/cache/NetworkCache.cpp:    if (mimeType.startsWith("audio/", /*caseSensitive*/ false))

Our goal will be to eliminate the boolean case sensitivity argument to startsWith entirely, soon.

[Darin Adler]

Please note a couple points about this for future consideration:

1) Using non-member functions for string operations is forward looking; it lets us overload the same operations for non-class types such as "const char*" or "pointers plus lengths both as separate argument". So over time we should start moving these to non-member functions if they can be done efficiently without any special private access.

2) We normally overload for the following types: StringView, StringImpl&, StringImpl*, const String&, const char* (for all-ASCII strings). In some cases we also overload for ASCII literals, char (&)[length], and this is one of those. We can also overload for const CString& if it’s handy. In some cases we may also need to overload for const AtomicString&, AtomicStringImpl&, AtomicStringImpl*, and but this is usually not necessary.

There are additional issues only for string operations that sometimes produce a new string and sometimes just return the original string. And additional issues for string operations that return offsets within a string. And additional issues for string operations that can operate on a substring (such as a find operation that takes a start character offset for where to begin searching). For some of these we may even chose to support the operation only on StringView rather than overloading.

[WebKit Commit Bot]

Comment on attachment 269876 [details]
Fully aligning with equalLetters implementation

Clearing flags on attachment: 269876

Committed r198019: <http://trac.webkit.org/changeset/198019>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.