Bug 65817

Summary: move FrameLoadRequest to loader/
Product: WebKit Reporter: jochen
Component: New BugsAssignee: jochen
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, fishd, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

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.