[webkit-reviews] review granted: [Bug 126606] ASSERT(!m_textCheckingRequest) on editing/spelling/spellcheck-async-remove-frame.html : [Attachment 393108] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 126606: ASSERT(!m_textCheckingRequest) on
editing/spelling/spellcheck-async-remove-frame.html
https://bugs.webkit.org/show_bug.cgi?id=126606

Attachment 393108: Patch

https://bugs.webkit.org/attachment.cgi?id=393108&action=review




--- Comment #12 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 393108
  --> https://bugs.webkit.org/attachment.cgi?id=393108
Patch

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

r=mews

> Source/WebKitLegacy/mac/ChangeLog:37
> +2020-03-09  Simon Fraser  <simon.fraser at apple.com>
> +
> +	   ASSERT(!m_textCheckingRequest) on
editing/spelling/spellcheck-async-remove-frame.html
> +	   https://bugs.webkit.org/show_bug.cgi?id=126606
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Make WebEditorClient able to handle multiple in-flight spell check
requests by storing
> +	   a HashMap of sequence numbers to TextCheckingRequests.
> +
> +	   * WebCoreSupport/WebEditorClient.h:
> +	   * WebCoreSupport/WebEditorClient.mm:
> +	   (-[WebEditorSpellCheckResponder initWithClient:sequence:results:]):
> +	   (WebEditorClient::didCheckSucceed):
> +	   (WebEditorClient::requestCheckingOfString):

Double changelog!

> Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.mm:1242
> +void WebEditorClient::didCheckSucceed(TextCheckingRequestIdentifier
identifier, NSArray* results)

Nit - * on the other side.

> Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.mm:1269
> +    [[NSSpellChecker sharedSpellChecker]
requestCheckingOfString:request.data().text() range:range
types:NSTextCheckingAllSystemTypes options:options inSpellDocumentWithTag:0
completionHandler:^(NSInteger, NSArray* results, NSOrthography*, NSInteger) {

Nit - not new in this patch, but there are some *s on the wrong side here.