Skip to content

GitLab

T
tangled
Commit 0b4a4a6e authored by Nicolas Pena's avatar Nicolas Pena Committed by Commit Bot
Browse files
Options

Omit reporting tasks whose local root does not match monitor's local root 

Currently, if two tabs share the renderer process then the longtasks from one
tab are also reported to observers in the other tab. This CL fixes that by
ensuring that only tasks involving the PerformanceMonitor's |local_root_| are
reported by the PerformanceMonitor.

Bug: chromium:736509
Change-Id: I3be72ec81c0c2735b6a092f848f4c4b6cb58ddfe
Reviewed-on: https://chromium-review.googlesource.com/705175Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarTimothy Dresser <tdresser@chromium.org>
Reviewed-by: default avatarShubhie Panicker <panicker@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509274}
parent 80ce5941
Hide whitespace changes
Inline Side-by-side
Showing with 113 additions and 6 deletions
third_party/WebKit/LayoutTests/VirtualTestSuites
View file @ 0b4a4a6e
...@@ -565,5 +565,10 @@ ...@@ -565,5 +565,10 @@
"prefix": "cors-rfc1918", "prefix": "cors-rfc1918",
"base": "http/tests/security/cors-rfc1918", "base": "http/tests/security/cors-rfc1918",
"args": ["--enable-blink-features=CorsRFC1918"] "args": ["--enable-blink-features=CorsRFC1918"]
},
{
"prefix": "single-renderer-process",
"base": "external/wpt/longtask-timing/shared-renderer",
"args": ["--renderer-process-limit=1"]
} }
] ]
third_party/WebKit/LayoutTests/external/wpt/longtask-timing/shared-renderer/longtask-in-new-window.html 0 → 100644
View file @ 0b4a4a6e
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>LongTask Timing: long task in another window</title>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
/* This test should pass even when windows share a single renderer process.
This window opens a new window which contains a longtask. We test that the
longtask from the new window is not observed by the observer of this window. */
async_test(t => {
const observer = new PerformanceObserver(
t.step_func(function (entryList) {
const entries = entryList.getEntries();
let markFound = false;
for (let i = 0; i < entries.length; ++i) {
const entry = entries[i];
// We do not expect to observe longtasks but the work being made in this window may produce a longtask.
assert_true(entry.entryType === 'longtask' ||
entry.entryType === 'mark');
if (entry.entryType === 'mark') {
markFound = true;
continue;
}
// If a longtask is observed, it must come from this window.
assert_equals(entry.name, 'self');
}
// If we found the mark, then the other window longtask is done.
if (markFound)
t.done();
})
);
observer.observe({entryTypes: ['mark', 'longtask']});
// Open a window with a longtask.
const other_window = window.open('resources/frame-with-longtask.html');
window.addEventListener('message', t.step_func(e => {
// Do a mark (after the other window's longtask) to fire the callback.
self.performance.mark('mark1');
}));
}, 'A longtask in a frame from window.open is not reported in original frame');
</script>
</body>
third_party/WebKit/LayoutTests/external/wpt/longtask-timing/shared-renderer/resources/frame-with-longtask.html 0 → 100644
View file @ 0b4a4a6e
<!DOCTYPE HTML>
<meta charset=utf-8>
<meta name="viewport" content="width=device-width">
<title>Long Task Frame</title>
<body>
<h1>Long Task plus PostMessage</h1>
<script>
const begin = window.performance.now();
while (window.performance.now() < begin + 51);
window.opener.postMessage('Finished.', '*');
</script>
</body>
third_party/WebKit/LayoutTests/virtual/single-renderer-process/external/wpt/longtask-timing/shared-renderer/README.txt 0 → 100644
View file @ 0b4a4a6e
# This suite runs the external/wpt/longtask-timing/shared-renderer tests with
# the flag --renderer-process-limit=1.
third_party/WebKit/Source/core/frame/PerformanceMonitor.cpp
View file @ 0b4a4a6e
...@@ -137,17 +137,29 @@ void PerformanceMonitor::WillExecuteScript(ExecutionContext* context) { ...@@ -137,17 +137,29 @@ void PerformanceMonitor::WillExecuteScript(ExecutionContext* context) {
// In V2, timing of script execution along with style & layout updates will be // In V2, timing of script execution along with style & layout updates will be
// accounted for detailed and more accurate attribution. // accounted for detailed and more accurate attribution.
++script_depth_; ++script_depth_;
if (!task_execution_context_) UpdateTaskAttribution(context);
task_execution_context_ = context;
else if (task_execution_context_ != context)
task_has_multiple_contexts_ = true;
} }
void PerformanceMonitor::DidExecuteScript() { void PerformanceMonitor::DidExecuteScript() {
--script_depth_; --script_depth_;
} }
void PerformanceMonitor::UpdateTaskAttribution(ExecutionContext* context) {
// If |context| is not a document, unable to attribute a frame context.
if (!context || !context->IsDocument())
return;
LocalFrame* frame_context = ToDocument(context)->GetFrame();
if (frame_context && local_root_ == &(frame_context->LocalFrameRoot()))
task_should_be_reported_ = true;
if (!task_execution_context_)
task_execution_context_ = context;
else if (task_execution_context_ != context)
task_has_multiple_contexts_ = true;
}
void PerformanceMonitor::Will(const probe::RecalculateStyle& probe) { void PerformanceMonitor::Will(const probe::RecalculateStyle& probe) {
task_should_be_reported_ = true;
if (enabled_ && thresholds_[kLongLayout] && script_depth_) if (enabled_ && thresholds_[kLongLayout] && script_depth_)
probe.CaptureStartTime(); probe.CaptureStartTime();
} }
...@@ -159,6 +171,7 @@ void PerformanceMonitor::Did(const probe::RecalculateStyle& probe) { ...@@ -159,6 +171,7 @@ void PerformanceMonitor::Did(const probe::RecalculateStyle& probe) {
void PerformanceMonitor::Will(const probe::UpdateLayout& probe) { void PerformanceMonitor::Will(const probe::UpdateLayout& probe) {
++layout_depth_; ++layout_depth_;
task_should_be_reported_ = true;
if (!enabled_) if (!enabled_)
return; return;
if (layout_depth_ > 1 || !script_depth_ || !thresholds_[kLongLayout]) if (layout_depth_ > 1 || !script_depth_ || !thresholds_[kLongLayout])
...@@ -226,6 +239,7 @@ void PerformanceMonitor::Did(const probe::CallFunction& probe) { ...@@ -226,6 +239,7 @@ void PerformanceMonitor::Did(const probe::CallFunction& probe) {
} }
void PerformanceMonitor::Will(const probe::V8Compile& probe) { void PerformanceMonitor::Will(const probe::V8Compile& probe) {
UpdateTaskAttribution(probe.context);
if (!enabled_ || !thresholds_[kLongTask]) if (!enabled_ || !thresholds_[kLongTask])
return; return;
...@@ -256,7 +270,7 @@ void PerformanceMonitor::Did(const probe::V8Compile& probe) { ...@@ -256,7 +270,7 @@ void PerformanceMonitor::Did(const probe::V8Compile& probe) {
void PerformanceMonitor::Will(const probe::UserCallback& probe) { void PerformanceMonitor::Will(const probe::UserCallback& probe) {
++user_callback_depth_; ++user_callback_depth_;
UpdateTaskAttribution(probe.context);
if (!enabled_ || user_callback_depth_ != 1 || if (!enabled_ || user_callback_depth_ != 1 ||
!thresholds_[probe.recurring ? kRecurringHandler : kHandler]) !thresholds_[probe.recurring ? kRecurringHandler : kHandler])
return; return;
...@@ -284,6 +298,7 @@ void PerformanceMonitor::WillProcessTask(double start_time) { ...@@ -284,6 +298,7 @@ void PerformanceMonitor::WillProcessTask(double start_time) {
// as it is needed in ReportTaskTime which occurs after didProcessTask. // as it is needed in ReportTaskTime which occurs after didProcessTask.
task_execution_context_ = nullptr; task_execution_context_ = nullptr;
task_has_multiple_contexts_ = false; task_has_multiple_contexts_ = false;
task_should_be_reported_ = false;
if (!enabled_) if (!enabled_)
return; return;
...@@ -298,7 +313,7 @@ void PerformanceMonitor::WillProcessTask(double start_time) { ...@@ -298,7 +313,7 @@ void PerformanceMonitor::WillProcessTask(double start_time) {
} }
void PerformanceMonitor::DidProcessTask(double start_time, double end_time) { void PerformanceMonitor::DidProcessTask(double start_time, double end_time) {
if (!enabled_) if (!enabled_ || !task_should_be_reported_)
return; return;
double layout_threshold = thresholds_[kLongLayout]; double layout_threshold = thresholds_[kLongLayout];
if (layout_threshold && per_task_style_and_layout_time_ > layout_threshold) { if (layout_threshold && per_task_style_and_layout_time_ > layout_threshold) {
......
third_party/WebKit/Source/core/frame/PerformanceMonitor.h
View file @ 0b4a4a6e
...@@ -134,6 +134,8 @@ class CORE_EXPORT PerformanceMonitor final ...@@ -134,6 +134,8 @@ class CORE_EXPORT PerformanceMonitor final
void WillExecuteScript(ExecutionContext*); void WillExecuteScript(ExecutionContext*);
void DidExecuteScript(); void DidExecuteScript();
void UpdateTaskAttribution(ExecutionContext*);
std::pair<String, DOMWindow*> SanitizedAttribution( std::pair<String, DOMWindow*> SanitizedAttribution(
const HeapHashSet<Member<Frame>>& frame_contexts, const HeapHashSet<Member<Frame>>& frame_contexts,
Frame* observer_frame); Frame* observer_frame);
...@@ -153,6 +155,7 @@ class CORE_EXPORT PerformanceMonitor final ...@@ -153,6 +155,7 @@ class CORE_EXPORT PerformanceMonitor final
Member<LocalFrame> local_root_; Member<LocalFrame> local_root_;
Member<ExecutionContext> task_execution_context_; Member<ExecutionContext> task_execution_context_;
bool task_has_multiple_contexts_ = false; bool task_has_multiple_contexts_ = false;
bool task_should_be_reported_ = false;
using ClientThresholds = HeapHashMap<WeakMember<Client>, double>; using ClientThresholds = HeapHashMap<WeakMember<Client>, double>;
HeapHashMap<Violation, HeapHashMap<Violation,
Member<ClientThresholds>, Member<ClientThresholds>,
......
third_party/WebKit/Source/core/frame/PerformanceMonitorTest.cpp
View file @ 0b4a4a6e
...@@ -43,6 +43,10 @@ class PerformanceMonitorTest : public ::testing::Test { ...@@ -43,6 +43,10 @@ class PerformanceMonitorTest : public ::testing::Test {
void DidProcessTask(double start_time, double end_time) { void DidProcessTask(double start_time, double end_time) {
monitor_->DidProcessTask(start_time, end_time); monitor_->DidProcessTask(start_time, end_time);
} }
void UpdateTaskAttribution(ExecutionContext* execution_context) {
monitor_->UpdateTaskAttribution(execution_context);
}
bool TaskShouldBeReported() { return monitor_->task_should_be_reported_; }
String FrameContextURL(); String FrameContextURL();
int NumUniqueFrameContextsSeen(); int NumUniqueFrameContextsSeen();
...@@ -133,4 +137,23 @@ TEST_F(PerformanceMonitorTest, NoScriptInLongTask) { ...@@ -133,4 +137,23 @@ TEST_F(PerformanceMonitorTest, NoScriptInLongTask) {
EXPECT_EQ(0, NumUniqueFrameContextsSeen()); EXPECT_EQ(0, NumUniqueFrameContextsSeen());
} }
TEST_F(PerformanceMonitorTest, TaskWithoutLocalRoot) {
WillProcessTask(1234.5678);
UpdateTaskAttribution(AnotherExecutionContext());
DidProcessTask(1234.5678, 2345.6789);
EXPECT_FALSE(TaskShouldBeReported());
EXPECT_EQ(1, NumUniqueFrameContextsSeen());
}
TEST_F(PerformanceMonitorTest, TaskWithLocalRoot) {
WillProcessTask(1234.5678);
UpdateTaskAttribution(GetExecutionContext());
EXPECT_TRUE(TaskShouldBeReported());
EXPECT_EQ(1, NumUniqueFrameContextsSeen());
UpdateTaskAttribution(AnotherExecutionContext());
DidProcessTask(1234.5678, 2345.6789);
EXPECT_TRUE(TaskShouldBeReported());
EXPECT_EQ(2, NumUniqueFrameContextsSeen());
}
} // namespace blink } // namespace blink
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment