Bug 163436

Summary: Document request not updated after willSendRequest is called for a redirect
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, bugzilla, cdumez, commit-queue, darin, dbates, japhet, mcatanzaro
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=146306
Bug Depends on:    
Bug Blocks: 163389    
Attachments:
Description Flags
Patch mcatanzaro: review+

Description Carlos Garcia Campos 2016-10-14 01:55:08 PDT
Adding unit tests for bug #146306 in the GTK+ port I've noticed that patch for bug #146306 still doesn't work when a request is modified by willSendRequest after a redirect. The thing is that DocumentLoader::setRequest is not called after willSendRequest is called for a redirect. The first willSendRequest happens before DocumentLoader::startLoadingMainResource, that calls setRequest, but the second one happens after DocumentLoader::redirectReceived() and then the request is never updated again.
Comment 1 Carlos Garcia Campos 2016-10-14 02:02:32 PDT
Created attachment 291595 [details]
Patch

Added unit tests for GTK+ only, because I'm not sure it's even possible to test a server redirect with the cross-platform C API unit tests.
Comment 2 Michael Catanzaro 2016-10-14 11:15:22 PDT
(In reply to comment #1)
> Created attachment 291595 [details]
> Patch
> 
> Added unit tests for GTK+ only, because I'm not sure it's even possible to
> test a server redirect with the cross-platform C API unit tests.

I think it's not currently possible, because we have no cross-platform equivalent to SoupServer.
Comment 3 Michael Catanzaro 2016-10-14 11:31:04 PDT
Comment on attachment 291595 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=291595&action=review

Great tests.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestLoaderClient.cpp:225
> +    enum State { Provisional, ProvisionalAfterRedirect, Commited, Finished };

Personally, I'd still favor using enum class even in a file-private unit test class, but your call.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestLoaderClient.cpp:254
> +    void checkURIAtState(State state, const char* uri)

Please rename the second parameter (maybe to "path") because it's not a URI.
Comment 4 Carlos Garcia Campos 2016-10-16 01:38:08 PDT
Committed r207388: <http://trac.webkit.org/changeset/207388>