Skip to content

Commit

Permalink
[UserTimingL3] Fix crash: create mark in worker
Browse files Browse the repository at this point in the history
Currently, the renderer crashes when creating mark entry in worker.
The crash is because PerformanceMark::Create() return nullptr without
setting exception_state. The caller of the function assumes that
no exception indicates the PerformanceMark entry exists. Thus, when the
entry is visited, the crash occurs.

There is another bug hidden in this issue. Currently, when
window-performance is not found, Create() returns nullptr directly. It
should instead also check whether worker-performance exist.

This CL fix these issues.

To verify the change, the CL also changes the related web tests from
*.html to *.any.js, which enables these tests in worker's context.

Bug: 981982

Change-Id: Ia20ce1c194e4db2810ff3f1c52070e5970951600
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696085
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676484}
  • Loading branch information
maxlgu authored and chromium-wpt-export-bot committed Jul 11, 2019
1 parent 11051ca commit 1333238
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 254 deletions.
40 changes: 40 additions & 0 deletions user-timing/mark-entry-constructor.any.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// META: script=resources/user-timing-helper.js

test(()=>{
const entry = new PerformanceMark("name");
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark"});
}, "Mark entry can be created by 'new PerformanceMark(string)'.");

test(()=>{
const entry = new PerformanceMark("name", {});
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark"});
}, "Mark entry can be created by 'new PerformanceMark(string, {})'.");

test(()=>{
const entry = new PerformanceMark("name", {startTime: 1});
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark", startTime: 1});
}, "Mark entry can be created by 'new PerformanceMark(string, {startTime})'.");

test(()=>{
const entry = new PerformanceMark("name", {detail: {info: "abc"}});
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark", detail: {info: "abc"}});
}, "Mark entry can be created by 'new PerformanceMark(string, {detail})'.");

test(()=>{
const entry =
new PerformanceMark("name", {startTime: 1, detail: {info: "abc"}});
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark", startTime: 1, detail: {info: "abc"}});
}, "Mark entry can be created by " +
"'new PerformanceMark(string, {startTime, detail})'.");

test(()=>{
const entry = new PerformanceMark("name");
assert_true(entry instanceof PerformanceMark);
checkEntry(entry, {name: "name", entryType: "mark"});
assert_equals(performance.getEntriesByName("name").length, 0);
}, "Using new PerformanceMark() shouldn't add the entry to performance timeline.");
50 changes: 0 additions & 50 deletions user-timing/mark-entry-constructor.html

This file was deleted.

15 changes: 15 additions & 0 deletions user-timing/mark-errors.any.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
test(function() {
assert_throws(new TypeError(), function() { self.performance.mark("mark1", 123); }, "Number passed as a dict argument should cause type-error.")
}, "Number should be rejected as the mark-options.")

test(function() {
assert_throws(new TypeError(), function() { self.performance.mark("mark1", NaN); }, "NaN passed as a dict argument should cause type-error.")
}, "NaN should be rejected as the mark-options.")

test(function() {
assert_throws(new TypeError(), function() { self.performance.mark("mark1", Infinity); }, "Infinity passed as a dict argument should cause type-error.")
}, "Infinity should be rejected as the mark-options.")

test(function() {
assert_throws(new TypeError(), function() { self.performance.mark("mark1", "string"); }, "String passed as a dict argument should cause type-error.")
}, "String should be rejected as the mark-options.")
23 changes: 0 additions & 23 deletions user-timing/mark-errors.html

This file was deleted.

14 changes: 4 additions & 10 deletions user-timing/mark-l3.html → user-timing/mark-l3.any.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>User Timing L3: mark</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/user-timing-helper.js"></script>
<script>
async_test(function (t) {
// META: script=resources/user-timing-helper.js

async_test(function (t) {
let mark_entries = [];
const expected_entries =
[{ entryType: "mark", name: "mark1", detail: null},
Expand Down Expand Up @@ -41,5 +36,4 @@
returned_entries.push(self.performance.mark("mark8", {startTime: 234.56}));
returned_entries.push(self.performance.mark("mark9", {detail: {count: 3}, startTime: 345.67}));
checkEntries(returned_entries, expected_entries);
}, "mark entries' detail and startTime are customizable.");
</script>
}, "mark entries' detail and startTime are customizable.");
37 changes: 37 additions & 0 deletions user-timing/mark-measure-return-objects.any.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
async_test(function (t) {
self.performance.clearMeasures();
const measure = self.performance.measure("measure1");
assert_true(measure instanceof PerformanceMeasure);
t.done();
}, "L3: performance.measure(name) should return an entry.");

async_test(function (t) {
self.performance.clearMeasures();
const measure = self.performance.measure("measure2",
{ startTime: 12, endTime:23 });
assert_true(measure instanceof PerformanceMeasure);
t.done();
}, "L3: performance.measure(name, param1) should return an entry.");

async_test(function (t) {
self.performance.clearMeasures();
self.performance.mark("1");
self.performance.mark("2");
const measure = self.performance.measure("measure3", "1", "2");
assert_true(measure instanceof PerformanceMeasure);
t.done();
}, "L3: performance.measure(name, param1, param2) should return an entry.");

async_test(function (t) {
self.performance.clearMarks();
const mark = self.performance.mark("mark1");
assert_true(mark instanceof PerformanceMark);
t.done();
}, "L3: performance.mark(name) should return an entry.");

async_test(function (t) {
self.performance.clearMarks();
const mark = self.performance.mark("mark2", { startTime: 34 });
assert_true(mark instanceof PerformanceMark);
t.done();
}, "L3: performance.mark(name, param) should return an entry.");
46 changes: 0 additions & 46 deletions user-timing/mark-measure-return-objects.html

This file was deleted.

35 changes: 35 additions & 0 deletions user-timing/measure-l3.any.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// META: script=resources/user-timing-helper.js

function endTime(entry) {
return entry.startTime + entry.duration;
}

test(function() {
performance.clearMarks();
performance.clearMeasures();
const markEntry = performance.mark("mark", {startTime: 123});
const measureEntry = performance.measure("A", undefined, "mark");
assert_equals(measureEntry.startTime, 0);
assert_equals(endTime(measureEntry), markEntry.startTime);
}, "When the end mark is given and the start is unprovided, the end time of the measure entry should be the end mark's time, the start time should be 0.");

test(function() {
performance.clearMarks();
performance.clearMeasures();
const markEntry = performance.mark("mark", {startTime: 123});
const endMin = performance.now();
const measureEntry = performance.measure("A", "mark", undefined);
const endMax = performance.now();
assert_equals(measureEntry.startTime, markEntry.startTime);
assert_greater_than_equal(endTime(measureEntry), endMin);
assert_greater_than_equal(endMax, endTime(measureEntry));
}, "When the start mark is given and the end is unprovided, the start time of the measure entry should be the start mark's time, the end should be now.");

test(function() {
performance.clearMarks();
performance.clearMeasures();
const markEntry = performance.mark("mark", {startTime: 123});
const measureEntry = performance.measure("A", "mark", "mark");
assert_equals(endTime(measureEntry), markEntry.startTime);
assert_equals(measureEntry.startTime, markEntry.startTime);
}, "When start and end mark are both given, the start time and end time of the measure entry should be the the marks' time, repectively");
40 changes: 0 additions & 40 deletions user-timing/measure-l3.html

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>User Timing L3: measure is customizable</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/user-timing-helper.js"></script>
<script>
function cleanupPerformanceTimeline() {
// META: script=resources/user-timing-helper.js

function cleanupPerformanceTimeline() {
performance.clearMarks();
performance.clearMeasures();
}
}

async_test(function (t) {
async_test(function (t) {
this.add_cleanup(cleanupPerformanceTimeline);
let measureEntries = [];
const timeStamp1 = 784.4;
Expand Down Expand Up @@ -91,9 +86,9 @@
returnedEntries.push(self.performance.measure("measure20", undefined, 'mark1'));
returnedEntries.push(self.performance.measure("measure21", { invalidDict:1 }, 'mark1'));
checkEntries(returnedEntries, expectedEntries);
}, "measure entries' detail and start/end are customizable");
}, "measure entries' detail and start/end are customizable");

test(function() {
test(function() {
this.add_cleanup(cleanupPerformanceTimeline);
assert_throws(new TypeError(), function() {
self.performance.measure("optionsAndNumberEnd", {'start': 2}, 12);
Expand All @@ -107,5 +102,5 @@
assert_throws(new TypeError(), function() {
self.performance.measure("negativeEndInOptions", {'end': -1});
}, "measure cannot have a negative time stamp for end.");
}, "measure should throw a TypeError when passed an invalid argument combination");
</script>
}, "measure should throw a TypeError when passed an invalid argument combination");

Loading

0 comments on commit 1333238

Please sign in to comment.