[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