Bug 179641 (r225702) updated SubresourceLoader::willSendRequestInternal with a return call without first calling the completionHandler. Address this by adding the call to the completionHandler.
<rdar://problem/59436915>
Created attachment 390686 [details] Patch
Comment on attachment 390686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390686&action=review > Source/WebCore/loader/SubresourceLoader.cpp:242 > + return completionHandler(WTFMove(newRequest)); If we reached terminal state, shouldn't we return completionHandler({ }); to not go through with the redirection?
Comment on attachment 390686 [details] Patch Clearing flags on attachment: 390686 Committed r256569: <https://trac.webkit.org/changeset/256569>
All reviewed patches have been landed. Closing bug.
Comment on attachment 390686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390686&action=review >> Source/WebCore/loader/SubresourceLoader.cpp:242 >> + return completionHandler(WTFMove(newRequest)); > > If we reached terminal state, shouldn't we return completionHandler({ }); to not go through with the redirection? What Keith did seems consistent with other early return cases for error cases in this method.
Comment on attachment 390686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390686&action=review > Source/WebCore/ChangeLog:15 > + No new tests -- it's not possible to control execution into the > + affected code path. Seems surprising. Why not? A limitation in our test machinery?
Comment on attachment 390686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390686&action=review >> Source/WebCore/ChangeLog:15 >> + affected code path. > > Seems surprising. Why not? A limitation in our test machinery? If I understand the code correctly, you'd have to cancel the load just at the right time. Then since the load would be cancelled, I don't think there would be anything to observe from the web point-of-view. Since we fail to call the completion handler, the load simply will be hung on network process side I believe (likely until some timeout is reached), which again, would not be easily observable I believe.