Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(382)

Unified Diff: third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp

Issue 2449673002: Refactor InspectorWebPerfAgent: update lifecycle management to be per Local Frame root; replace hea… (Closed)
Patch Set: sync and rebase Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
diff --git a/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp b/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
index 5d539b6f7449780b8807a56677df6cec76528d05..1c026385145b7da198aea7ff91dfaf81401e12c7 100644
--- a/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
+++ b/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
@@ -10,9 +10,7 @@
#include "core/frame/DOMWindow.h"
#include "core/frame/Frame.h"
#include "core/frame/LocalFrame.h"
-#include "core/frame/Location.h"
#include "core/frame/UseCounter.h"
-#include "core/inspector/InspectedFrames.h"
#include "core/timing/DOMWindowPerformance.h"
#include "core/timing/Performance.h"
#include "public/platform/Platform.h"
@@ -33,52 +31,76 @@ bool canAccessOrigin(Frame* frame1, Frame* frame2) {
frame1->securityContext()->getSecurityOrigin();
SecurityOrigin* securityOrigin2 =
frame2->securityContext()->getSecurityOrigin();
- // TODO(panicker): Confirm this pending security review.
return securityOrigin1->canAccess(securityOrigin2);
}
+
} // namespace
-InspectorWebPerfAgent::InspectorWebPerfAgent(LocalFrame* localFrame)
- : m_localFrame(localFrame) {}
+InspectorWebPerfAgent::InspectorWebPerfAgent(LocalFrame* localRoot)
+ : m_localRoot(localRoot) {}
InspectorWebPerfAgent::~InspectorWebPerfAgent() {
DCHECK(!m_enabled);
+ DCHECK(!m_webPerformanceObservers.size());
}
void InspectorWebPerfAgent::enable() {
// Update usage count.
- UseCounter::count(m_localFrame, UseCounter::LongTaskObserver);
+ UseCounter::count(m_localRoot, UseCounter::LongTaskObserver);
Platform::current()->currentThread()->addTaskTimeObserver(this);
Platform::current()->currentThread()->addTaskObserver(this);
- m_localFrame->instrumentingAgents()->addInspectorWebPerfAgent(this);
+ m_localRoot->instrumentingAgents()->addInspectorWebPerfAgent(this);
m_enabled = true;
}
void InspectorWebPerfAgent::disable() {
Platform::current()->currentThread()->removeTaskTimeObserver(this);
Platform::current()->currentThread()->removeTaskObserver(this);
- m_localFrame->instrumentingAgents()->removeInspectorWebPerfAgent(this);
+ m_localRoot->instrumentingAgents()->removeInspectorWebPerfAgent(this);
m_enabled = false;
}
+bool InspectorWebPerfAgent::isEnabled() {
+ return m_enabled;
+}
+
+void InspectorWebPerfAgent::addWebPerformanceObserver(
+ Performance* performance) {
+ DCHECK(m_webPerformanceObservers.find(performance) ==
+ m_webPerformanceObservers.end());
+ m_webPerformanceObservers.add(performance);
+}
+
+void InspectorWebPerfAgent::removeWebPerformanceObserver(
+ Performance* performance) {
+ DCHECK(m_webPerformanceObservers.find(performance) !=
+ m_webPerformanceObservers.end());
+ m_webPerformanceObservers.remove(performance);
+}
+
+bool InspectorWebPerfAgent::hasWebPerformanceObservers() {
+ return m_webPerformanceObservers.size() > 0;
+}
+
void InspectorWebPerfAgent::willExecuteScript(ExecutionContext* context) {
- // Heuristic for minimal frame context attribution: note the Location URL
+ // Heuristic for minimal frame context attribution: note the frame context
// for each script execution. When a long task is encountered,
- // if there is only one Location URL involved, then report it.
- // Otherwise don't report Location URL.
+ // if there is only one frame context involved, then report it.
+ // Otherwise don't report frame context.
// NOTE: This heuristic is imperfect and will be improved in V2 API.
// In V2, timing of script execution along with style & layout updates will be
// accounted for detailed and more accurate attribution.
- if (context->isDocument())
- m_frameContextLocations.add(toDocument(context)->location());
+ if (context->isDocument() && toDocument(context)->frame()) {
+ m_frameContexts.add(toDocument(context)->frame());
+ }
}
void InspectorWebPerfAgent::didExecuteScript() {}
void InspectorWebPerfAgent::willProcessTask() {
- // Reset m_frameContextLocations. We don't clear this in didProcessTask
+ // Reset m_frameContexts. We don't clear this in didProcessTask
// as it is needed in ReportTaskTime which occurs after didProcessTask.
- m_frameContextLocations.clear();
+ m_frameContexts.clear();
}
void InspectorWebPerfAgent::didProcessTask() {}
@@ -88,16 +110,16 @@ void InspectorWebPerfAgent::ReportTaskTime(scheduler::TaskQueue*,
double endTime) {
if (((endTime - startTime) * 1000) <= kLongTaskThresholdMillis)
return;
- DOMWindow* domWindow = m_localFrame->domWindow();
- if (!domWindow)
- return;
- Performance* performance = DOMWindowPerformance::performance(*domWindow);
- DCHECK(performance);
- std::pair<String, DOMWindow*> attribution =
- sanitizedAttribution(m_frameContextLocations, m_localFrame);
- performance->addLongTaskTiming(startTime, endTime, attribution.first,
- attribution.second);
+ for (Performance* performance : m_webPerformanceObservers) {
+ if (!performance->frame())
+ continue;
+ DCHECK(performance->observingLongTasks());
+ std::pair<String, DOMWindow*> attribution =
+ sanitizedAttribution(m_frameContexts, performance->frame());
+ performance->addLongTaskTiming(startTime, endTime, attribution.first,
+ attribution.second);
+ }
}
/**
@@ -105,34 +127,34 @@ void InspectorWebPerfAgent::ReportTaskTime(scheduler::TaskQueue*,
* See detailed Security doc here: http://bit.ly/2duD3F7
*/
std::pair<String, DOMWindow*> InspectorWebPerfAgent::sanitizedAttribution(
- const HeapHashSet<Member<Location>>& frameContextLocations,
+ const HeapHashSet<Member<Frame>>& frames,
Frame* observerFrame) {
- if (frameContextLocations.size() == 0) {
+ if (frames.size() == 0) {
// Unable to attribute as no script was involved.
return std::make_pair(kUnknownAttribution, nullptr);
}
- if (frameContextLocations.size() > 1) {
+ if (frames.size() > 1) {
// Unable to attribute, multiple script execution contents were involved.
return std::make_pair(kAmbugiousAttribution, nullptr);
}
// Exactly one culprit location, attribute based on origin boundary.
- DCHECK_EQ(1u, frameContextLocations.size());
- Location* culpritLocation = *frameContextLocations.begin();
- if (canAccessOrigin(observerFrame, culpritLocation->frame())) {
+ DCHECK_EQ(1u, frames.size());
+ Frame* culpritFrame = *frames.begin();
+ DCHECK(culpritFrame);
+ if (canAccessOrigin(observerFrame, culpritFrame)) {
// From accessible frames or same origin, return culprit location URL.
- return std::make_pair(kSameOriginAttribution,
- culpritLocation->frame()->domWindow());
+ return std::make_pair(kSameOriginAttribution, culpritFrame->domWindow());
}
// For cross-origin, if the culprit is the descendant or ancestor of
// observer then indicate the *closest* cross-origin frame between
// the observer and the culprit, in the corresponding direction.
- if (culpritLocation->frame()->tree().isDescendantOf(observerFrame)) {
+ if (culpritFrame->tree().isDescendantOf(observerFrame)) {
// If the culprit is a descendant of the observer, then walk up the tree
// from culprit to observer, and report the *last* cross-origin (from
// observer) frame. If no intermediate cross-origin frame is found, then
// report the culprit directly.
- Frame* lastCrossOriginFrame = culpritLocation->frame();
- for (Frame* frame = culpritLocation->frame(); frame != observerFrame;
+ Frame* lastCrossOriginFrame = culpritFrame;
+ for (Frame* frame = culpritFrame; frame != observerFrame;
frame = frame->tree().parent()) {
if (!canAccessOrigin(observerFrame, frame)) {
lastCrossOriginFrame = frame;
@@ -141,15 +163,16 @@ std::pair<String, DOMWindow*> InspectorWebPerfAgent::sanitizedAttribution(
return std::make_pair(kDescendantAttribution,
lastCrossOriginFrame->domWindow());
}
- if (observerFrame->tree().isDescendantOf(culpritLocation->frame())) {
+ if (observerFrame->tree().isDescendantOf(culpritFrame)) {
return std::make_pair(kAncestorAttribution, nullptr);
}
return std::make_pair(kCrossOriginAttribution, nullptr);
}
DEFINE_TRACE(InspectorWebPerfAgent) {
- visitor->trace(m_localFrame);
- visitor->trace(m_frameContextLocations);
+ visitor->trace(m_localRoot);
+ visitor->trace(m_frameContexts);
+ visitor->trace(m_webPerformanceObservers);
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698