This task aims to polish AuthenticatorManager such that: 1) it is no longer a singleton. Instead, it will live with Page. It was a singleton simply because static PublicKeyCredential::isUserVerifyingPlatformAuthenticatorAvailable() have to access it. However, the issue can be solved by adding an attribute [CallWith=Document]. Therefore, there is no such need. Also, the singleton is illy implemented as it also owns a single IPC proxy to UI Process which means different web pages will talk to the same web page proxy. Anyway, making it live with Page should solve the problem. 2) Since we are now planning to support external authenticators, the manager of all authenticators will then have to live in UI Process which makes this AuthenticatorManager obsolete. Instead, rename it to AuthenticatorCoordinator. 3) Rename CredentialsMessenger to AuthenticatorCoordinatorClient to tight it to WebAuthN. Also, simplify the message reply model as PublicKeyCredentialCreationOptions/PublicKeyCredentialRequestOptions => ExceptionData/PublicKeyCredential for makeCredential/getAssertion operations.
<rdar://problem/44115936>
Created attachment 349266 [details] Patch
Created attachment 349274 [details] Patch
Created attachment 349318 [details] Patch
Comment on attachment 349318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349318&action=review r=me with suggestions. > Source/WebCore/Modules/webauthn/PublicKeyCredentialCreationOptions.h:98 > + encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(user.id.data()), user.id.length(), 1); Why do we need the reinterpret_cast here? Also, it seems like this could construct a temporary IPC::DataReference and encode that. > Source/WebCore/page/Page.h:910 > + std::unique_ptr<AuthenticatorCoordinator> m_authenticatorCoordinator; Could this be a UniqueRef instead of a unique_ptr? It seems it cannot be null.
Comment on attachment 349318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349318&action=review Thanks Chris for r+ this patch. >> Source/WebCore/Modules/webauthn/PublicKeyCredentialCreationOptions.h:98 >> + encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(user.id.data()), user.id.length(), 1); > > Why do we need the reinterpret_cast here? > > Also, it seems like this could construct a temporary IPC::DataReference and encode that. It doesn't need to. IPC::DataReference is in WebKit so I could't use it in WebCore. >> Source/WebCore/page/Page.h:910 >> + std::unique_ptr<AuthenticatorCoordinator> m_authenticatorCoordinator; > > Could this be a UniqueRef instead of a unique_ptr? It seems it cannot be null. Sure, I was not aware of UniqueRef.
Created attachment 349336 [details] Patch for landing
Created attachment 349347 [details] Patch for landing
Comment on attachment 349347 [details] Patch for landing Added #include <JavaScriptCore/HeapInlines.h> to VRStageParameters.cpp to resolve the Win EWS bots linker error.
Comment on attachment 349347 [details] Patch for landing Clearing flags on attachment: 349347 Committed r235888: <https://trac.webkit.org/changeset/235888>
Build fix: Committed r235900: <https://trac.webkit.org/changeset/235900>