203937 2019-11-06 18:40:53 -0800 [WebAuthn] Return NotAllowedError immediately for UI cancellations 2019-11-18 11:09:12 -0800 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 181943 1 jiewen_tan jiewen_tan alex.gaynor bfulgham jiewen_tan kieun.shin webkit-bug-importer oldest_to_newest 1588063 0 jiewen_tan 2019-11-06 18:40:53 -0800 return NotAllowedError immediately for UI cancellations. 1588064 1 jiewen_tan 2019-11-06 18:41:17 -0800 <rdar://problem/56962420> 1588070 2 383003 jiewen_tan 2019-11-06 18:47:59 -0800 Created attachment 383003 Patch 1588506 3 383003 bfulgham 2019-11-07 17:09:05 -0800 Comment on attachment 383003 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383003&action=review r=me, but I would make the patch as small as possible to make it easier to integrate quickly. > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 > + clearState(); To keep the patch small, I would just keep using resetState here. > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-217 > - resetState(); Then you don't need this change. > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:-357 > -void AuthenticatorManager::resetState() ... or this change. > Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.h:-95 > - void resetState(); Or this change. > Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:562 > + EXPECT_TRUE(webAuthenticationPanelFailed); Nice! 1588519 4 383003 jiewen_tan 2019-11-07 17:29:36 -0800 Comment on attachment 383003 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383003&action=review Thanks Brent for r+ this change. >> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 >> + clearState(); > > To keep the patch small, I would just keep using resetState here. I think this patch is already small enough for quick integration. Splitting this change into two doesn't seem necessary. 1588653 5 383003 bfulgham 2019-11-08 08:38:08 -0800 Comment on attachment 383003 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383003&action=review >>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 >>> + clearState(); >> >> To keep the patch small, I would just keep using resetState here. > > I think this patch is already small enough for quick integration. Splitting this change into two doesn't seem necessary. I'm not suggesting you split the change; I just think it's unnecessary to remove the 'resetState' method. Just keep it as-is. 1588734 6 383003 jiewen_tan 2019-11-08 11:24:24 -0800 Comment on attachment 383003 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383003&action=review >>>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 >>>> + clearState(); >>> >>> To keep the patch small, I would just keep using resetState here. >> >> I think this patch is already small enough for quick integration. Splitting this change into two doesn't seem necessary. > > I'm not suggesting you split the change; I just think it's unnecessary to remove the 'resetState' method. Just keep it as-is. The reason to have the 'resetState' method is to avoid the m_pendingCompletionHandler check in 'clearState', and is specifically crafted for this method. Now this method will invoke m_pendingCompletionHandler as well, and therefore there is no need to have the 'resetState' method anymore. That's why I remove it. 1588735 7 bfulgham 2019-11-08 11:26:24 -0800 (In reply to Jiewen Tan from comment #6) > Comment on attachment 383003 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383003&action=review > > >>>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 > >>>> + clearState(); > >>> > >>> To keep the patch small, I would just keep using resetState here. > >> > >> I think this patch is already small enough for quick integration. Splitting this change into two doesn't seem necessary. > > > > I'm not suggesting you split the change; I just think it's unnecessary to remove the 'resetState' method. Just keep it as-is. > > The reason to have the 'resetState' method is to avoid the > m_pendingCompletionHandler check in 'clearState', and is specifically > crafted for this method. Now this method will invoke > m_pendingCompletionHandler as well, and therefore there is no need to have > the 'resetState' method anymore. That's why I remove it. Fine. Let's just get it landed so I can try to get it in. 1588740 8 jiewen_tan 2019-11-08 11:37:00 -0800 (In reply to Brent Fulgham from comment #7) > (In reply to Jiewen Tan from comment #6) > > Comment on attachment 383003 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=383003&action=review > > > > >>>> Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:202 > > >>>> + clearState(); > > >>> > > >>> To keep the patch small, I would just keep using resetState here. > > >> > > >> I think this patch is already small enough for quick integration. Splitting this change into two doesn't seem necessary. > > > > > > I'm not suggesting you split the change; I just think it's unnecessary to remove the 'resetState' method. Just keep it as-is. > > > > The reason to have the 'resetState' method is to avoid the > > m_pendingCompletionHandler check in 'clearState', and is specifically > > crafted for this method. Now this method will invoke > > m_pendingCompletionHandler as well, and therefore there is no need to have > > the 'resetState' method anymore. That's why I remove it. > > Fine. Let's just get it landed so I can try to get it in. Thanks, Brent. 1588745 9 jiewen_tan 2019-11-08 11:44:41 -0800 Committed r252248: <https://trac.webkit.org/changeset/252248> 1591121 10 jiewen_tan 2019-11-18 11:09:12 -0800 *** Bug 204186 has been marked as a duplicate of this bug. *** 383003 2019-11-06 18:47:59 -0800 2019-11-07 17:09:05 -0800 Patch bug-203937-20191106184758.patch text/plain 4927 jiewen_tan U3VidmVyc2lvbiBSZXZpc2lvbjogMjUyMTYxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDkzMWEwY2MxMTVhODE4OTdh MWE3NTlmMWEwNWExZjhiN2Y0YWI3NmUuLjQ1YmEwNjlmODYzY2UwM2QzMmJkOTNhM2YwMGMzNDVj MjlmNTFmODMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIgQEAKKzIwMTktMTEtMDYgIEppZXdlbiBU YW4gIDxqaWV3ZW5fdGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbV2ViQXV0aG5dIFJldHVybiBO b3RBbGxvd2VkRXJyb3IgaW1tZWRpYXRlbHkgZm9yIFVJIGNhbmNlbGxhdGlvbnMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwMzkzNworICAgICAgICA8 cmRhcjovL3Byb2JsZW0vNTY5NjI0MjA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgTm90QWxsb3dlZEVycm9yIHJlcHJlc2VudGluZyBVSSBjYW5jZWxs YXRpb25zIHNob3VsZCBiZSByZXR1cm5lZCB0byBzaXRlcworICAgICAgICBpbW1lZGlhdGVseSBz dWNoIHRoYXQgc2l0ZXMgY291bGQgc2hvdyBhcHByb3ByaWF0ZSBlcnJvciBwYWdlIGltbWVkaWF0 ZWx5LgorCisgICAgICAgIENvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMuCisKKyAgICAgICAgKiBV SVByb2Nlc3MvV2ViQXV0aGVudGljYXRpb24vQXV0aGVudGljYXRvck1hbmFnZXIuY3BwOgorICAg ICAgICAoV2ViS2l0OjpBdXRoZW50aWNhdG9yTWFuYWdlcjo6Y2FuY2VsUmVxdWVzdCk6CisgICAg ICAgIChXZWJLaXQ6OkF1dGhlbnRpY2F0b3JNYW5hZ2VyOjpjbGVhclN0YXRlKToKKyAgICAgICAg KFdlYktpdDo6QXV0aGVudGljYXRvck1hbmFnZXI6OnJlc2V0U3RhdGUpOiBEZWxldGVkLgorICAg ICAgICAqIFVJUHJvY2Vzcy9XZWJBdXRoZW50aWNhdGlvbi9BdXRoZW50aWNhdG9yTWFuYWdlci5o OgorCiAyMDE5LTExLTA2ICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAg ICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjI1MTkwNy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQvVUlQcm9jZXNzL1dlYkF1dGhlbnRpY2F0aW9uL0F1dGhlbnRpY2F0b3JNYW5hZ2VyLmNw cCBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYkF1dGhlbnRpY2F0aW9uL0F1dGhlbnRpY2F0 b3JNYW5hZ2VyLmNwcAppbmRleCA2ZjM3MWQ3ZTM2ZmExZmE0MmZjZGZmNDNlZjFkODYwODA2NjEy NWZlLi5lOGM0MGViNDc4OThjNzc0ZTUzMGRlNWIwNDBlZTk3MDVjZTBlYTk1IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9XZWJBdXRoZW50aWNhdGlvbi9BdXRoZW50aWNhdG9y TWFuYWdlci5jcHAKKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvV2ViQXV0aGVudGljYXRp b24vQXV0aGVudGljYXRvck1hbmFnZXIuY3BwCkBAIC0xOTgsNyArMTk4LDkgQEAgdm9pZCBBdXRo ZW50aWNhdG9yTWFuYWdlcjo6Y2FuY2VsUmVxdWVzdChjb25zdCBBUEk6OldlYkF1dGhlbnRpY2F0 aW9uUGFuZWwmIHBhbmUKICAgICBSRUxFQVNFX0FTU0VSVChSdW5Mb29wOjppc01haW4oKSk7CiAg ICAgaWYgKCFtX3BlbmRpbmdDb21wbGV0aW9uSGFuZGxlciB8fCBtX3BlbmRpbmdSZXF1ZXN0RGF0 YS5wYW5lbC5nZXQoKSAhPSAmcGFuZWwpCiAgICAgICAgIHJldHVybjsKLSAgICByZXNldFN0YXRl KCk7CisgICAgaW52b2tlUGVuZGluZ0NvbXBsZXRpb25IYW5kbGVyKEV4Y2VwdGlvbkRhdGEgeyBO b3RBbGxvd2VkRXJyb3IsICJUaGlzIHJlcXVlc3QgaGFzIGJlZW4gY2FuY2VsbGVkIGJ5IHRoZSB1 c2VyLiJfcyB9KTsKKyAgICBjbGVhclN0YXRlKCk7CisgICAgbV9yZXF1ZXN0VGltZU91dFRpbWVy LnN0b3AoKTsKIH0KIAogdm9pZCBBdXRoZW50aWNhdG9yTWFuYWdlcjo6Y2xlYXJTdGF0ZUFzeW5j KCkKQEAgLTIxNCw3ICsyMTYsOSBAQCB2b2lkIEF1dGhlbnRpY2F0b3JNYW5hZ2VyOjpjbGVhclN0 YXRlKCkKIHsKICAgICBpZiAobV9wZW5kaW5nQ29tcGxldGlvbkhhbmRsZXIpCiAgICAgICAgIHJl dHVybjsKLSAgICByZXNldFN0YXRlKCk7CisgICAgbV9hdXRoZW50aWNhdG9ycy5jbGVhcigpOwor ICAgIG1fc2VydmljZXMuY2xlYXIoKTsKKyAgICBtX3BlbmRpbmdSZXF1ZXN0RGF0YSA9IHsgfTsK IH0KIAogdm9pZCBBdXRoZW50aWNhdG9yTWFuYWdlcjo6YXV0aGVudGljYXRvckFkZGVkKFJlZjxB dXRoZW50aWNhdG9yPiYmIGF1dGhlbnRpY2F0b3IpCkBAIC0zNTQsMTMgKzM1OCw2IEBAIHZvaWQg QXV0aGVudGljYXRvck1hbmFnZXI6Omludm9rZVBlbmRpbmdDb21wbGV0aW9uSGFuZGxlcihSZXNw b25kJiYgcmVzcG9uZCkKICAgICBtX3BlbmRpbmdDb21wbGV0aW9uSGFuZGxlcihXVEZNb3ZlKHJl c3BvbmQpKTsKIH0KIAotdm9pZCBBdXRoZW50aWNhdG9yTWFuYWdlcjo6cmVzZXRTdGF0ZSgpCi17 Ci0gICAgbV9hdXRoZW50aWNhdG9ycy5jbGVhcigpOwotICAgIG1fc2VydmljZXMuY2xlYXIoKTsK LSAgICBtX3BlbmRpbmdSZXF1ZXN0RGF0YSA9IHsgfTsKLX0KLQogdm9pZCBBdXRoZW50aWNhdG9y TWFuYWdlcjo6cmVzdGFydERpc2NvdmVyeSgpCiB7CiAgICAgZm9yIChhdXRvJiBzZXJ2aWNlIDog bV9zZXJ2aWNlcykKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYkF1dGhl bnRpY2F0aW9uL0F1dGhlbnRpY2F0b3JNYW5hZ2VyLmggYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vz cy9XZWJBdXRoZW50aWNhdGlvbi9BdXRoZW50aWNhdG9yTWFuYWdlci5oCmluZGV4IDA1YTI0ODc4 MGQ4Y2MwYjBlM2Q2ZjM3NjVjMjY2YTE0OTFkNGY1NGIuLmVlZTI0ZWExYWQ0ZTMxNTJmOWQ3NzU4 NGY3ZWI3ZGQwNjMyNDIyN2UgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dl YkF1dGhlbnRpY2F0aW9uL0F1dGhlbnRpY2F0b3JNYW5hZ2VyLmgKKysrIGIvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvV2ViQXV0aGVudGljYXRpb24vQXV0aGVudGljYXRvck1hbmFnZXIuaApAQCAt OTIsNyArOTIsNiBAQCBwcml2YXRlOgogICAgIHZvaWQgaW5pdFRpbWVPdXRUaW1lcigpOwogICAg IHZvaWQgdGltZU91dFRpbWVyRmlyZWQoKTsKICAgICB2b2lkIHJ1blBhbmVsKCk7Ci0gICAgdm9p ZCByZXNldFN0YXRlKCk7CiAgICAgdm9pZCByZXN0YXJ0RGlzY292ZXJ5KCk7CiAgICAgVHJhbnNw b3J0U2V0IGdldFRyYW5zcG9ydHMoKSBjb25zdDsKIApkaWZmIC0tZ2l0IGEvVG9vbHMvQ2hhbmdl TG9nIGIvVG9vbHMvQ2hhbmdlTG9nCmluZGV4IGY4YmViMTExNGRiOGI1M2M5ZjM3ZWRmZWEzNjZj MzAzMzg0OGQyZmMuLjA5MjVhODcwNWM0ZjQxNGQyMjI4OWM5Zjk0NmQ2ZTQyZmU1NTNjNzEgMTAw NjQ0Ci0tLSBhL1Rvb2xzL0NoYW5nZUxvZworKysgYi9Ub29scy9DaGFuZ2VMb2cKQEAgLTEsMyAr MSwxNCBAQAorMjAxOS0xMS0wNiAgSmlld2VuIFRhbiAgPGppZXdlbl90YW5AYXBwbGUuY29tPgor CisgICAgICAgIFtXZWJBdXRobl0gUmV0dXJuIE5vdEFsbG93ZWRFcnJvciBpbW1lZGlhdGVseSBm b3IgVUkgY2FuY2VsbGF0aW9ucworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjAzOTM3CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS81Njk2MjQyMD4KKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFRlc3RXZWJL aXRBUEkvVGVzdHMvV2ViS2l0Q29jb2EvX1dLV2ViQXV0aGVudGljYXRpb25QYW5lbC5tbToKKyAg ICAgICAgKFRlc3RXZWJLaXRBUEk6OlRFU1QpOgorCiAyMDE5LTExLTA2ICBLZWl0aCBSb2xsaW4g IDxrcm9sbGluQGFwcGxlLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkIGJ1aWxkIGZpeDogZml4 IFRlc3RXZWJLaXRBUEkgaW4gbWFjQ2F0YWx5c3QsIHR2T1MsIGFuZApkaWZmIC0tZ2l0IGEvVG9v bHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXRDb2NvYS9fV0tXZWJBdXRoZW50aWNhdGlvblBh bmVsLm1tIGIvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XZWJLaXRDb2NvYS9fV0tXZWJBdXRo ZW50aWNhdGlvblBhbmVsLm1tCmluZGV4IGYyM2NmYzc5MzE2YTMxMzdhNjk1Y2Q1OWEzMTcyMjcx YTAyOTUwMGYuLjY2Zjc0ZDM4MmRjNGQwZGNiZWM1YTZmYTQwNmQ0YzQ0NzllZjljOGMgMTAwNjQ0 Ci0tLSBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2ViS2l0Q29jb2EvX1dLV2ViQXV0aGVu dGljYXRpb25QYW5lbC5tbQorKysgYi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdENv Y29hL19XS1dlYkF1dGhlbnRpY2F0aW9uUGFuZWwubW0KQEAgLTU1OCw5ICs1NTgsOCBAQCBURVNU KFdlYkF1dGhlbnRpY2F0aW9uUGFuZWwsIFBhbmVsSGlkQ2FuY2VsKQogICAgIFt3ZWJWaWV3IGxv YWRSZXF1ZXN0OltOU1VSTFJlcXVlc3QgcmVxdWVzdFdpdGhVUkw6dGVzdFVSTC5nZXQoKV1dOwog ICAgIFV0aWw6OnJ1bigmd2ViQXV0aGVudGljYXRpb25QYW5lbFJhbik7CiAgICAgW1tkZWxlZ2F0 ZSBwYW5lbF0gY2FuY2VsXTsKLSAgICBbd2ViVmlldyB3YWl0Rm9yTWVzc2FnZTpAIk9wZXJhdGlv biB0aW1lZCBvdXQuIl07Ci0gICAgRVhQRUNUX0ZBTFNFKHdlYkF1dGhlbnRpY2F0aW9uUGFuZWxG YWlsZWQpOwotICAgIEVYUEVDVF9GQUxTRSh3ZWJBdXRoZW50aWNhdGlvblBhbmVsU3VjY2VkZWQp OworICAgIFt3ZWJWaWV3IHdhaXRGb3JNZXNzYWdlOkAiVGhpcyByZXF1ZXN0IGhhcyBiZWVuIGNh bmNlbGxlZCBieSB0aGUgdXNlci4iXTsKKyAgICBFWFBFQ1RfVFJVRSh3ZWJBdXRoZW50aWNhdGlv blBhbmVsRmFpbGVkKTsKIH0KIAogVEVTVChXZWJBdXRoZW50aWNhdGlvblBhbmVsLCBQYW5lbEhp ZEN0YXBOb0NyZWRlbnRpYWxzRm91bmQpCg==