Bug 238362

Summary:

Use StringView::split() instead of String::split() in more places

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebCore Misc.

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, changseok, cmarcelo, darin, dino, esprehn+autocc, ews-watchlist, fmalita, ggaren, gyuyoung.kim, hi, joepeck, kondapallykalyan, mifenton, pangle, pdr, sabouhallawa, schenney, sergio, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Chris Dumez

Reported 2022-03-24 22:26:09 PDT

Use StringView::split() instead of String::split() where suitable and more efficient.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2022-03-24 22:42:43 PDT

Created attachment 455725 [details] Patch

Geoffrey Garen

Comment 2 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) ... }

Chris Dumez

Comment 3 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.

Chris Dumez

Comment 4 2022-03-25 11:06:09 PDT

Created attachment 455786 [details] Patch

EWS

Comment 5 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].

Radar WebKit Bug Importer

Comment 6 2022-03-25 16:44:17 PDT

<rdar://problem/90862764>

Note You need to log in before you can comment on or make changes to this bug.