194823 2019-02-19 11:40:39 -0800 Always call CompletionHandlers after r240909 2019-02-20 14:59:53 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 achristensen achristensen beidson cdumez commit-queue dbates ews-watchlist ggaren japhet rniwa webkit-bug-importer oldest_to_newest 1507743 0 achristensen 2019-02-19 11:40:39 -0800 Always call CompletionHandlers after r240909 1507744 1 362404 achristensen 2019-02-19 11:40:59 -0800 Created attachment 362404 Patch 1507770 2 rniwa 2019-02-19 12:23:27 -0800 Looks like the patch isn't building?? 1507781 3 362417 achristensen 2019-02-19 12:57:20 -0800 Created attachment 362417 Patch 1507801 4 362417 rniwa 2019-02-19 13:48:12 -0800 Comment on attachment 362417 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362417&action=review > Source/WebCore/loader/PolicyChecker.cpp:196 > if (!responseIdentifier.isValidFor(requestIdentifier)) > - return; > + return function({ }, nullptr, NavigationPolicyDecision::IgnoreLoad); I don't think this is quite right. When the response identifier is less than what we have, it means the response was for some other resource we had requested a policy check in the past. It’s still possible to receive the right policy check response in the future. 1507816 5 achristensen 2019-02-19 14:18:23 -0800 We definitely need to do something with the CompletionHandlers. If we don't call them we need to store them somewhere. 1507965 6 rniwa 2019-02-19 19:01:50 -0800 (In reply to Alex Christensen from comment #5) > We definitely need to do something with the CompletionHandlers. If we don't > call them we need to store them somewhere. Oh, that's a good point. Maybe we DO need to fail in this case because we don't store the completion handler anywhere. 1507966 7 rniwa 2019-02-19 19:02:55 -0800 But hang on, if we did that, wouldn't it mean that WebKit layer's listener map would still contain a stale completion handler which would never get called anyway? 1508121 8 achristensen 2019-02-20 10:17:38 -0800 No 1508215 9 rniwa 2019-02-20 13:35:38 -0800 (In reply to Alex Christensen from comment #8) > No Why not? If we've reached this point, it means that we've got listenerID confused, and invoked the wrong listener with a policy check response. That would mean that the other listener for which this policy check response was really meant for would be left in the map. How does the other listener ever get a policy check response? 1508233 10 achristensen 2019-02-20 14:05:04 -0800 listenerID corresponds to WebFrame's m_policyFunction, and if they don't line up then WebFrame::invalidatePolicyListener calls the CompletionHandler owned by the WebFrame in that case. The CompletionHandlers being fixed by this patch are not owned by anything. They need to be called or owned. 1508235 11 rniwa 2019-02-20 14:07:29 -0800 (In reply to Alex Christensen from comment #10) > listenerID corresponds to WebFrame's m_policyFunction, and if they don't > line up then WebFrame::invalidatePolicyListener calls the CompletionHandler > owned by the WebFrame in that case. Oh, that's a good point. 1508278 12 362417 commit-queue 2019-02-20 14:52:24 -0800 Comment on attachment 362417 Patch Clearing flags on attachment: 362417 Committed r241842: <https://trac.webkit.org/changeset/241842> 1508279 13 commit-queue 2019-02-20 14:52:25 -0800 All reviewed patches have been landed. Closing bug. 1508284 14 webkit-bug-importer 2019-02-20 14:59:53 -0800 <rdar://problem/48253351> 362404 2019-02-19 11:40:59 -0800 2019-02-19 12:57:18 -0800 Patch bug-194823-20190219114058.patch text/plain 1776 achristensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI0MTc2NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE5LTAyLTE5ICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgQWx3YXlzIGNh bGwgQ29tcGxldGlvbkhhbmRsZXJzIGFmdGVyIHIyNDA5MDkKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5NDgyMworCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogbG9hZGVyL1BvbGljeUNoZWNrZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6UG9saWN5Q2hlY2tlcjo6Y2hlY2tOYXZpZ2F0aW9uUG9saWN5KToK KyAgICAgICAgKFdlYkNvcmU6OlBvbGljeUNoZWNrZXI6OmNoZWNrTmV3V2luZG93UG9saWN5KToK KwogMjAxOS0wMi0xOSAgQ29tbWl0IFF1ZXVlICA8Y29tbWl0LXF1ZXVlQHdlYmtpdC5vcmc+CiAK ICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjI0MTcyMi4KSW5kZXg6IFNvdXJjZS9X ZWJDb3JlL2xvYWRlci9Qb2xpY3lDaGVja2VyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9sb2FkZXIvUG9saWN5Q2hlY2tlci5jcHAJKHJldmlzaW9uIDI0MTc1MykKKysrIFNvdXJj ZS9XZWJDb3JlL2xvYWRlci9Qb2xpY3lDaGVja2VyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTkz LDcgKzE5Myw3IEBAIHZvaWQgUG9saWN5Q2hlY2tlcjo6Y2hlY2tOYXZpZ2F0aW9uUG9saWMKICAg ICAgICAgIGJsb2JVUkxMaWZldGltZUV4dGVuc2lvbiA9IFdURk1vdmUoYmxvYlVSTExpZmV0aW1l RXh0ZW5zaW9uKSwgcmVxdWVzdElkZW50aWZpZXJdIChQb2xpY3lBY3Rpb24gcG9saWN5QWN0aW9u LCBQb2xpY3lDaGVja0lkZW50aWZpZXIgcmVzcG9uc2VJZGVudGlmaWVyKSBtdXRhYmxlIHsKIAog ICAgICAgICBpZiAoIXJlc3BvbnNlSWRlbnRpZmllci5pc1ZhbGlkRm9yKHJlcXVlc3RJZGVudGlm aWVyKSkKLSAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAgICAgIHJldHVybiBmdW5jdGlvbih7 IH0sIG51bGxwdHIsIE5hdmlnYXRpb25Qb2xpY3lEZWNpc2lvbjo6SWdub3JlTG9hZCk7CiAKICAg ICAgICAgbV9kZWxlZ2F0ZUlzRGVjaWRpbmdOYXZpZ2F0aW9uUG9saWN5ID0gZmFsc2U7CiAKQEAg LTIzNCw3ICsyMzQsNyBAQCB2b2lkIFBvbGljeUNoZWNrZXI6OmNoZWNrTmV3V2luZG93UG9saWN5 CiAgICAgICAgIHJlcXVlc3RJZGVudGlmaWVyXSAoUG9saWN5QWN0aW9uIHBvbGljeUFjdGlvbiwg UG9saWN5Q2hlY2tJZGVudGlmaWVyIHJlc3BvbnNlSWRlbnRpZmllcikgbXV0YWJsZSB7CiAKICAg ICAgICAgaWYgKCFyZXNwb25zZUlkZW50aWZpZXIuaXNWYWxpZEZvcihyZXF1ZXN0SWRlbnRpZmll cikpCi0gICAgICAgICAgICByZXR1cm47CisgICAgICAgICAgICByZXR1cm4gZnVuY3Rpb24oeyB9 LCBudWxscHRyLCBOYXZpZ2F0aW9uUG9saWN5RGVjaXNpb246Oklnbm9yZUxvYWQpOwogCiAgICAg ICAgIHN3aXRjaCAocG9saWN5QWN0aW9uKSB7CiAgICAgICAgIGNhc2UgUG9saWN5QWN0aW9uOjpE b3dubG9hZDoK 362417 2019-02-19 12:57:20 -0800 2019-02-20 14:52:24 -0800 Patch bug-194823-20190219125719.patch text/plain 1768 achristensen SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI0MTc2NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE5LTAyLTE5ICBBbGV4IENo cmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtpdC5vcmc+CisKKyAgICAgICAgQWx3YXlzIGNh bGwgQ29tcGxldGlvbkhhbmRsZXJzIGFmdGVyIHIyNDA5MDkKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5NDgyMworCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogbG9hZGVyL1BvbGljeUNoZWNrZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6UG9saWN5Q2hlY2tlcjo6Y2hlY2tOYXZpZ2F0aW9uUG9saWN5KToK KyAgICAgICAgKFdlYkNvcmU6OlBvbGljeUNoZWNrZXI6OmNoZWNrTmV3V2luZG93UG9saWN5KToK KwogMjAxOS0wMi0xOSAgQ29tbWl0IFF1ZXVlICA8Y29tbWl0LXF1ZXVlQHdlYmtpdC5vcmc+CiAK ICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjI0MTcyMi4KSW5kZXg6IFNvdXJjZS9X ZWJDb3JlL2xvYWRlci9Qb2xpY3lDaGVja2VyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9sb2FkZXIvUG9saWN5Q2hlY2tlci5jcHAJKHJldmlzaW9uIDI0MTc1MykKKysrIFNvdXJj ZS9XZWJDb3JlL2xvYWRlci9Qb2xpY3lDaGVja2VyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTkz LDcgKzE5Myw3IEBAIHZvaWQgUG9saWN5Q2hlY2tlcjo6Y2hlY2tOYXZpZ2F0aW9uUG9saWMKICAg ICAgICAgIGJsb2JVUkxMaWZldGltZUV4dGVuc2lvbiA9IFdURk1vdmUoYmxvYlVSTExpZmV0aW1l RXh0ZW5zaW9uKSwgcmVxdWVzdElkZW50aWZpZXJdIChQb2xpY3lBY3Rpb24gcG9saWN5QWN0aW9u LCBQb2xpY3lDaGVja0lkZW50aWZpZXIgcmVzcG9uc2VJZGVudGlmaWVyKSBtdXRhYmxlIHsKIAog ICAgICAgICBpZiAoIXJlc3BvbnNlSWRlbnRpZmllci5pc1ZhbGlkRm9yKHJlcXVlc3RJZGVudGlm aWVyKSkKLSAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAgICAgIHJldHVybiBmdW5jdGlvbih7 IH0sIG51bGxwdHIsIE5hdmlnYXRpb25Qb2xpY3lEZWNpc2lvbjo6SWdub3JlTG9hZCk7CiAKICAg ICAgICAgbV9kZWxlZ2F0ZUlzRGVjaWRpbmdOYXZpZ2F0aW9uUG9saWN5ID0gZmFsc2U7CiAKQEAg LTIzNCw3ICsyMzQsNyBAQCB2b2lkIFBvbGljeUNoZWNrZXI6OmNoZWNrTmV3V2luZG93UG9saWN5 CiAgICAgICAgIHJlcXVlc3RJZGVudGlmaWVyXSAoUG9saWN5QWN0aW9uIHBvbGljeUFjdGlvbiwg UG9saWN5Q2hlY2tJZGVudGlmaWVyIHJlc3BvbnNlSWRlbnRpZmllcikgbXV0YWJsZSB7CiAKICAg ICAgICAgaWYgKCFyZXNwb25zZUlkZW50aWZpZXIuaXNWYWxpZEZvcihyZXF1ZXN0SWRlbnRpZmll cikpCi0gICAgICAgICAgICByZXR1cm47CisgICAgICAgICAgICByZXR1cm4gZnVuY3Rpb24oeyB9 LCBudWxscHRyLCB7IH0sIHsgfSwgU2hvdWxkQ29udGludWU6Ok5vKTsKIAogICAgICAgICBzd2l0 Y2ggKHBvbGljeUFjdGlvbikgewogICAgICAgICBjYXNlIFBvbGljeUFjdGlvbjo6RG93bmxvYWQ6 Cg==