WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
189283
[WebAuthN] Polish WebAuthN auto-test environment
https://bugs.webkit.org/show_bug.cgi?id=189283
Summary
[WebAuthN] Polish WebAuthN auto-test environment
Jiewen Tan
Reported
2018-09-04 16:31:58 PDT
Currently, tests of WebAuthN spread in three places: 1) LayoutTests/http/tests/webauthn/ 2) LayoutTests/http/tests/wpt/webauthn/ 3) TestWebKitAPI/Tests/ios/LocalAuthenticator.mm It will be great to have them all in LayoutTests/http/tests/wpt/webauthn/. This could be achieved by: 1) making tests under LayoutTests/http/tests/webauthn/ become wpt async tests. 2) mocking transport layer abstraction, says HIDService, and providing necessary test configuration functions in testRunner.idl such that layout tests could then directly manipulate the most underlying layer of WebAuthN to have test behaviors/results they expected.
Attachments
Patch
(125.75 KB, patch)
2018-09-27 15:55 PDT
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Patch
(126.00 KB, patch)
2018-09-27 16:32 PDT
,
Jiewen Tan
cdumez
: review+
Details
Formatted Diff
Diff
Patch for landing
(125.96 KB, patch)
2018-09-28 16:02 PDT
,
Jiewen Tan
jiewen_tan
: commit-queue+
Details
Formatted Diff
Diff
Patch for landing
(125.89 KB, patch)
2018-09-28 16:21 PDT
,
Jiewen Tan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2018-09-04 16:34:47 PDT
<
rdar://problem/44117828
>
Jiewen Tan
Comment 2
2018-09-27 15:55:54 PDT
Created
attachment 351016
[details]
Patch
Jiewen Tan
Comment 3
2018-09-27 16:32:37 PDT
Created
attachment 351019
[details]
Patch
Chris Dumez
Comment 4
2018-09-28 15:42:15 PDT
Comment on
attachment 351019
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=351019&action=review
r=me
> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:582 > + MockWebAuthenticationConfiguration::Local local;
Any reason we are using a local variable instead of assigning straight into configuration.local?
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:77 > + if (!configuration.local.value().acceptAuthentication) {
Cannot we use onfiguration.local->acceptAuthentication ?
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:90 > + if (!configuration.local.value().acceptAuthentication) {
ditto.
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:105 > + if (!configuration.local.value().acceptAttestation) {
ditto.
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:118 > + (__bridge CFDataRef)adoptNS([[NSData alloc] initWithBase64EncodedString:configuration.local.value().privateKeyBase64 options:NSDataBase64DecodingIgnoreUnknownCharacters]).get(),
ditto.
> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalService.cpp:45 > + if (!m_configuration.local)
return !!m_configuration.local; ?
Jiewen Tan
Comment 5
2018-09-28 15:58:24 PDT
Comment on
attachment 351019
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=351019&action=review
Thanks Chris for r+ my patch.
>> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:582 >> + MockWebAuthenticationConfiguration::Local local; > > Any reason we are using a local variable instead of assigning straight into configuration.local?
configuration.local is a std::optional. How?
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:77 >> + if (!configuration.local.value().acceptAuthentication) { > > Cannot we use onfiguration.local->acceptAuthentication ?
Sure.
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:90 >> + if (!configuration.local.value().acceptAuthentication) { > > ditto.
Fixed.
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:105 >> + if (!configuration.local.value().acceptAttestation) { > > ditto.
Fixed.
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalConnection.mm:118 >> + (__bridge CFDataRef)adoptNS([[NSData alloc] initWithBase64EncodedString:configuration.local.value().privateKeyBase64 options:NSDataBase64DecodingIgnoreUnknownCharacters]).get(), > > ditto.
Fixed.
>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalService.cpp:45 >> + if (!m_configuration.local) > > return !!m_configuration.local; ?
I don't think so.
Chris Dumez
Comment 6
2018-09-28 15:59:44 PDT
Comment on
attachment 351019
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=351019&action=review
>>> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:582 >>> + MockWebAuthenticationConfiguration::Local local; >> >> Any reason we are using a local variable instead of assigning straight into configuration.local? > > configuration.local is a std::optional. How?
Ah, I missed that it was an optional. Please disregard this comment.
>>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalService.cpp:45 >>> + if (!m_configuration.local) >> >> return !!m_configuration.local; ? > > I don't think so.
Why not? :)
Jiewen Tan
Comment 7
2018-09-28 16:02:10 PDT
Comment hidden (obsolete)
Created
attachment 351134
[details]
Patch for landing
Jiewen Tan
Comment 8
2018-09-28 16:04:50 PDT
Comment on
attachment 351019
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=351019&action=review
>>>> Source/WebKit/UIProcess/WebAuthentication/Mock/MockLocalService.cpp:45 >>>> + if (!m_configuration.local) >>> >>> return !!m_configuration.local; ? >> >> I don't think so. > > Why not? :)
I was thinking you suggested me to replace !m_configuration.local with !!m_configuration.local. lol. Yup. Why not.
Jiewen Tan
Comment 9
2018-09-28 16:21:17 PDT
Created
attachment 351137
[details]
Patch for landing
WebKit Commit Bot
Comment 10
2018-09-28 17:02:49 PDT
Comment on
attachment 351137
[details]
Patch for landing Clearing flags on attachment: 351137 Committed
r236625
: <
https://trac.webkit.org/changeset/236625
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug