Navigation Timing Level 2

third_party/WebKit/Source/core/timing/PerformanceBase.cpp

Index: third_party/WebKit/Source/core/timing/PerformanceBase.cpp
diff --git a/third_party/WebKit/Source/core/timing/PerformanceBase.cpp b/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
index 88094fa06e3bbc734cc74a5cfebffa8c64b236f7..8622ff236c150b09538c73efd1afe4fc3679940c 100644
--- a/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
+++ b/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
@@ -32,12 +32,19 @@
 #include "core/timing/PerformanceBase.h"
 
 #include "core/dom/Document.h"
+#include "core/dom/DocumentTiming.h"
+#include "core/dom/Element.h"

[Yoav Weiss, 2016/11/10 14:58:58]

I believe DocumentTiming.h and Element.h are not needed here

[sunjian, 2016/11/11 21:48:05]

I think i can be onboard with getting rid of "core/dom/Element.h" cause it
doesn't seem to be used at all. But "core/dom/DocumentTiming.h" is actually
being used. I tried getting rid of "core/dom/DocumentTiming.h" and it does
compile successfully cause "core/dom/Document.h" included
"core/dom/DocumentTiming.h" already. But that is implicit right?

 #include "core/events/Event.h"
+#include "core/frame/LocalFrame.h"
 #include "core/frame/UseCounter.h"
+#include "core/loader/DocumentLoadTiming.h"
+#include "core/loader/DocumentLoader.h"
 #include "core/timing/PerformanceLongTaskTiming.h"
+#include "core/timing/PerformanceNavigationTiming.h"
 #include "core/timing/PerformanceObserver.h"
 #include "core/timing/PerformanceResourceTiming.h"
 #include "core/timing/PerformanceUserTiming.h"
+#include "platform/RuntimeEnabledFeatures.h"
 #include "platform/network/ResourceTimingInfo.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "wtf/CurrentTime.h"
@@ -53,6 +60,7 @@ static const size_t defaultFrameTimingBufferSize = 150;
 PerformanceBase::PerformanceBase(double timeOrigin)
     : m_frameTimingBufferSize(defaultFrameTimingBufferSize),
       m_resourceTimingBufferSize(defaultResourceTimingBufferSize),
+      m_navigationTiming(nullptr),

[Yoav Weiss, 2016/11/10 14:58:58]

Is the nullptr initialization needed? (both here and in line below)
I think Member automatically initializes to nullptr when no other pointer is
given

[sunjian, 2016/11/11 21:48:05]

I will get rid of m_navigationTiming's redundant initialization only this since
the line below it is not related to this cl.

       m_userTiming(nullptr),

[kinuko, 2016/11/10 17:55:33]

it's not related to this change but ditto for this (no need to initialize it
with nullptr for Member)

[sunjian, 2016/11/11 21:48:05]

Acknowledged.

       m_timeOrigin(timeOrigin),
       m_observerFilterOptions(PerformanceEntry::Invalid),
@@ -74,6 +82,8 @@ PerformanceEntryVector PerformanceBase::getEntries() const {
   PerformanceEntryVector entries;
 
   entries.appendVector(m_resourceTimingBuffer);
+  if (RuntimeEnabledFeatures::performanceNavigationTiming2Enabled())

[Yoav Weiss, 2016/11/10 14:58:58]

We should append m_navigationTiming only when it's not nullptr, as scripts can
call getEntries before the load event was dispatched and m_navigationTiming
created.
This check here would also make it unnecessary to check the runtime flag, as
when it's false, m_navigationTiming is guaranteed to be null.

[sunjian, 2016/11/11 21:48:05]

Done.

+    entries.append(m_navigationTiming);
   entries.appendVector(m_frameTimingBuffer);
 
   if (m_userTiming) {
@@ -103,6 +113,10 @@ PerformanceEntryVector PerformanceBase::getEntriesByType(
       for (const auto& resource : m_resourceTimingBuffer)
         entries.append(resource);
       break;
+    case PerformanceEntry::Navigation:
+      if (RuntimeEnabledFeatures::performanceNavigationTiming2Enabled())

[Yoav Weiss, 2016/11/10 14:58:58]

same as above. Only add when non-null and no need to check flag.

[sunjian, 2016/11/11 21:48:05]

Done.

+        entries.append(m_navigationTiming);
+      break;
     case PerformanceEntry::Composite:
     case PerformanceEntry::Render:
       for (const auto& frame : m_frameTimingBuffer) {
@@ -143,6 +157,12 @@ PerformanceEntryVector PerformanceBase::getEntriesByName(
     }
   }
 
+  if (entryType.isNull() || type == PerformanceEntry::Navigation) {
+    if (RuntimeEnabledFeatures::performanceNavigationTiming2Enabled() &&
+        m_navigationTiming->name() == name)

[Yoav Weiss, 2016/11/10 14:58:58]

A check for nav timing not being null would prevent a crash here.

[sunjian, 2016/11/11 21:48:05]

Done.

+      entries.append(m_navigationTiming);
+  }
+
   if (entryType.isNull() || type == PerformanceEntry::Composite ||
       type == PerformanceEntry::Render) {
     for (const auto& frame : m_frameTimingBuffer) {
@@ -292,6 +312,44 @@ void PerformanceBase::addResourceTiming(const ResourceTimingInfo& info) {
     addResourceTimingBuffer(*entry);
 }
 
+void PerformanceBase::addNavigationTiming(LocalFrame* frame) {
+  if (!RuntimeEnabledFeatures::performanceNavigationTiming2Enabled())
+    return;
+  DCHECK(frame);

[kinuko, 2016/11/10 17:55:33]

We have null-checks for frame() at the callsite, are you sure this DCHECK always
holds?

[panicker, 2016/11/11 17:14:02]

Yep, if frame is NULL we should not call addNavigationTiming, so add "if
(frame)" at the call-site.

[sunjian, 2016/11/11 21:48:05]

Done.

+  const DocumentLoader* documentLoader = frame->loader().documentLoader();
+  DCHECK(documentLoader);
+  const DocumentLoadTiming documentLoadTiming = documentLoader->timing();
+  const DocumentTiming documentTiming = frame->document()->timing();
+
+  ResourceResponse finalResponse = documentLoader->response();

[kinuko, 2016/11/10 17:55:33]

This creates a copy, looks like just using a const ref should be fine?

[sunjian, 2016/11/11 21:48:05]

Done.

+
+  ResourceLoadTiming* resourceLoadTiming = finalResponse.resourceLoadTiming();
+  double lastRedirectEndTime = documentLoadTiming.redirectEnd();
+  double finishTime = documentLoadTiming.loadEventEnd();
+
+  // TODO(sunjian) Implement transfer size. crbug/663187
+  unsigned long long transferSize = 0;
+  unsigned long long encodedBodyLength = finalResponse.encodedBodyLength();
+  unsigned long long decodedBodyLength = finalResponse.decodedBodyLength();
+  bool didReuseConnection = finalResponse.connectionReused();
+
+  m_navigationTiming = new PerformanceNavigationTiming(
+      timeOrigin(), documentLoadTiming.unloadEventStart(),
+      documentLoadTiming.unloadEventEnd(), documentLoadTiming.loadEventStart(),
+      documentLoadTiming.loadEventEnd(), documentLoadTiming.redirectCount(),
+      documentTiming.domInteractive(),
+      documentTiming.domContentLoadedEventStart(),
+      documentTiming.domContentLoadedEventEnd(), documentTiming.domComplete(),
+      documentLoader->getNavigationType(), documentLoadTiming.redirectStart(),
+      documentLoadTiming.redirectEnd(), documentLoadTiming.fetchStart(),
+      documentLoadTiming.responseEnd(),
+      documentLoadTiming.hasCrossOriginRedirect(),
+      documentLoadTiming.hasSameOriginAsPreviousDocument(), resourceLoadTiming,
+      lastRedirectEndTime, finishTime, transferSize, encodedBodyLength,
+      decodedBodyLength, didReuseConnection);
+  notifyObserversOfEntry(*m_navigationTiming);
+}
+
 void PerformanceBase::addResourceTimingBuffer(PerformanceEntry& entry) {
   m_resourceTimingBuffer.append(&entry);
 
@@ -464,6 +522,7 @@ DOMHighResTimeStamp PerformanceBase::now() const {
 DEFINE_TRACE(PerformanceBase) {
   visitor->trace(m_frameTimingBuffer);
   visitor->trace(m_resourceTimingBuffer);
+  visitor->trace(m_navigationTiming);
   visitor->trace(m_userTiming);
   visitor->trace(m_observers);
   visitor->trace(m_activeObservers);