Bug 65817 - move FrameLoadRequest to loader/
Summary: move FrameLoadRequest to loader/
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: jochen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-06 13:08 PDT by jochen
Modified: 2011-08-07 04:29 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.66 KB, patch)
2011-08-06 13:09 PDT, jochen
no flags Details | Formatted Diff | Diff
Patch (4.87 KB, patch)
2011-08-06 13:16 PDT, jochen
no flags Details | Formatted Diff | Diff
Patch (10.77 KB, patch)
2011-08-06 13:26 PDT, jochen
no flags Details | Formatted Diff | Diff
Patch (10.66 KB, patch)
2011-08-06 14:12 PDT, jochen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jochen 2011-08-06 13:08:56 PDT
move FrameLoadRequest to loader/
Comment 1 jochen 2011-08-06 13:09:25 PDT
Created attachment 103159 [details]
Patch
Comment 2 Darin Adler 2011-08-06 13:12:38 PDT
Comment on attachment 103159 [details]
Patch

Patch seems to lack the actual header file move.
Comment 3 jochen 2011-08-06 13:14:46 PDT
meh, seems like webkit-patch doesn't like git mv :(
Comment 4 jochen 2011-08-06 13:16:50 PDT
Created attachment 103160 [details]
Patch
Comment 5 jochen 2011-08-06 13:18:07 PDT
Comment on attachment 103160 [details]
Patch

still doesn't work :-/
Comment 6 jochen 2011-08-06 13:26:49 PDT
Created attachment 103161 [details]
Patch
Comment 7 jochen 2011-08-06 13:28:58 PDT
Alexey, could you take a look please?

This patch moves FrameLoadRequest to the right location. I intend to add a LoadRequest that FrameLoadRequest will inherit from.

LoadRequest will be a wrapper around ResourceRequst that knows about TargetType
Comment 8 Adam Barth 2011-08-06 13:30:58 PDT
Comment on attachment 103161 [details]
Patch

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

> Source/WebCore/loader/FrameLoadRequest.h:35
> +    struct FrameLoadRequest {
> +    public:

Code inside an namespace shouldn't be indented.  If you're going to touch this file anyway, you might as well fix the style.  :)
Comment 9 jochen 2011-08-06 14:12:09 PDT
Created attachment 103164 [details]
Patch
Comment 10 Alexey Proskuryakov 2011-08-06 17:50:23 PDT
Comment on attachment 103164 [details]
Patch

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

I'm not the best person to judge what goes into page, and what goes into loader directory. Since Darin and Adam had a look at the bug, I feel confident enough to say r=me now.

> LoadRequest will be a wrapper around ResourceRequst that knows about TargetType

It really helps that you explain your next steps as you go. The plan looks good to me so far, but I expect some discussion about division of responsibility between LoadRequest and ResourceLoader to take place soon.

> Source/WebCore/loader/FrameLoadRequest.h:36
> +    explicit FrameLoadRequest(PassRefPtr<SecurityOrigin> requester)

I'm not sure if we reached consensus in a webkit-dev discussion a few months ago, but I think that this is a misuse of PassRefPtr, and a plain pointer should be used.

Of course, this should not be changed in this patch, which only moves a file.
Comment 11 WebKit Review Bot 2011-08-07 04:29:29 PDT
Comment on attachment 103164 [details]
Patch

Clearing flags on attachment: 103164

Committed r92572: <http://trac.webkit.org/changeset/92572>
Comment 12 WebKit Review Bot 2011-08-07 04:29:34 PDT
All reviewed patches have been landed.  Closing bug.