Bug 42201 – Use ResourceHandle object for synchronous loading

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Alexey Proskuryakov]

Improve code sharing, pave the way to eventually making callbacks into frame loader client.

[Alexey Proskuryakov]

Created attachment 61418 [details]
Part 1: Mac

[Early Warning System Bot]

Attachment 61418 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3531081

[WebKit Review Bot]

Attachment 61418 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3508225

[Alexey Proskuryakov]

I have these fixed locally.

[Alexey Proskuryakov]

Created attachment 61431 [details]
Now with CFNetwork

[WebKit Review Bot]

Attachment 61418 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3446249

[WebKit Review Bot]

Attachment 61431 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3455252

[WebKit Review Bot]

Attachment 61431 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3513240

[Brady Eidson]

I'm still reviewing, but I don't like the name "firstRequest".  "initialRequest" sounds much better to me.

[Alexey Proskuryakov]

Created attachment 61452 [details]
updated patch

More build fixes, plus a fix for an issue found by Windows tests - do set ResourceHandleInternal::m_connection.

[Alexey Proskuryakov]

> "initialRequest" sounds much better to me.

As discussed on IRC, that's less precise. ResourceHandle changes the initial request in many ways before sending if (removing credentials, setting various properties). So, it's the first request that's being actually sent.

[WebKit Review Bot]

Attachment 61452 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3438264

[Darin Adler]

Comment on attachment 61452 [details]
updated patch

In some other code we use originalRequest rather than firstRequest. Is this the same concept?

> +    void setAllowStoredCredentials(bool allow) { m_allowStoredCredentials = allow; }
> +    bool isDone() { return m_isDone; }
> +
> +    CFMutableDataRef data() { return m_data.get(); }
> +
> +    virtual void willSendRequest(ResourceHandle*, ResourceRequest&, const ResourceResponse& /*redirectResponse*/);
> +    virtual bool shouldUseCredentialStorage(ResourceHandle*);
> +    virtual void didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge&);
> +    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&);
> +    virtual void didReceiveData(ResourceHandle*, const char*, int, int /*lengthReceived*/);
> +    virtual void didFinishLoading(ResourceHandle*);
> +    virtual void didFail(ResourceHandle*, const ResourceError&);
> +#if USE(PROTECTION_SPACE_AUTH_CALLBACK)
> +    virtual bool canAuthenticateAgainstProtectionSpace(ResourceHandle*, const ProtectionSpace&);
> +#endif

Can any of these be private?

Looks like a compile failure on GTK.

[Alexey Proskuryakov]

> In some other code we use originalRequest rather than firstRequest. Is this the same concept?

I think that originalRequest is used for the request that hasn't been modified by network loader level yet (and we also have originalRequestCopy for the one that hasn't been modified by someone else, maybe a client). It's not quite the same.

> Can any of these be private?
> Looks like a compile failure on GTK.

Addressed those.

Committed <http://trac.webkit.org/changeset/63332>.

[Alexey Proskuryakov]

More build fixes in r63340.

[WebKit Review Bot]

http://trac.webkit.org/changeset/63340 might have broken Chromium Mac Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/63339
http://trac.webkit.org/changeset/63340
http://trac.webkit.org/changeset/63341
http://trac.webkit.org/changeset/63342

[Alexey Proskuryakov]

Tiger behavior fix in r63376.

[Alexey Proskuryakov]

And r63380.