PageLoadMetrics renderer and browser implementation.

components/page_load_metrics/common/page_load_timing.cc

Index: components/page_load_metrics/common/page_load_timing.cc
diff --git a/components/page_load_metrics/common/page_load_timing.cc b/components/page_load_metrics/common/page_load_timing.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f7b844f4fe012721afffb3dee1fed3a403211e7b
--- /dev/null
+++ b/components/page_load_metrics/common/page_load_timing.cc
@@ -0,0 +1,63 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/page_load_metrics/common/page_load_timing.h"
+#include "third_party/WebKit/public/web/WebPerformance.h"
+
+namespace page_load_metrics {
+
+PageLoadTiming::PageLoadTiming() {}
+PageLoadTiming::PageLoadTiming(blink::WebPerformance& p) {

[Bryan McQuade, 2015/09/02 18:26:46]

in this case I think a single-character variable name is probably reasonably
readable, but I rarely see single-char variables other than 'i' for increment
variables. Can we call this something slightly longer, maybe 'perf'? I know that
doesn't make much of a difference - if for no other reason than for convention
that variable names are generally not just one char.

[Charlie Harrison, 2015/09/03 14:00:52]

Done.

+  double start = p.navigationStart();
+  navigation_start = base::Time::FromDoubleT(start);
+  response_start = ClampDelta(p.responseStart(), start);
+  dom_content_loaded_event_start =
+      ClampDelta(p.domContentLoadedEventStart(), start);
+  load_event_start = ClampDelta(p.loadEventStart(), start);
+  first_layout = ClampDelta(p.firstLayout(), start);
+}
+
+base::TimeDelta PageLoadTiming::ClampDelta(double event_ms,

[Bryan McQuade, 2015/09/02 18:26:46]

clamp delta doesn't need to be a member function - it can be a non-member
function in an anonymous namespace of the cc file. if you can avoid putting a
method in the header, i generally think it's better to make it just an
implementation detail where possible.

+                                           double start_ms) const {
+  double delta = event_ms - start_ms;
+  return base::TimeDelta::FromMillisecondsD(delta < 0 ? 0 : delta);
+}
+
+PageLoadTiming::~PageLoadTiming() {}
+
+bool PageLoadTiming::operator==(const PageLoadTiming& other) const {
+  return navigation_start == other.navigation_start &&
+         response_start == other.response_start &&
+         dom_content_loaded_event_start ==
+             other.dom_content_loaded_event_start &&
+         load_event_start == other.load_event_start &&
+         first_layout == other.first_layout;
+}
+
+bool PageLoadTiming::IsChild(const PageLoadTiming& parent) const {

[Bryan McQuade, 2015/09/02 18:26:46]

I do think these policy methods are slightly implementation details of the web
contents observer, and might belong there instead. especially given that this is
a struct and shouldn't have a lot of functionality associated with it (see
related comment in header).

+  return (parent.navigation_start.is_null() ||
+          navigation_start == parent.navigation_start);
+}
+
+bool PageLoadTiming::IsComplete() const {
+  return !navigation_start.is_null() && !response_start.is_zero() &&
+         !dom_content_loaded_event_start.is_zero() &&
+         !load_event_start.is_zero() && !first_layout.is_zero();
+}
+
+bool PageLoadTiming::IsEmpty() const {
+  return navigation_start.is_null() && response_start.is_zero() &&
+         dom_content_loaded_event_start.is_zero() &&
+         load_event_start.is_zero() && first_layout.is_zero();
+}
+
+// Order: navigation_start => response_start =>
+// dom_content_loaded_event_start => load_event_start
+bool PageLoadTiming::IsOrdered() const {
+  return !response_start.is_zero() &&
+         response_start < dom_content_loaded_event_start &&
+         dom_content_loaded_event_start < load_event_start;
+}
+
+}  // namespace page_load_metrics