Bug 184293 - REGRESSION(r229831): Test WebKit2.ProvisionalURLAfterWillSendRequestCallback times out since r229831
Summary: REGRESSION(r229831): Test WebKit2.ProvisionalURLAfterWillSendRequestCallback ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2018-04-04 02:17 PDT by Carlos Garcia Campos
Modified: 2018-04-05 02:02 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.85 KB, patch)
2018-04-04 02:24 PDT, Carlos Garcia Campos
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2018-04-04 02:17:22 PDT
The problem is that after willSendRequest callback changes the request, the load is cancelled while transitioning to committed state. This happens because the load is not waiting for the response policy check, so it continues and when transitioning to committed, FrameLoader::closeURL() invalidates the current policy check that causes a load failure. The new request returned by the API doesn't have any requester, so it's no longer considered a main resource load. In the network process the resource load task doesn't wait for the response policy and continues the load, sending the data to the web process. Once the first data is received the load transitions to commit, but the response policy check is still ongoing. This can only happen when using the C API (I don't know about the Cocoa API), but not with the GLib API because it doesn't allow to create a new request, only to modify the passed in one. With the C API we loss other internal things of the request like the priority, but I guess the most important one is the requester.
Comment 1 Carlos Garcia Campos 2018-04-04 02:24:58 PDT
Created attachment 337153 [details]
Patch
Comment 2 Alex Christensen 2018-04-04 10:09:00 PDT
Comment on attachment 337153 [details]
Patch

We should probably run that API test on Mac, too.
Comment 3 Michael Catanzaro 2018-04-04 10:19:29 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 337153 [details]
> Patch
> 
> We should probably run that API test on Mac, too.

Right now, I believe we have a bunch of cross-platform API tests that are run only with XCode, when most of them should really be run on all ports. I did not realize that the reverse was true as well. It would be good to comprehensively improve this situation.
Comment 4 Carlos Garcia Campos 2018-04-05 01:44:30 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 337153 [details]
> Patch
> 
> We should probably run that API test on Mac, too.

Indeed, I was surprised it was still prefixed WebKit2, then I realized it was because it's not run in Mac. We should rename it and add it to xcode in a follow up, but I prefer if someone with a mac can do it, it's painful process for me.
Comment 5 Carlos Garcia Campos 2018-04-05 02:01:37 PDT
Committed r230298: <https://trac.webkit.org/changeset/230298>
Comment 6 Radar WebKit Bug Importer 2018-04-05 02:02:54 PDT
<rdar://problem/39202609>