Bug 238362 - Use StringView::split() instead of String::split() in more places
Summary: Use StringView::split() instead of String::split() in more places
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-24 22:26 PDT by Chris Dumez
Modified: 2022-03-25 16:44 PDT (History)
20 users (show)

See Also:


Attachments
Patch (41.01 KB, patch)
2022-03-24 22:42 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (41.17 KB, patch)
2022-03-25 11:06 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2022-03-24 22:26:09 PDT
Use StringView::split() instead of String::split() where suitable and more efficient.
Comment 1 Chris Dumez 2022-03-24 22:42:43 PDT
Created attachment 455725 [details]
Patch
Comment 2 Geoffrey Garen 2022-03-25 09:59:56 PDT
Comment on attachment 455725 [details]
Patch

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

r=me

> Source/WebCore/html/ValidationMessage.cpp:137
> +    size_t i = 0;
> +    for (auto line : lines) {

As you add more loops with explicit 'I' counters, I wonder if we can create some helper class whose purpose is to wrap an iterator and offer a counter automatically. Something like:

for (auto line : iteratorWithIndex(lines) {
    if (line.index() > 1)
        ...
}

Or:

for (auto line : countedIterator(lines) {
    if (line.counter() > 1)
        ...
}
Comment 3 Chris Dumez 2022-03-25 10:06:31 PDT
Comment on attachment 455725 [details]
Patch

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

>> Source/WebCore/html/ValidationMessage.cpp:137
>> +    for (auto line : lines) {
> 
> As you add more loops with explicit 'I' counters, I wonder if we can create some helper class whose purpose is to wrap an iterator and offer a counter automatically. Something like:
> 
> for (auto line : iteratorWithIndex(lines) {
>     if (line.index() > 1)
>         ...
> }
> 
> Or:
> 
> for (auto line : countedIterator(lines) {
>     if (line.counter() > 1)
>         ...
> }

Hmm, this is the only case in this patch where I added an 'i' counter it seems. Also, I think in this particular instance, I could probably replace this counter by a simple boolean.
Comment 4 Chris Dumez 2022-03-25 11:06:09 PDT
Created attachment 455786 [details]
Patch
Comment 5 EWS 2022-03-25 16:43:39 PDT
Committed r291902 (248895@main): <https://commits.webkit.org/248895@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455786 [details].
Comment 6 Radar WebKit Bug Importer 2022-03-25 16:44:17 PDT
<rdar://problem/90862764>