Bug 220469

Summary:

[macOS] -[WKWebView acceptsFirstMouse:] sometimes crashes in IPC::Connection::createSyncMessageEncoder

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

Platform

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=220520

Attachments:

Description	Flags
Patch	none
Patch	none

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].