Adopt new UI for the Platform Authenticator makeCredential flow.
<rdar://problem/72154774>
Created attachment 415966 [details] Patch
Created attachment 415970 [details] Patch
Comment on attachment 415970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415970&action=review > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:237 > + // This is for the new UI. I don't think you need this comment since the next line is conditional on the new UI. > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:315 > + SecAccessControlRef accessControlRef = accessControl.get(); Does accessControlRef need to be retained in some fashion? Will calling 'verifyUser' with this be safe? > Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:119 > + [context evaluateAccessControl:accessControl operation:LAAccessControlOperationUseKeySign options:options.get() reply:reply.get()]; This method call seems to use 'accessControl', which is not guaranteed to still live since you moved the underlying object in the calling method.
Comment on attachment 415970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415970&action=review Thanks Brent for reviewing this patch. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:237 >> + // This is for the new UI. > > I don't think you need this comment since the next line is conditional on the new UI. Removed. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:315 >> + SecAccessControlRef accessControlRef = accessControl.get(); > > Does accessControlRef need to be retained in some fashion? Will calling 'verifyUser' with this be safe? Yes, it is retained in the callback. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:119 >> + [context evaluateAccessControl:accessControl operation:LAAccessControlOperationUseKeySign options:options.get() reply:reply.get()]; > > This method call seems to use 'accessControl', which is not guaranteed to still live since you moved the underlying object in the calling method. accessControl is held by the completionHandler, which is the held by reply. As long as the callee doesn't destruct the reply before verifying the accessControl, it should be safe.
Comment on attachment 415970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415970&action=review r=me >>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:315 >>> + SecAccessControlRef accessControlRef = accessControl.get(); >> >> Does accessControlRef need to be retained in some fashion? Will calling 'verifyUser' with this be safe? > > Yes, it is retained in the callback. Ah! Gotcha, My mistake! >>> Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm:119 >>> + [context evaluateAccessControl:accessControl operation:LAAccessControlOperationUseKeySign options:options.get() reply:reply.get()]; >> >> This method call seems to use 'accessControl', which is not guaranteed to still live since you moved the underlying object in the calling method. > > accessControl is held by the completionHandler, which is the held by reply. As long as the callee doesn't destruct the reply before verifying the accessControl, it should be safe. Got it.
Comment on attachment 415970 [details] Patch Thanks Brent for r+ this patch.
Committed r270694: <https://trac.webkit.org/changeset/270694>
A build fix: Committed r270721: <https://trac.webkit.org/changeset/270721>