Move DidReceiveServerRedirectForProvisionalLoad from RenderView to RenderFrame

Move DidReceiveServerRedirectForProvisionalLoad from RenderView to RenderFrame

BUG=304341
R=jam@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244265

[nasko, 2014-01-10 05:41:12]

Hey John,
Can you review this CL for me? It moves another navigation IPC over from
RenderView to RenderFrame.
Thanks in advance!
Nasko

[jam, 2014-01-10 16:19:31]

lgtm

https://codereview.chromium.org/129813006/diff/1/content/browser/frame_host/n...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigator_impl.cc:174: entry =
controller_->GetEntryWithPageID(render_frame_host->GetSiteInstance(),
nit: 80 chars

https://codereview.chromium.org/129813006/diff/1/content/browser/web_contents...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2212:
render_frame_host->render_view_host()));
it looks like none of the overrides for this method actually use the
renderviewhost parameter. it might have been used before, or probably passed
just to match the other navigation callbacks where the embedders needs a RVH.
let's just switch this to a RFH now so no one adds more work for us later by
using the RVH?

https://codereview.chromium.org/129813006/diff/1/content/common/frame_messages.h
File content/common/frame_messages.h (right):

https://codereview.chromium.org/129813006/diff/1/content/common/frame_message...
content/common/frame_messages.h:99: GURL /* last url */,
nit: I realize you're just moving, but the convention (although not always
followed) is that the comment is the parameter name in the dispatching method

https://codereview.chromium.org/129813006/diff/1/content/renderer/render_fram...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/renderer/render_fram...
content/renderer/render_frame_impl.cc:922: Send(new
FrameHostMsg_DidRedirectProvisionalLoad(routing_id_,
nit: i realize you just moved it, but can you fix the argument wrapping? this is
against the style guide since the arguments should start in the same column on
each line

[nasko, 2014-01-10 19:54:39]

Fixed all issues pointed out by John and layout tests failures.

https://codereview.chromium.org/129813006/diff/1/content/browser/frame_host/n...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/browser/frame_host/n...
content/browser/frame_host/navigator_impl.cc:174: entry =
controller_->GetEntryWithPageID(render_frame_host->GetSiteInstance(),
On 2014/01/10 16:19:31, jam wrote:
> nit: 80 chars

Done.

https://codereview.chromium.org/129813006/diff/1/content/browser/web_contents...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2212:
render_frame_host->render_view_host()));
On 2014/01/10 16:19:31, jam wrote:
> it looks like none of the overrides for this method actually use the
> renderviewhost parameter. it might have been used before, or probably passed
> just to match the other navigation callbacks where the embedders needs a RVH.
> let's just switch this to a RFH now so no one adds more work for us later by
> using the RVH?

Done.

https://codereview.chromium.org/129813006/diff/1/content/common/frame_messages.h
File content/common/frame_messages.h (right):

https://codereview.chromium.org/129813006/diff/1/content/common/frame_message...
content/common/frame_messages.h:99: GURL /* last url */,
On 2014/01/10 16:19:31, jam wrote:
> nit: I realize you're just moving, but the convention (although not always
> followed) is that the comment is the parameter name in the dispatching method

Thanks for the info. Updated and will keep it in mind in the future. It is code
move, but if I can be fixing style as I go, I'll do it.

https://codereview.chromium.org/129813006/diff/1/content/renderer/render_fram...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/129813006/diff/1/content/renderer/render_fram...
content/renderer/render_frame_impl.cc:922: Send(new
FrameHostMsg_DidRedirectProvisionalLoad(routing_id_,
On 2014/01/10 16:19:31, jam wrote:
> nit: i realize you just moved it, but can you fix the argument wrapping? this
is
> against the style guide since the arguments should start in the same column on
> each line

Done.

[commit-bot: I haz the power, 2014-01-10 19:58:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/129813006/180001

[commit-bot: I haz the power, 2014-01-10 22:14:17]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[nasko, 2014-01-10 22:42:04]

Committed patchset #3 manually as r244265 (presubmit successful).