[webkit-reviews] review granted: [Bug 195354] [iOS] Frequent 1 second IPC deadlocks when showing a paste callout : [Attachment 363731] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 195354: [iOS] Frequent 1 second IPC deadlocks when showing a paste callout
https://bugs.webkit.org/show_bug.cgi?id=195354

Attachment 363731: Patch

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




--- Comment #2 from Tim Horton <thorton at apple.com> ---
Comment on attachment 363731
  --> https://bugs.webkit.org/attachment.cgi?id=363731
Patch

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

> Source/WebKit/UIProcess/PageClient.h:399
> +    virtual void handleAutocorrectionContextSync(const
WebAutocorrectionContext&) = 0;

This name seems ... odd. Mostly because I think I expect the -Sync suffix on
things that block on sync IPC, but this isn't that!

> Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:579
> +    m_process->send(Messages::WebPage::RequestAutocorrectionContextSync(),
m_pageID);

Oh... Even the request one is named wrong! Get rid of all the -Sync; there's
nothing inherently synchronous about this message or functions related to it
anymore.

This is just request, the other one is just the reply, and we just *happen* to
block on the reply in the UI process.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6375
> +    // FIXME: Computing and sending an autocorrection context is a
workaround for the fact that autocorrection context

You know how I feel about this already but it is a clever solution to a problem
with no clear alternatives.

> LayoutTests/ChangeLog:10
> +	   Most of these tests currently encounter and rely on the 1 second IPC
timeout to finish. To test this fix, force
> +	   `ignoreSynchronousMessagingTimeouts=true` to make them fail if the
processes encounter a deadlock.

Hilarious