Bug 160628 - Simplify valueToUSVString
Summary: Simplify valueToUSVString
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-05 21:18 PDT by Darin Adler
Modified: 2016-08-06 11:20 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.75 KB, patch)
2016-08-05 21:27 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2016-08-05 21:18:50 PDT
Simplify valueToUSVString
Comment 1 Darin Adler 2016-08-05 21:27:05 PDT
Created attachment 285479 [details]
Patch
Comment 2 Darin Adler 2016-08-05 21:29:28 PDT
Note that the String::find function calls its function on each code unit, not each code point, so it can't be used to find unpaired surrogates. Instead it was finding all surrogates.
Comment 3 Darin Adler 2016-08-05 21:33:48 PDT
Comment on attachment 285479 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMBinding.cpp:141
> +    bool foundUnpairedSurrogate = false;
> +    for (auto codePoint : view.codePoints()) {
> +        if (U_IS_SURROGATE(codePoint)) {
> +            foundUnpairedSurrogate = true;
> +            break;
> +        }
> +    }
> +    if (!foundUnpairedSurrogate)
> +        return string;

Or I could write a StringView::contains that takes a function that takes a UChar32 and have it iterate code points to remove this loop from this function. To do that I would presumably rename CharacterMatchFunction to CodeUnitMatchFunction and then add a CodePointMatchFunction type. But function pointers are inefficient. I wonder if there is a simple way to write a version that will inline the lambda that is passed in.

(Strangely I noticed we have both CharacterMatchFunction in StringView.h and CharacterMatchFunctionPtr in StringImpl.h. We also have a function named stripWhiteSpace that strips arbitrary code units and simplifyWhiteSpace that "simplifies" arbitrary code units. Needs some cleanup I think.)
Comment 4 WebKit Commit Bot 2016-08-06 11:20:37 PDT
Comment on attachment 285479 [details]
Patch

Clearing flags on attachment: 285479

Committed r204228: <http://trac.webkit.org/changeset/204228>
Comment 5 WebKit Commit Bot 2016-08-06 11:20:43 PDT
All reviewed patches have been landed.  Closing bug.