Add null check for xpc_connection_t in AuthenticationManager::initializeConnection and AuthenticationChallengeProxy::sendClientCertificateCredentialOverXpc.
<rdar://problem/41536815>
Created attachment 343738 [details] Patch
Comment on attachment 343738 [details] Patch Attachment 343738 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8364146 New failing tests: http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
Created attachment 343775 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
The tree of win EWS bot is red.
The ChangeLog doesn't explain why this change is necessary.
Comment on attachment 343738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343738&action=review > Source/WebKit/Shared/Authentication/cocoa/AuthenticationManagerCocoa.mm:43 > + if (!connection || !connection->xpcConnection()) { I don't understand this change. It seems like you should add your nullptr check, but KEEP the XPC_TYPE_CONNECTION check. Why are you removing the XPC type check? > Source/WebKit/UIProcess/Authentication/cocoa/AuthenticationChallengeProxyCocoa.mm:56 > + if (!m_connection->xpcConnection()) { Is the issue that sometimes we go to relay client certificate data to the network process, but the network process has been terminated?
Comment on attachment 343738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343738&action=review I will add a description as well. >> Source/WebKit/Shared/Authentication/cocoa/AuthenticationManagerCocoa.mm:43 >> + if (!connection || !connection->xpcConnection()) { > > I don't understand this change. It seems like you should add your nullptr check, but KEEP the XPC_TYPE_CONNECTION check. Why are you removing the XPC type check? I actually make a mistake last time when I added this check. What I was trying to do is to do the nullptr check. Then I discovered xpc_get_type(). Therefore I used it to do a more general check. However, xpc_get_type() doesn't do the nullptr check for its parameters... Therefore, we now crash here. Since nullptr check is what I actually need, I don't have to check the type of the object. >> Source/WebKit/UIProcess/Authentication/cocoa/AuthenticationChallengeProxyCocoa.mm:56 >> + if (!m_connection->xpcConnection()) { > > Is the issue that sometimes we go to relay client certificate data to the network process, but the network process has been terminated? I haven't observed any issues here. I added this just in case there are some future issues. I can remove this check here.
Created attachment 343825 [details] Patch
Created attachment 343826 [details] Patch
Comment on attachment 343826 [details] Patch r=me
Comment on attachment 343826 [details] Patch Thanks Brent for r+ this patch.
Comment on attachment 343826 [details] Patch Clearing flags on attachment: 343826 Committed r233340: <https://trac.webkit.org/changeset/233340>
All reviewed patches have been landed. Closing bug.