NavigationTiming: set navigationStart for navigations in new tabs.

NavigationTiming: set navigationStart for navigations in new tabs.

This patch uses the existing browser_navigation_start field in
FrameMsg_Navigate to set the navigation start time for browser
navigations in new tabs.

For browser navigations that result in cross-site swap, the value is
later overriden with the end of the beforeunload event of the current
document.

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

[ppi, 2014-06-16 15:11:05]

This contains the fix as discussed + additional comments here and there
explaining what particular time recordings are for.

I see that we apparently can't use the InterprocessTimeTickConverter for the
values provided for browser_navigation_start in the IPC (see the comment in
RenderFrameImpl::OnNavigate). I wonder how bad of a problem is it - in any case,
we already have this issue for the cross-site navigation case.

Heads-up to the WG is yet to be sent, I will cc you on the mail.

Do you guys know who would be the best to chat about interprocess TimeTicks,
apart form James Simonsen?

[clamy, 2014-06-16 15:28:55]

Thanks. I have a question, please see below.

https://chromiumcodereview.appspot.com/339573003/diff/1/content/browser/frame...
File content/browser/frame_host/navigator_impl.cc (right):

https://chromiumcodereview.appspot.com/339573003/diff/1/content/browser/frame...
content/browser/frame_host/navigator_impl.cc:335: base::TimeTicks
navigation_start = base::TimeTicks::Now();
Is it possible to get a call to NavigatorImpl::NavigateToEntry in a non new tab?
In that case what happens with the timing of the beforeunload event?

[ppi, 2014-06-16 15:40:59]

Thanks, please see the reply inline.

https://chromiumcodereview.appspot.com/339573003/diff/1/content/browser/frame...
File content/browser/frame_host/navigator_impl.cc (right):

https://chromiumcodereview.appspot.com/339573003/diff/1/content/browser/frame...
content/browser/frame_host/navigator_impl.cc:335: base::TimeTicks
navigation_start = base::TimeTicks::Now();
On 2014/06/16 15:28:55, clamy wrote:
> Is it possible to get a call to NavigatorImpl::NavigateToEntry in a non new
tab?
> In that case what happens with the timing of the beforeunload event?

Yes, we go through this codepath for all browser navigations, including ones in
the existing tab.

The logic that sets the beforeunload end time as navigation start still works
normally, overriding the value that we set here (I also put a paragraph about it
in the commit message).

[clamy, 2014-06-16 15:41:57]

Thanks, lgtm

[ppi, 2014-06-27 12:22:52]

Thanks! I think this should be ready now. Adding:

- Tony for moral approval
- Nasko for review

[nasko, 2014-06-27 23:10:55]

LGTM with some nits.

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
content/browser/frame_host/navigator_impl.cc:96: // associated with the
RenderView. TODO(ppi): make it go away.
nit: TODO on new line.

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
content/browser/frame_host/navigator_impl.cc:334: // capture the time needed for
RFM initialization.
RFM? Did you mean RFH? Maybe expand it instead of abbreviation.

https://codereview.chromium.org/339573003/diff/80001/content/common/frame_mes...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/339573003/diff/80001/content/common/frame_mes...
content/common/frame_messages.h:250: // The navigationStart time to expose
through the Navigation Timing to JS.
nit: Navigation Timing API

[ppi, 2014-07-01 18:40:14]

Thanks, Nasko!

Adding John for render_frame_impl.

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
content/browser/frame_host/navigator_impl.cc:96: // associated with the
RenderView. TODO(ppi): make it go away.
On 2014/06/27 23:10:54, nasko wrote:
> nit: TODO on new line.

Done.

https://codereview.chromium.org/339573003/diff/80001/content/browser/frame_ho...
content/browser/frame_host/navigator_impl.cc:334: // capture the time needed for
RFM initialization.
On 2014/06/27 23:10:54, nasko wrote:
> RFM? Did you mean RFH? Maybe expand it instead of abbreviation.

Done.

https://codereview.chromium.org/339573003/diff/80001/content/common/frame_mes...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/339573003/diff/80001/content/common/frame_mes...
content/common/frame_messages.h:250: // The navigationStart time to expose
through the Navigation Timing to JS.
On 2014/06/27 23:10:54, nasko wrote:
> nit: Navigation Timing API

Done.

[jam, 2014-07-01 23:47:36]

lgtm

[tonyg, 2014-07-07 22:17:41]

lgtm

[ppi, 2014-07-10 20:27:01]

The CQ bit was checked by ppi@chromium.org

[commit-bot: I haz the power, 2014-07-10 20:27:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ppi@chromium.org/339573003/200001

[commit-bot: I haz the power, 2014-07-11 02:36:21]

Change committed as 282505