[webkit-reviews] review granted: [Bug 197177] Return annotated text checking strings via UIWKDocumentContext : [Attachment 367971] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 23 13:44:57 PDT 2019
Ryosuke Niwa <rniwa at webkit.org> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 197177: Return annotated text checking strings via UIWKDocumentContext
https://bugs.webkit.org/show_bug.cgi?id=197177
Attachment 367971: Patch
https://bugs.webkit.org/attachment.cgi?id=367971&action=review
--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 367971
--> https://bugs.webkit.org/attachment.cgi?id=367971
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=367971&action=review
> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.h:54
> + AttributedString
annotatedSubstringBetweenPositions(WebCore::VisiblePosition,
WebCore::VisiblePosition);
Use const &?
> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:171
> + RefPtr<Document> document = start.deepEquivalent().document();
makeRefPtr?
> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:185
> + RefPtr<Range> currentTextRange = it.range();
It's expensive to create range. Do this after the continue below.
> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:190
> + [string replaceCharactersInRange:NSMakeRange(stringLength, 0)
withString:it.text().createNSStringWithoutCopying().get()];
Why not just appendString?
> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:202
> + auto subrange = TextIterator::subrange(*currentTextRange,
marker->startOffset(), marker->endOffset() - marker->startOffset());
> +
> + size_t subrangeLocation;
> + size_t subrangeLength;
> +
TextIterator::getLocationAndLengthFromRange(commonAncestor.get(),
&subrange.get(), subrangeLocation, subrangeLength);
This is a pretty bad o(n^2)...
More information about the webkit-reviews
mailing list