[webkit-reviews] review granted: [Bug 181410] Add WKNavigationDelegate SPI exposing WebProcess crash reason : [Attachment 330747] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 181410: Add WKNavigationDelegate SPI exposing WebProcess crash reason
https://bugs.webkit.org/show_bug.cgi?id=181410

Attachment 330747: Patch

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




--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 330747
  --> https://bugs.webkit.org/attachment.cgi?id=330747
Patch

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

> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:894
> +static _WKProcessTerminationReason
toWKProcessTerminationReason(ProcessTerminationReason reason)

Nit - I think WebKit style guidelines recommend not having the "to" prefix
(https://webkit.org/code-style-guidelines/#names-verb). Maybe
processTerminationReason is sufficient?

> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:927
> +    if
(m_navigationState.m_navigationDelegateMethods.webViewWebContentProcessDidTermi
nateWithReason) {

Nit - Maybe this should go ahead of the webViewWebContentProcessDidTerminate
check, so that if a delegate implements both versions, the SPI version is
preferred. Or maybe this doesn't really matter :P