First version of PerformanceObserver

Source/core/timing/PerformanceBase.cpp

Index: Source/core/timing/PerformanceBase.cpp
diff --git a/Source/core/timing/PerformanceBase.cpp b/Source/core/timing/PerformanceBase.cpp
index 219f3f5b51331991db0f63fb8f2a52bb9ffdd724..901d3d56ce9cfa61b8d78ec2c554b236fc64e01e 100644
--- a/Source/core/timing/PerformanceBase.cpp
+++ b/Source/core/timing/PerformanceBase.cpp
@@ -35,6 +35,8 @@
 #include "core/dom/Document.h"
 #include "core/events/Event.h"
 #include "core/timing/PerformanceCompositeTiming.h"
+#include "core/timing/PerformanceObserver.h"
+#include "core/timing/PerformanceObserverRegistration.h"
 #include "core/timing/PerformanceRenderTiming.h"
 #include "core/timing/PerformanceResourceTiming.h"
 #include "core/timing/PerformanceUserTiming.h"
@@ -47,6 +49,16 @@ namespace blink {
 static const size_t defaultResourceTimingBufferSize = 150;
 static const size_t defaultFrameTimingBufferSize = 150;
 
+// The order here has to match the order in PerformanceBase.h enum FilterType
+static const char* kFilterTypes[] = {
+    "_dummy_",

[esprehn, 2015/07/18 22:24:15]

What is _dummy_/ we don't generally do this. maybe you want an Unknown or
Invalid entry at the start of the enum and the same here?

[MikeB, 2015/07/20 23:06:50]

Done.

+    "composite",
+    "mark",
+    "measure",
+    "render",
+    "resource"
+};
+
 PerformanceBase::PerformanceBase(double timeOrigin)
     : m_frameTimingBufferSize(defaultFrameTimingBufferSize)
     , m_resourceTimingBufferSize(defaultResourceTimingBufferSize)
@@ -211,7 +223,7 @@ static bool allowsTimingRedirect(const Vector<ResourceResponse>& redirectChain,
 
 void PerformanceBase::addResourceTiming(const ResourceTimingInfo& info)
 {
-    if (isResourceTimingBufferFull())
+    if (isResourceTimingBufferFull() && !hasObserverFor(Resource))
         return;
     SecurityOrigin* securityOrigin = nullptr;
     if (ExecutionContext* context = executionContext())
@@ -244,7 +256,9 @@ void PerformanceBase::addResourceTiming(const ResourceTimingInfo& info)
     double lastRedirectEndTime = lastRedirectTiming->receiveHeadersEnd();
 
     PerformanceEntry* entry = PerformanceResourceTiming::create(info, timeOrigin(), startTime, lastRedirectEndTime, allowTimingDetails, allowRedirectDetails);
-    addResourceTimingBuffer(entry);
+    notifyObserversOfEntry(Resource, entry);
+    if (!isResourceTimingBufferFull())
+        addResourceTimingBuffer(entry);
 }
 
 void PerformanceBase::addResourceTimingBuffer(PerformanceEntry* entry)
@@ -262,20 +276,24 @@ bool PerformanceBase::isResourceTimingBufferFull()
 
 void PerformanceBase::addRenderTiming(Document* initiatorDocument, unsigned sourceFrame, double startTime, double finishTime)
 {
-    if (isFrameTimingBufferFull())
+    if (isFrameTimingBufferFull() && !hasObserverFor(Render))
         return;
 
     PerformanceEntry* entry = PerformanceRenderTiming::create(initiatorDocument, sourceFrame, startTime, finishTime);
-    addFrameTimingBuffer(entry);
+    notifyObserversOfEntry(Render, entry);
+    if (!isFrameTimingBufferFull())
+        addFrameTimingBuffer(entry);
 }
 
 void PerformanceBase::addCompositeTiming(Document* initiatorDocument, unsigned sourceFrame, double startTime)
 {
-    if (isFrameTimingBufferFull())
+    if (isFrameTimingBufferFull() && !hasObserverFor(Composite))
         return;
 
     PerformanceEntry* entry = PerformanceCompositeTiming::create(initiatorDocument, sourceFrame, startTime);
-    addFrameTimingBuffer(entry);
+    notifyObserversOfEntry(Composite, entry);
+    if (!isFrameTimingBufferFull())
+        addFrameTimingBuffer(entry);
 }
 
 void PerformanceBase::addFrameTimingBuffer(PerformanceEntry* entry)
@@ -295,7 +313,8 @@ void PerformanceBase::mark(const String& markName, ExceptionState& exceptionStat
 {
     if (!m_userTiming)
         m_userTiming = UserTiming::create(this);
-    m_userTiming->mark(markName, exceptionState);
+    NewPerformanceEntryCallback callback = WTF::bind<PerformanceEntry*>(&PerformanceBase::notifyObserversOfEntry, this, Mark);
+    m_userTiming->mark(markName, callback, exceptionState);
 }
 
 void PerformanceBase::clearMarks(const String& markName)
@@ -309,7 +328,8 @@ void PerformanceBase::measure(const String& measureName, const String& startMark
 {
     if (!m_userTiming)
         m_userTiming = UserTiming::create(this);
-    m_userTiming->measure(measureName, startMark, endMark, exceptionState);
+    NewPerformanceEntryCallback callback = WTF::bind<PerformanceEntry*>(&PerformanceBase::notifyObserversOfEntry, this, Measure);
+    m_userTiming->measure(measureName, startMark, endMark, callback, exceptionState);

[esprehn, 2015/07/18 22:24:16]

Hmm, this seems like you're going to notify multiple times one per type since
you bind with Mark and Measure. I don't think you want to do that, or use this
bind.

What's with the ExceptionState here btw, is script running down there?

[MikeB, 2015/07/20 23:06:50]

That's how it's supposed to work, no? You can create marks without ever
measuring. If you are monitoring the performance timeline for marks, it should
trigger. And vice versa, if you are observing for measures, then you want to get
triggered when a measure happens. 

Yes, the UserTiming::mark and UserTiming::measure will throw exceptions if you
use an invlid mark name or try and measure without actually having the mark
defined. 

I use the bind because I need to trigger when a new PerformanceEntry is created,
and I want to use the enum for entryType rather than string comparisons all the
time. Following your other suggestion though, I think I'll try and make
PerformanceEntry know/own the enum and then I can drop the bind call. 

 }
 
 void PerformanceBase::clearMeasures(const String& measureName)
@@ -319,6 +339,48 @@ void PerformanceBase::clearMeasures(const String& measureName)
     m_userTiming->clearMeasures(measureName);
 }
 
+void PerformanceBase::registerPerformanceObserver(PerformanceObserverRegistration* registration)
+{
+    for (FilterTypeIndex filterIndex = 1; filterIndex < (sizeof(kFilterTypes) / sizeof(char*)); ++filterIndex) {

[esprehn, 2015/07/18 22:24:16]

I think you want WTF_ARRAY_LENGTH here.

1 is a thing, maybe assign Composite

+        if (registration->shouldReceiveEntryFrom(kFilterTypes[filterIndex])) {
+            PerformanceObserverRegistrations::AddResult result = m_observerRegistry.add(filterIndex, PerformanceObserverRegistrationVector());
+            registration->addReference();

[esprehn, 2015/07/18 22:24:15]

What is this addReference() ? That means each thing you listen for adds another
reference?

[MikeB, 2015/07/20 23:06:50]

Got rid of it (and PerformanceObserverRegistration entirely)

+            result.storedValue->value.append(registration);
+        }
+    }
+}
+PerformanceObserver* PerformanceBase::createPerformanceObserver(PerformanceObserverCallback* callback)

[esprehn, 2015/07/18 22:24:16]

What is this? it should be new PerformanceObserver() not
performance.createPerformanceObserver

[MikeB, 2015/07/20 23:06:50]

See issue tracker - switching back to new PerformanceObserver.

+{
+    return PerformanceObserver::create(this, callback);
+}
+
+void PerformanceBase::unregisterPerformanceObserver(PerformanceObserverRegistration* registration)
+{
+    for (auto& registry : m_observerRegistry) {
+        size_t index = registry.value.find(registration);

[esprehn, 2015/07/18 22:24:15]

I think you want to make this simpler, have a single LinkedHashSet of observers
and a combined unsigned of listened for types.

When adding a record you do

if (!(m_listenedTypes & type))
  return;

then adding a new observer you do;

m_observers.add(observer);
m_listenedTypes &= observer->filterTypes();

[MikeB, 2015/07/20 23:06:50]

Done.

+        if (index == kNotFound)
+            continue;
+        registration->dispose();
+        registry.value.remove(index);
+    }
+}
+
+void PerformanceBase::notifyObserversOfEntry(FilterType filterType, PerformanceEntry* entry)
+{
+    PerformanceObserverRegistrations::iterator registry = m_observerRegistry.find(filterType);
+    if (registry == m_observerRegistry.end())
+        return;
+    for (auto& registration : *registry.values()) {
+        registration->observer().enqueuePerformanceEntry(entry);
+    }
+}
+
+bool PerformanceBase::hasObserverFor(FilterType filterType)
+{
+    PerformanceObserverRegistrations::iterator registry = m_observerRegistry.find(filterType);
+    return registry != m_observerRegistry.end() && registry.values()->size();
+}
+
 double PerformanceBase::now() const
 {
     double nowSeconds = monotonicallyIncreasingTime() - m_timeOrigin;
@@ -331,6 +393,7 @@ DEFINE_TRACE(PerformanceBase)
     visitor->trace(m_frameTimingBuffer);
     visitor->trace(m_resourceTimingBuffer);
     visitor->trace(m_userTiming);
+    visitor->trace(m_observerRegistry);
     EventTargetWithInlineData::trace(visitor);
 }