Bug 220469 - [macOS] -[WKWebView acceptsFirstMouse:] sometimes crashes in IPC::Connection::createSyncMessageEncoder
Summary: [macOS] -[WKWebView acceptsFirstMouse:] sometimes crashes in IPC::Connection:...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-08 10:45 PST by Wenson Hsieh
Modified: 2021-01-11 11:58 PST (History)
3 users (show)

See Also:

Attachments
Patch (15.72 KB, patch)
2021-01-08 11:11 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Patch (4.57 KB, patch)
2021-01-08 11:59 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-01-08 10:45:54 PST 
<rdar://problem/72319199>
Comment 1 Wenson Hsieh 2021-01-08 11:11:54 PST 
Created attachment 417280 [details]
Patch
Comment 2 Chris Dumez 2021-01-08 11:43:06 PST 
Comment on attachment 417280 [details]
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:10346
> +bool WebPageProxy::canSendSyncMessage() const

Maybe we can update sendSync() in MessageSender.h to null-check messageSenderConnection() instead of asserting it?
Comment 3 Wenson Hsieh 2021-01-08 11:44:42 PST 
(In reply to Chris Dumez from comment #2)
> Comment on attachment 417280 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417280&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:10346
> > +bool WebPageProxy::canSendSyncMessage() const
> 
> Maybe we can update sendSync() in MessageSender.h to null-check
> messageSenderConnection() instead of asserting it?

Yes, that would work too (I was just under the impression that we wanted to keep the assertion).

I'll update the patch to just null check instead.
Comment 4 Chris Dumez 2021-01-08 11:45:57 PST 
(In reply to Wenson Hsieh from comment #3)
> (In reply to Chris Dumez from comment #2)
> > Comment on attachment 417280 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=417280&action=review
> > 
> > > Source/WebKit/UIProcess/WebPageProxy.cpp:10346
> > > +bool WebPageProxy::canSendSyncMessage() const
> > 
> > Maybe we can update sendSync() in MessageSender.h to null-check
> > messageSenderConnection() instead of asserting it?
> 
> Yes, that would work too (I was just under the impression that we wanted to
> keep the assertion).
> 
> I'll update the patch to just null check instead.

I don't think so. I think this crash just proved that this assertion does not hold since in theory you can do a sync IPC while the process is still launching.
Comment 5 Wenson Hsieh 2021-01-08 11:59:28 PST 
Created attachment 417286 [details]
Patch
Comment 6 Chris Dumez 2021-01-08 12:00:25 PST 
Comment on attachment 417286 [details]
Patch

r=me, assuming the bots are happy.
Comment 7 EWS 2021-01-08 14:35:33 PST 
Committed r271322: <https://trac.webkit.org/changeset/271322>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417286 [details].