Next typing after canceling a reversion panel would hang. <rdar://problem/9427970>
Created attachment 93316 [details] Patch (v1)
Verified this patch with all existing automatic and manual autocorrection tests. I probably forgot to run manual tests on the offending changeset. Otherwise I would have caught this.
Comment on attachment 93316 [details] Patch (v1) View in context: https://bugs.webkit.org/attachment.cgi?id=93316&action=review Iām really worried about the thread-safety issues here. > Source/WebKit/mac/ChangeLog:10 > + Need to clear m_view pointer in handleAcceptedReplacement(), since this function can be invoked > + by AppKit on different thread without going through dismissInternal(). Does this code have enough locking for this to be called on a different thread!? > Source/WebKit2/ChangeLog:10 > + Need to clear m_view pointer in handleAcceptedReplacement(), since this function can be invoked > + by AppKit on different thread without going through dismissInternal(). Does this code have enough locking for this to be called on a different thread!?
Comment on attachment 93316 [details] Patch (v1) Thanks for the review, Darin! Committed in http://trac.webkit.org/changeset/86371
(In reply to comment #4) > (From update of attachment 93316 [details]) > Thanks for the review, Darin! Committed in http://trac.webkit.org/changeset/86371 Jessie, actually Darin raised a valid concern. I'm talking to some AppKit folks to gain better understanding on the threading model. I may have another change.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 93316 [details] [details]) > > Thanks for the review, Darin! Committed in http://trac.webkit.org/changeset/86371 > > Jessie, actually Darin raised a valid concern. I'm talking to some AppKit folks to gain better understanding on the threading model. I may have another change. That was a total fail by me. It was meant for another bug, not this one. Sorry!
Reopen since it was closed by mistake.
Created attachment 93357 [details] Patch (v2)
Comment on attachment 93357 [details] Patch (v2) Having looked at the AppKit source code, it seems all callbacks eventually are delivered on main thread. So we really don't need any locking here. I will upload another patch soon.
Created attachment 93365 [details] Patch (v3)
Comment on attachment 93365 [details] Patch (v3) View in context: https://bugs.webkit.org/attachment.cgi?id=93365&action=review > Source/WebKit/mac/ChangeLog:14 > + Since now we don't have distincation between dismiss() and dismissSoon(). dismissSoon() has Typo: distinction
Created attachment 93374 [details] Patch (v4) Fixed typo.
Comment on attachment 93374 [details] Patch (v4) Clearing flags on attachment: 93374 Committed r86406: <http://trac.webkit.org/changeset/86406>
All reviewed patches have been landed. Closing bug.