Always call CompletionHandlers after r240909
Created attachment 362404 [details] Patch
Looks like the patch isn't building??
Created attachment 362417 [details] Patch
Comment on attachment 362417 [details] 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.
We definitely need to do something with the CompletionHandlers. If we don't call them we need to store them somewhere.
(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.
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?
No
(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?
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.
(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.
Comment on attachment 362417 [details] Patch Clearing flags on attachment: 362417 Committed r241842: <https://trac.webkit.org/changeset/241842>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48253351>