Bug 204024

<rdar://problem/56962320>

Created attachment 383167 [details] Patch

Comment on attachment 383167 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383167&action=review r=me, but please run the autoreleasepool stuff by David before committing. > Source/WebKit/ChangeLog:12 > + of those legacy key. actually speak U2F, indicating we are interacting with one of these legacy keys. > Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:52 > +{ Should the code in here be wrapped in @autoreleasepool? > Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:53 > + auto versionData = [session transceive:adoptNS([[NSData alloc] initWithBytes:kCtapNfcAppletSelectionCommand length:sizeof(kCtapNfcAppletSelectionCommand)]).get()]; auto *versionData maybe? > Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:61 > + // of those legacy key. Same rephrasing I suggest above. > Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:-71 > - @autoreleasepool { Why don't you need the @autoreleasepool block anymore? Did you check with David Kilzer to see if this is good to do? > Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:94 > + auto responseData = [m_session transceive:adoptNS([[NSData alloc] initWithBytes:data.data() length:data.size()]).get()]; auto *responseData =

CCing David Kilzer to look at the @autoreleasepool stuff.

Comment on attachment 383167 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383167&action=review Thanks Brent for r+ this patch. >> Source/WebKit/ChangeLog:12 >> + of those legacy key. > > actually speak U2F, indicating we are interacting with one of these legacy keys. Fixed. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:52 >> +{ > > Should the code in here be wrapped in @autoreleasepool? Before, I used [NSData dataWithBytes:data.data() length:data.size()] which is not wrapped with adoptNS. Now, I switched to adoptNS([[NSData alloc] initWithBytes:kCtapNfcAppletSelectionCommand length:sizeof(kCtapNfcAppletSelectionCommand)]).get(). It should be fine. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:53 >> + auto versionData = [session transceive:adoptNS([[NSData alloc] initWithBytes:kCtapNfcAppletSelectionCommand length:sizeof(kCtapNfcAppletSelectionCommand)]).get()]; > > auto *versionData maybe? Yes. Right. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:61 >> + // of those legacy key. > > Same rephrasing I suggest above. Fixed. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:-71 >> - @autoreleasepool { > > Why don't you need the @autoreleasepool block anymore? Did you check with David Kilzer to see if this is good to do? Same as above. >> Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcConnection.mm:94 >> + auto responseData = [m_session transceive:adoptNS([[NSData alloc] initWithBytes:data.data() length:data.size()]).get()]; > > auto *responseData = Fixed.

Created attachment 383177 [details] Patch for landing

Committed r252297: <https://trac.webkit.org/changeset/252297>