PlzNavigate: remove extraneous DidStartLoading IPC

PlzNavigate: remove extraneous DidStartLoading IPC

This CL removes an extraneous DidStartLoading IPC sent from
RenderFrameImpl::BeginNavigation. This was needed pre-DidStart/StopLoading
refactoring, but is now causing issues with 204 renderer-initiated navigations.

BUG=504347
Committed: https://crrev.com/11e115163a19088ebed550c0ba20fe7b17828ba4
Cr-Commit-Position: refs/heads/master@{#337617}

[clamy, 2015-07-07 14:09:27]

clamy@chromium.org changed reviewers:
+ carlosk@chromium.org, fdegans@chromium.org, nasko@chromium.org

[clamy, 2015-07-07 14:09:46]

@nasko: PTAL
@carlosk, fdegans: FYI

[nasko, 2015-07-07 14:56:32]

LGTM

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

https://codereview.chromium.org/1221083007/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:4759: // TODO(clamy): Data urls should not
be sent back to the browser either.
Sidenote: Isn't this TODO taken care of by Fabrice's CL
(https://codereview.chromium.org/1222203004/)?

[clamy, 2015-07-07 15:13:51]

Thanks! I also updated the patchset since I uncovered an issue with cross-site
navigations in iframes in site-per-process mode (we need to check that the
speculative RFH is loading as well).

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

https://codereview.chromium.org/1221083007/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:4759: // TODO(clamy): Data urls should not
be sent back to the browser either.
On 2015/07/07 14:56:32, nasko (paris) wrote:
> Sidenote: Isn't this TODO taken care of by Fabrice's CL
> (https://codereview.chromium.org/1222203004/)

Indeed. Removed the TODO.

[Fabrice (no longer in Chrome), 2015-07-07 15:16:35]

Thanks!
lgtm with a nit

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
File content/browser/frame_host/frame_tree_node.cc (right):

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
content/browser/frame_host/frame_tree_node.cc:239: // Check if a navigation is
about ot commit in the speculative
nit: s/ot/to/

[nasko, 2015-07-07 15:18:57]

LGTM with a nit.

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
File content/browser/frame_host/frame_tree_node.cc (right):

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
content/browser/frame_host/frame_tree_node.cc:239: // Check if a navigation is
about ot commit in the speculative
nit: Does it really matter whether it is about to commit or not? Technically, if
the speculative RFH says it is loading, we should take it into account, right?

[clamy, 2015-07-07 15:23:58]

Thanks!

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
File content/browser/frame_host/frame_tree_node.cc (right):

https://codereview.chromium.org/1221083007/diff/20001/content/browser/frame_h...
content/browser/frame_host/frame_tree_node.cc:239: // Check if a navigation is
about ot commit in the speculative
On 2015/07/07 15:18:57, nasko (paris) wrote:
> nit: Does it really matter whether it is about to commit or not? Technically,
if
> the speculative RFH says it is loading, we should take it into account, right?

Well it should only tell us that it is loading when it is about to commit. Since
it is not adding that much, I just removed the comment.

[clamy, 2015-07-07 15:39:28]

The CQ bit was checked by clamy@chromium.org

[clamy, 2015-07-07 15:39:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
fdegans@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1221083007/#ps40001
(title: "Removed comment")

[commit-bot: I haz the power, 2015-07-07 15:40:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1221083007/40001

[carlosk, 2015-07-07 15:54:30]

LGTM.

[commit-bot: I haz the power, 2015-07-07 16:42:25]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-07 16:43:34]

Patchset 3 (id:??) landed as
https://crrev.com/11e115163a19088ebed550c0ba20fe7b17828ba4
Cr-Commit-Position: refs/heads/master@{#337617}