Switch chrome.csi() to use times from WebPerformance. (issue 2456293003 by bmcquade@chromium.org)

[bmcq...@chromium.org]

Reviewers: Pat Meenan
CL: https://codereview.chromium.org/2456293003/

Message:
PTAL, thanks!

Description:
Switch chrome.csi() to use times from WebPerformance.

This is a follow on change to https://codereview.chromium.org/2084583002
which converted chrome.loadTimes() to use WebPerformance-based timings.

The current chrome.csi() implementation uses the unsupported
DocumentState-based metrics. startE, which is derived from
DocumentState.request_time, has sometimes been reporting invalid
values for the last 12 months due to crbug.com/615781. This change
moves us to using data from a trusted/maintained/accurate data source.

BUG=621512

Affected files (+6, -11 lines):
M chrome/renderer/loadtimes_extension_bindings.cc


Index: chrome/renderer/loadtimes_extension_bindings.cc
diff --git a/chrome/renderer/loadtimes_extension_bindings.cc b/chrome/renderer/loadtimes_extension_bindings.cc
index 12f64be342135e37d437dddb0caa2f1d6b43ec0d..5efe5116506557dc48bc26a4e904465928b99d9e 100644
--- a/chrome/renderer/loadtimes_extension_bindings.cc
+++ b/chrome/renderer/loadtimes_extension_bindings.cc
@@ -346,20 +346,15 @@ class LoadTimesExtensionWrapper : public v8::Extension {
if (!data_source) {
return;
}
- DocumentState* document_state = DocumentState::FromDataSource(data_source);
- if (!document_state) {
- return;
- }
+ WebPerformance web_performance = frame->performance();
base::Time now = base::Time::Now();
- base::Time start = document_state->request_time().is_null()
- ? document_state->start_load_time()
- : document_state->request_time();
- base::Time onload = document_state->finish_document_load_time();
+ base::Time start =
+ base::Time::FromDoubleT(web_performance.navigationStart());
+ base::Time onload = base::Time::FromDoubleT(web_performance.loadEventEnd());
base::TimeDelta page = now - start;
int navigation_type = GetCSITransitionType(data_source->navigationType());
- // Important: |frame|, |data_source| and |document_state| should not be
- // referred to below this line, as JS setters below can invalidate these
- // pointers.
+ // Important: |frame| and |data_source| should not be referred to below this
+ // line, as JS setters below can invalidate these pointers.
v8::Isolate* isolate = args.GetIsolate();
v8::Local<v8::Context> ctx = isolate->GetCurrentContext();
v8::Local<v8::Object> csi = v8::Object::New(isolate);

[pme...@chromium.org]

LGTM

https://codereview.chromium.org/2456293003/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2456293003/20001


https://codereview.chromium.org/2456293003/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Committed patchset #2 (id:20001)

https://codereview.chromium.org/2456293003/

[commit-bot@chromium.org via chromiumcodereview-hr.appspot.com]

Patchset 2 (id:??) landed as
https://crrev.com/a2b3e8c87b3f3372fc50ebd8a5e249995d1402ea
Cr-Commit-Position: refs/heads/master@{#429018}

https://codereview.chromium.org/2456293003/