Bug 185225 – Load hangs if the WebProcess fails to launch

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Load hangs if the WebProcess fails to launch because we do not attempt to relaunch, nor do we notify the client that we failed to launch the WebProcess.
Also, since WebPageProxy::isValid() returns true while in this state, follow-up loads / reloads do not work and do not relaunch the WebProcess.

[Chris Dumez]

<rdar://problem/38249058>

[Chris Dumez]

Created attachment 339362 [details]
Patch

[Geoffrey Garen]

/Volumes/Data/EWS/WebKit/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:37:8: error: unknown type name '_WKProcessTerminationReason'; did you mean 'WKProcessTerminationReason'?
static _WKProcessTerminationReason expectedCrashReason;

[Geoffrey Garen]

Comment on attachment 339362 [details]
Patch

r=me

Please fix the 32bit build before landing.

Seems bad that one can get into the state where a web process never launches. Why do we believe that a re-launch would succeed in this case, when the original launch failed? Do we need to investigate or log more to learn why the original launch failed?

[Ryosuke Niwa]

Comment on attachment 339362 [details]
Patch

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

> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:234
> +    xpc_connection_set_event_handler(m_xpcConnection.get(), errorHandler);

xpc_connection_set_event_handler adds the event handler asynchronously so there might be a race here.
See https://trac.webkit.org/changeset/230314

[Chris Dumez]

Comment on attachment 339362 [details]
Patch

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

>> Source/WebKit/UIProcess/Launcher/mac/ProcessLauncherMac.mm:234
>> +    xpc_connection_set_event_handler(m_xpcConnection.get(), errorHandler);
> 
> xpc_connection_set_event_handler adds the event handler asynchronously so there might be a race here.
> See https://trac.webkit.org/changeset/230314

The commit you refer to is about dispatch_source_set_event_handler in IPC code. This is xpc_connection_set_event_handler() in process launching code, which we use only to listen for errors.
Could you please clarify what the issue is here? Do you believe my patch introduced a race? As far as I can tell, I am not not changing the ordering of things here, I merely moved the lambda into its own variable so that I could call it to simulate an error.

[Chris Dumez]

Created attachment 339436 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 339436 [details]
Patch

Clearing flags on attachment: 339436

Committed r231321: <https://trac.webkit.org/changeset/231321>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.