Moving DidFinishLoad from RenderView to RenderFrame.

Moving DidFinishLoad from RenderView to RenderFrame.

BUG=304341
R=creis@chromium.org

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

[nasko, 2014-03-05 18:36:09]

Hey Charlie,
Can you review this CL for me? It moves the DidFinishLoad IPC from RenderView to
RenderFrame.
Thanks in advance!
Nasko

[Charlie Reis, 2014-03-05 18:50:25]

Great.  LGTM with one question below.

https://codereview.chromium.org/185993008/diff/40001/content/browser/web_cont...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/185993008/diff/40001/content/browser/web_cont...
content/browser/web_contents/web_contents_impl.cc:2269: rfh->render_view_host()
!= GetRenderViewHost()) {
Can we use rfh != GetMainFrame() here?

(Side note: I'm fine with adding braces here, but style doesn't require it for
multi-line conditionals as long as there's a one-line body and it's still
"readable.")

https://codereview.chromium.org/185993008/diff/40001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/185993008/diff/40001/content/renderer/render_...
content/renderer/render_frame_impl.cc:1577: ds->request().url(),
nit: 1 more space indent.

[nasko, 2014-03-05 19:00:46]

https://codereview.chromium.org/185993008/diff/40001/content/browser/web_cont...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/185993008/diff/40001/content/browser/web_cont...
content/browser/web_contents/web_contents_impl.cc:2269: rfh->render_view_host()
!= GetRenderViewHost()) {
On 2014/03/05 18:50:26, Charlie Reis wrote:
> Can we use rfh != GetMainFrame() here?
> 
> (Side note: I'm fine with adding braces here, but style doesn't require it for
> multi-line conditionals as long as there's a one-line body and it's still
> "readable.")

Yes, we can. Fixed.
On the style: since we have 3 levels of indentation on 3 lines of code, it makes
sense to me to be explicit.

https://codereview.chromium.org/185993008/diff/40001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/185993008/diff/40001/content/renderer/render_...
content/renderer/render_frame_impl.cc:1577: ds->request().url(),
On 2014/03/05 18:50:26, Charlie Reis wrote:
> nit: 1 more space indent.

Done.

[nasko, 2014-03-05 19:01:04]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2014-03-05 19:01:25]

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

[commit-bot: I haz the power, 2014-03-05 20:30:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-05 20:30:55]

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

[nasko, 2014-03-05 21:03:23]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2014-03-05 21:05:18]

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

[nasko, 2014-03-05 22:20:55]

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