Bug 65817 – move FrameLoadRequest to loader/

• Component: New Bugs
• Status: RESOLVED FIXED

move FrameLoadRequest to loader/

Created attachment 103159 [details]
Patch

[Darin Adler]

Comment on attachment 103159 [details]
Patch

Patch seems to lack the actual header file move.

meh, seems like webkit-patch doesn't like git mv :(

Created attachment 103160 [details]
Patch

Comment on attachment 103160 [details]
Patch

still doesn't work :-/

Created attachment 103161 [details]
Patch

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

[Adam Barth]

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.  :)

Created attachment 103164 [details]
Patch

[Alexey Proskuryakov]

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.

[WebKit Review Bot]

Comment on attachment 103164 [details]
Patch

Clearing flags on attachment: 103164

Committed r92572: <http://trac.webkit.org/changeset/92572>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.