[webkit-reviews] review denied: [Bug 60090] REGRESSION(r73886): Frequent crashes in replaceSelectionWithFragment : [Attachment 92921] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied MORITA Hajime <morrita at google.com>'s
request for review:
Bug 60090: REGRESSION(r73886): Frequent crashes in replaceSelectionWithFragment
https://bugs.webkit.org/show_bug.cgi?id=60090

Attachment 92921: Patch
https://bugs.webkit.org/attachment.cgi?id=92921&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92921&action=review

> Source/WebCore/ChangeLog:9
> +	   SpellChecker uses TextCheckerClient, which belongs Page object,
> +	   which is possibly destroyed during SpellChecker's lifetime.

Wow! That sounds like a serious issue. We have a raw pointer in SpellChecker.h

> Source/WebCore/editing/SpellChecker.cpp:119
>      if (!initRequest(node))
>	   return;
> -    m_client->requestCheckingOfString(this, m_requestSequence, mask,
m_requestText);
> +    client()->requestCheckingOfString(this, m_requestSequence, mask,
m_requestText);

We should should have an early exit here as well when the page is gone.

> Source/WebCore/editing/SpellChecker.h:59
>      TextCheckerClient* m_client;

I don't think it's safe to store this pointer.