Bug 202561

<rdar://problem/55973910>

Created attachment 381658 [details] Patch

This patch here conflicts with Bug 202563 on some minor ways. Will rebase it depending on which one get landed first.

Comment on attachment 381658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381658&action=review > Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.cpp:53 > + m_transports.append(AuthenticatorTransport::Usb); uncheckedAppend if we have m_transports.reserveInitialCapacity > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:321 > + page->uiClient().runWebAuthenticationPanel(*page, panel, [transports = WTFMove(transports), weakPanel = makeWeakPtr(panel), weakThis = makeWeakPtr(*this), this] (WebAuthenticationPanelResult result) { Do we need to capture transports since we have weakPanel that stores roughly the same information? > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:358 > + processGoogleLegacyAppIdSupportExtension(options.extensions, transports); > + }, [&](const PublicKeyCredentialRequestOptions& options) { s/ / / > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h:87 > + virtual void respondReceivedInternal(Respond&&) { }; s/;//

Comment on attachment 381658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381658&action=review Thanks Youenn for r+ this patch. >> Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.cpp:53 >> + m_transports.append(AuthenticatorTransport::Usb); > > uncheckedAppend if we have m_transports.reserveInitialCapacity Thanks for catching me every time for this mistakes. Now, it is in my head. >> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:321 >> + page->uiClient().runWebAuthenticationPanel(*page, panel, [transports = WTFMove(transports), weakPanel = makeWeakPtr(panel), weakThis = makeWeakPtr(*this), this] (WebAuthenticationPanelResult result) { > > Do we need to capture transports since we have weakPanel that stores roughly the same information? No, probably not. The Panel only capture transports for external authenticators, i.e., USB and NFC. And we do have experimental support for Internal authenticators which are not going to expose to clients at this moment. >> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:358 >> + }, [&](const PublicKeyCredentialRequestOptions& options) { > > s/ / / Fixed. >> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h:87 >> + virtual void respondReceivedInternal(Respond&&) { }; > > s/;// Fixed.

Created attachment 381714 [details] Patch for landing

Comment on attachment 381714 [details] Patch for landing Rejecting attachment 381714 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 381714, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=381714&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=202561&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 381714 from bug 202561. Fetching: https://bugs.webkit.org/attachment.cgi?id=381714 Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 18 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebKit/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp patching file Source/WebCore/Modules/webauthn/WebAuthenticationConstants.h patching file Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.cpp Hunk #1 FAILED at 30. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.cpp.rej patching file Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.h Hunk #2 FAILED at 47. 1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/UIProcess/API/APIWebAuthenticationPanel.h.rej patching file Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.h patching file Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm patching file Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp patching file Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h patching file Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcService.h patching file Source/WebKit/UIProcess/WebAuthentication/Cocoa/NfcService.mm patching file Source/WebKit/UIProcess/WebAuthentication/Mock/MockAuthenticatorManager.cpp patching file Source/WebKit/UIProcess/WebAuthentication/Mock/MockAuthenticatorManager.h patching file Tools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj patching file Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm patching file Tools/TestWebKitAPI/Tests/WebKitCocoa/web-authentication-make-credential-hid.html Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/13170214

Committed r251498: <https://trac.webkit.org/changeset/251498>

Committed r251512: <https://trac.webkit.org/changeset/251512>

(In reply to Jiewen Tan from comment #9) > Committed r251512: <https://trac.webkit.org/changeset/251512> This is a speculative build fix.

Comment on attachment 381714 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=381714&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:284 > +TEST(WebAuthenticationPanel, PanelHidSuccess2) This test times out a lot on the bots. https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebAuthenticationPanel.PanelHidSuccess2

(In reply to Alex Christensen from comment #11) > Comment on attachment 381714 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381714&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:284 > > +TEST(WebAuthenticationPanel, PanelHidSuccess2) > > This test times out a lot on the bots. > https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI. > WebAuthenticationPanel.PanelHidSuccess2 Thanks, Alex. Will investigate.

*** Bug 203380 has been marked as a duplicate of this bug. ***

A test fix: Committed r251562: <https://trac.webkit.org/changeset/251562>

Another attempt to fix the test: Committed r251579: <https://trac.webkit.org/changeset/251579>

TestWebKitAPI.WebAuthenticationPanel.PanelHidSuccess2 is still failing, both on iOS and mac. It is slowing down EWS (since in case of any test failure, EWS has to retry the tests to check for flakiness, and and re-run the tests on clean tree to verify if the test is pre-existing). https://ews-build.webkit.org/#/builders/3 https://ews-build.webkit.org/#/builders/9

All commits were rolled out in https://trac.webkit.org/changeset/251602/webkit

Created attachment 382041 [details] Patch for relanding

Comment on attachment 382041 [details] Patch for relanding r=me. Please keep an eye on the Windows build failure -- I'm not sure why this change would impact it.

Created attachment 382242 [details] Patch for relanding

Created attachment 382245 [details] Patch for relanding

Comment on attachment 382245 [details] Patch for relanding Thanks Brent for r+ the patch too. I borrowed an EWS bot (159) from Aakash. And this patch passes consistently on that bot with both macOS and iOS simulator for 1000 times. cq+. Will watch out for other bots' behavior.

Created attachment 382255 [details] EWS 159 iOS simulator debug results

Comment on attachment 382245 [details] Patch for relanding Clearing flags on attachment: 382245 Committed r251762: <https://trac.webkit.org/changeset/251762>

*** Bug 204187 has been marked as a duplicate of this bug. ***