Bug 193150

<rdar://problem/47054028>

Created attachment 358358 [details] Patch

Created attachment 358399 [details] Patch

Attachment 358399 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/webauthn/WebAuthenticationConstants.h:41: P_256 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 358399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358399&action=review R=me > Source/WebCore/ChangeLog:20 > + Besides importing stuffs from Chroimum, it also gathers a bunch of constants and helper functions into WebAuthenticationConstants.h Besides importing stuff from Chromium ... > Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:90 > + if (parameters.alg == COSE::ES256) This really seems like something a std::find would work for. But it’s fine as-is. > Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:114 > + return constructU2fSignCommand(produceRpIdHash(request.rp.id), clientDataHash, keyHandle.idVector, true /* checkOnly */); The need for this comment indicates this should be an enumeration! But since this is imported code, land as-is and then post a patch to clean up the interface. > Source/WebCore/Modules/webauthn/fido/U2fResponseConverter.cpp:71 > + x.append(u2fData.data() + pos, ES256FieldElementLength); Can the vector be constructed directly with this data? Or at least sized properly, rather than having a separate allocation event, then a resize as we append. This is true for all such cases. > Source/WebCore/Modules/webgpu/WebGPUCommandBuffer.cpp:34 > +#include "GPURenderPipeline.h" Is this needed as part of this patch? Was it causing a build error for you?

Comment on attachment 358399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358399&action=review Thanks Brent for r+ this patch. >> Source/WebCore/ChangeLog:20 >> + Besides importing stuffs from Chroimum, it also gathers a bunch of constants and helper functions into WebAuthenticationConstants.h > > Besides importing stuff from Chromium ... Fixed. >> Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:90 >> + if (parameters.alg == COSE::ES256) > > This really seems like something a std::find would work for. But it’s fine as-is. You are right! Fixed. >> Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:114 >> + return constructU2fSignCommand(produceRpIdHash(request.rp.id), clientDataHash, keyHandle.idVector, true /* checkOnly */); > > The need for this comment indicates this should be an enumeration! But since this is imported code, land as-is and then post a patch to clean up the interface. Sure. >> Source/WebCore/Modules/webauthn/fido/U2fResponseConverter.cpp:71 >> + x.append(u2fData.data() + pos, ES256FieldElementLength); > > Can the vector be constructed directly with this data? Or at least sized properly, rather than having a separate allocation event, then a resize as we append. > > This is true for all such cases. It looks like that the default cntr of Vector does nothing. Therefore we will have only one allocation event when we append. >> Source/WebCore/Modules/webgpu/WebGPUCommandBuffer.cpp:34 >> +#include "GPURenderPipeline.h" > > Is this needed as part of this patch? Was it causing a build error for you? Some kind of unified source build magic...

Created attachment 358470 [details] Patch for landing

Comment on attachment 358470 [details] Patch for landing Clearing flags on attachment: 358470 Committed r239665: <https://trac.webkit.org/changeset/239665>