Refactor InspectorWebPerfAgent: update lifecycle management to be per Local Frame root; replace hea…

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

Index: third_party/WebKit/Source/core/timing/Performance.cpp
diff --git a/third_party/WebKit/Source/core/timing/Performance.cpp b/third_party/WebKit/Source/core/timing/Performance.cpp
index abfaffecadc78123316515dd844a43b4598daa68..608e1c9b2330f0fa74f9548df1d2d18e98863fac 100644
--- a/third_party/WebKit/Source/core/timing/Performance.cpp
+++ b/third_party/WebKit/Source/core/timing/Performance.cpp
@@ -35,7 +35,6 @@
 #include "bindings/core/v8/V8ObjectBuilder.h"
 #include "core/dom/Document.h"
 #include "core/frame/LocalFrame.h"
-#include "core/inspector/InspectedFrames.h"
 #include "core/inspector/InspectorWebPerfAgent.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/origin_trials/OriginTrials.h"
@@ -59,9 +58,17 @@ static double toTimeOrigin(LocalFrame* frame) {
 }
 
 Performance::Performance(LocalFrame* frame)
-    : PerformanceBase(toTimeOrigin(frame)), DOMWindowProperty(frame) {}
-
-Performance::~Performance() {}
+    : PerformanceBase(toTimeOrigin(frame)),
+      DOMWindowProperty(frame),
+      m_observingLongTasks(false) {}
+
+Performance::~Performance() {
+  LocalFrame* localRoot = frame()->localFrameRoot();

[krasin1, 2016/11/02 20:06:14]

For the record, this broke UBSan bots:
https://build.chromium.org/p/chromium.fyi/builders/UBSanVptr%20Linux/builds/1452

The reason is that frame() can return NULL on destruction. What is the correct
fix here? Just quit from the destructor early?

[panickercorp, 2016/11/02 20:17:47]

Apologies!
Should that have run on presubmit?

I'll prepare a fix and send right away. Thanks!
(yes cannot assume frame is non-null)

+  if (m_observingLongTasks && localRoot) {
+    m_observingLongTasks = false;
+    localRoot->disableInspectorWebPerfAgent(this);
+  }
+}
 
 ExecutionContext* Performance::getExecutionContext() const {
   if (!frame())
@@ -88,16 +95,20 @@ PerformanceTiming* Performance::timing() const {
 }
 
 void Performance::updateLongTaskInstrumentation() {
-  if (hasObserverFor(PerformanceEntry::LongTask) && !m_longTaskInspectorAgent) {
-    if (!frame() || !frame()->document() ||
-        !OriginTrials::longTaskObserverEnabled(frame()->document()))
-      return;
-    m_longTaskInspectorAgent = new InspectorWebPerfAgent(frame());
-    m_longTaskInspectorAgent->enable();
-  } else if (!hasObserverFor(PerformanceEntry::LongTask) &&
-             m_longTaskInspectorAgent) {
-    m_longTaskInspectorAgent->disable();
-    m_longTaskInspectorAgent = nullptr;
+  DCHECK(frame());
+  if (!frame()->document() ||
+      !OriginTrials::longTaskObserverEnabled(frame()->document()))
+    return;
+  LocalFrame* localRoot = frame()->localFrameRoot();
+  DCHECK(localRoot);
+
+  if (!m_observingLongTasks && hasObserverFor(PerformanceEntry::LongTask)) {
+    m_observingLongTasks = true;
+    localRoot->enableInspectorWebPerfAgent(this);
+  } else if (m_observingLongTasks &&
+             !hasObserverFor(PerformanceEntry::LongTask)) {
+    m_observingLongTasks = false;
+    localRoot->disableInspectorWebPerfAgent(this);
   }
 }
 
@@ -108,10 +119,13 @@ ScriptValue Performance::toJSONForBinding(ScriptState* scriptState) const {
   return result.scriptValue();
 }
 
+bool Performance::observingLongTasks() {
+  return m_observingLongTasks;
+}
+
 DEFINE_TRACE(Performance) {
   visitor->trace(m_navigation);
   visitor->trace(m_timing);
-  visitor->trace(m_longTaskInspectorAgent);
   DOMWindowProperty::trace(visitor);
   PerformanceBase::trace(visitor);
 }