Bug 42201 - Use ResourceHandle object for synchronous loading
Summary: Use ResourceHandle object for synchronous loading
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on: 42216
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-13 14:30 PDT by Alexey Proskuryakov
Modified: 2010-07-14 17:20 PDT (History)
8 users (show)

See Also:


Attachments
Part 1: Mac (56.57 KB, patch)
2010-07-13 14:45 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
Now with CFNetwork (81.86 KB, patch)
2010-07-13 16:38 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
updated patch (84.85 KB, patch)
2010-07-13 18:42 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-07-13 14:30:48 PDT
Improve code sharing, pave the way to eventually making callbacks into frame loader client.
Comment 1 Alexey Proskuryakov 2010-07-13 14:45:25 PDT
Created attachment 61418 [details]
Part 1: Mac
Comment 2 Early Warning System Bot 2010-07-13 14:52:28 PDT
Attachment 61418 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3531081
Comment 3 WebKit Review Bot 2010-07-13 15:03:21 PDT
Attachment 61418 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3508225
Comment 4 Alexey Proskuryakov 2010-07-13 15:23:05 PDT
I have these fixed locally.
Comment 5 Alexey Proskuryakov 2010-07-13 16:38:03 PDT
Created attachment 61431 [details]
Now with CFNetwork
Comment 6 WebKit Review Bot 2010-07-13 16:44:32 PDT
Attachment 61418 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3446249
Comment 7 WebKit Review Bot 2010-07-13 17:07:23 PDT
Attachment 61431 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3455252
Comment 8 WebKit Review Bot 2010-07-13 17:23:47 PDT
Attachment 61431 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3513240
Comment 9 Brady Eidson 2010-07-13 17:36:35 PDT
I'm still reviewing, but I don't like the name "firstRequest".  "initialRequest" sounds much better to me.
Comment 10 Alexey Proskuryakov 2010-07-13 18:42:56 PDT
Created attachment 61452 [details]
updated patch

More build fixes, plus a fix for an issue found by Windows tests - do set ResourceHandleInternal::m_connection.
Comment 11 Alexey Proskuryakov 2010-07-13 18:47:29 PDT
> "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.
Comment 12 WebKit Review Bot 2010-07-13 19:41:52 PDT
Attachment 61452 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3438264
Comment 13 Darin Adler 2010-07-14 10:44:24 PDT
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.
Comment 14 Alexey Proskuryakov 2010-07-14 11:10:38 PDT
> 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>.
Comment 15 Alexey Proskuryakov 2010-07-14 11:57:15 PDT
More build fixes in r63340.
Comment 17 Alexey Proskuryakov 2010-07-14 17:17:53 PDT
Tiger behavior fix in r63376.
Comment 18 Alexey Proskuryakov 2010-07-14 17:20:44 PDT
And r63380.