Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(16)

Issue 379873002: Fix Performance::now() for browser navigations. (Closed)

Created:
6 years, 5 months ago by ppi
Modified:
6 years, 5 months ago
Reviewers:
clamy, Nate Chapin, tonyg
CC:
blink-reviews, Nate Chapin, gavinp+loader_chromium.org, tonyg
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Fix Performance::now() for browser navigations. Performance::now() is defined[1] as number of milliseconds since navigationStart. It works correctly for renderer-initiated navigations, but for browser-initiated ones we override the navigationStart with the recording taken on the browser side and this is not represented in Performace::now(). Among other reasons to be sad about this, this bug makes Chromium's pagecyclers not represent the renderer initialization times in their plt measurements. This patch fixes this by adjusting the fields that are supposed to correspond to navigationStart when we change navigationStart from the embedder. [1] http://www.w3.org/TR/hr-time/#dom-performance-now BUG=376004 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178558

Patch Set 1 #

Patch Set 2 : Make the comparison in the assert "<=". #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -0 lines) Patch
M Source/core/loader/DocumentLoadTiming.cpp View 1 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
ppi
Camille, Nate - ptal. Tony - fyi.
6 years, 5 months ago (2014-07-09 14:58:14 UTC) #1
clamy
Lgtm, thanks!
6 years, 5 months ago (2014-07-09 15:05:19 UTC) #2
tonyg
lgtm You might want to put a note to the perf sheriffs in the CL ...
6 years, 5 months ago (2014-07-09 15:08:02 UTC) #3
ppi
Yup, sent a heads-up. I think we would send another with broader scope if the ...
6 years, 5 months ago (2014-07-21 12:06:44 UTC) #4
ppi
The CQ bit was checked by ppi@chromium.org
6 years, 5 months ago (2014-07-21 12:06:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ppi@chromium.org/379873002/20001
6 years, 5 months ago (2014-07-21 12:07:45 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_compile_dbg on tryserver.blink ...
6 years, 5 months ago (2014-07-21 13:16:44 UTC) #7
commit-bot: I haz the power
Change committed as 178558
6 years, 5 months ago (2014-07-21 13:29:16 UTC) #8
Peter Kasting
6 years, 5 months ago (2014-07-21 23:22:47 UTC) #9
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/409723002/ by pkasting@chromium.org.

The reason for reverting is: Broke
org.chromium.android_webview.test.AwContentsClientShouldOverrideUrlLoadingTest#testDoubleNavigateDoesNotSuppressInitialNavigate
on the Android test bots.  See http://crbug.com/395817 ..

Powered by Google App Engine
This is Rietveld 408576698