Skip to content

Commit 5c571b8

Browse files
foolipchromium-wpt-export-bot
foolip
authored and
chromium-wpt-export-bot
committed
Revert "WPT: Allow window.onload to contain multiple test()s"
This reverts commit 4a03c6c459fdbf11976a424aa02a1d094484134c. Reason for revert: This has caused tests in upstream WPT to fail, blocking unrelated PRs. It was still possible to upstream this because those tests weren't triggered on the change due to a bug: web-platform-tests#37623 There was an attempted fix for this: web-platform-tests#37549 But, quoting jgraham from the WPT Matrix chat: > the actual fix failed a test I wrote and now I need to spend some more time investigating Original change's description: > WPT: Allow `window.onload` to contain multiple `test()`s > > ****************************************************************** > *** SHERIFFS: please don't revert this CL if it causes web_tests > to flake/fail. If that happens, the cause is a bad > test. Please mark that test as flaky/fail in > TestExpectations, with a new crbug. Please block the > new bug against crbug.com/1395228. Thanks! > ****************************************************************** > > Prior to this CL, a test like this: > > ``` > <script> > window.onload = () => { > test((t) => { ... }, 'test 1'); > test((t) => { ... }, 'test 2'); > test((t) => { ... }, 'test 3'); > }; > </script> > ``` > > would not run anything after test #1. The issue is that the testharness > immediately adds a window load handler that marks `all_loaded = true`, > and that ends the tests as soon as the first result from the first test > is processed. (The test runner waits for the first test because > `Tests.prototype.all_done()` also waits until `this.tests.length > 0`.) > There were various mitigating corner cases, such as if you started > the list of tests with a promise_test(), that would increment a > counter that kept the rest of the tests alive. Etc. > > With this CL, the testharness-added window.onload handler runs a > setTimeout(0), so that `all_loaded` is only set to true after all of > the tests are loaded by any window.onload handler. > > This exposed a few tests that should have been failing but were > masked by the lack of test coverage - bugs have been filed for > those. Also, several tests that were working around this via various > means are also cleaned up in this CL. I'm sure there are more of > those. > > Bug: 1395228,1395226,1307772 > Change-Id: I6f12b5922186af4e1e06808ad23b47ceac68559c > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4074305 > Reviewed-by: Weizhong Xia <weizhong@google.com> > Auto-Submit: Mason Freed <masonf@chromium.org> > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Mason Freed <masonf@chromium.org> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1081558} Bug: 1395228,1395226,1307772 Change-Id: Icbddad3a8bb47473bcbc331f424661b9041addf2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111318 Reviewed-by: David Baron <dbaron@chromium.org> Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/main@{#1085925}
1 parent 8073ec1 commit 5c571b8

File tree

6 files changed

+114
-130
lines changed

6 files changed

+114
-130
lines changed

‎content-security-policy/style-src/inline-style-allowed-while-cloning-objects.sub.html

+107-96
Original file line numberDiff line numberDiff line change
@@ -8,108 +8,119 @@
88
<script src="/resources/testharness.js"></script>
99
<script src="/resources/testharnessreport.js"></script>
1010
<script>
11+
setup({ explicit_done: true });
12+
1113
var t = async_test("Test that violation report event was fired");
1214
window.addEventListener("securitypolicyviolation", t.step_func_done(function(e) {
1315
assert_equals(e.violatedDirective, "style-src-attr");
1416
}));
1517
window.onload = function() {
16-
window.nodes = document.getElementById('nodes');
17-
window.node1 = document.getElementById('node1');
18-
window.node1.style.background = "yellow";
19-
window.node1.style.color = "red";
20-
window.node2 = document.getElementById('node1').cloneNode(true);
21-
window.node2.id = "node2";
22-
window.node3 = document.getElementById('node3');
23-
window.node3.style.background = "blue";
24-
window.node3.style.color = "green";
25-
window.node4 = document.getElementById('node3').cloneNode(false);
26-
window.node4.id = "node4";
27-
window.node4.innerHTML = "Node #4";
28-
nodes.appendChild(node1);
29-
nodes.appendChild(node2);
30-
nodes.appendChild(node3);
31-
nodes.appendChild(node4);
32-
test(function() {
33-
assert_equals(node1.style.background.match(/yellow/)[0], "yellow")
34-
});
35-
test(function() {
36-
assert_equals(node2.style.background.match(/yellow/)[0], "yellow")
37-
});
38-
test(function() {
39-
assert_equals(node3.style.background.match(/blue/)[0], "blue")
40-
});
41-
test(function() {
42-
assert_equals(node4.style.background.match(/blue/)[0], "blue")
43-
});
44-
test(function() {
45-
assert_equals(node1.style.color, "red")
46-
});
47-
test(function() {
48-
assert_equals(node2.style.color, "red")
49-
});
50-
test(function() {
51-
assert_equals(node3.style.color, "green")
52-
});
53-
test(function() {
54-
assert_equals(node4.style.color, "green")
55-
});
56-
test(function() {
57-
assert_equals(window.getComputedStyle(node1).background, window.getComputedStyle(node2).background)
58-
});
59-
test(function() {
60-
assert_equals(window.getComputedStyle(node3).background, window.getComputedStyle(node4).background)
61-
});
62-
test(function() {
63-
assert_equals(window.getComputedStyle(node1).color, window.getComputedStyle(node2).color)
64-
});
65-
test(function() {
66-
assert_equals(window.getComputedStyle(node3).color, window.getComputedStyle(node4).color)
67-
});
68-
window.ops = document.getElementById('ops');
69-
ops.style.color = 'red';
70-
window.clonedOps = ops.cloneNode(true);
71-
window.violetOps = document.getElementById('violetOps');
72-
violetOps.style.background = 'rgb(238, 130, 238)';
73-
document.getElementsByTagName('body')[0].appendChild(clonedOps);
74-
test(function() {
75-
assert_equals(ops.style.background, "")
76-
});
77-
test(function() {
78-
assert_equals(ops.style.color, "red")
79-
});
80-
test(function() {
81-
assert_equals(clonedOps.style.background, "")
82-
});
83-
test(function() {
84-
assert_equals(violetOps.style.background.match(/rgb\(238, 130, 238\)/)[0], "rgb(238, 130, 238)")
85-
});
86-
test(function() {
87-
assert_equals(window.getComputedStyle(clonedOps).background, window.getComputedStyle(ops).background)
88-
});
89-
test(function() {
90-
assert_equals(window.getComputedStyle(clonedOps).color, window.getComputedStyle(ops).color)
91-
});
92-
test(function() {
93-
assert_equals(window.getComputedStyle(ops).background, window.getComputedStyle(violetOps).background)
94-
});
95-
test(function() {
96-
assert_equals(window.getComputedStyle(clonedOps).background, window.getComputedStyle(violetOps).background)
97-
});
98-
test(function() {
99-
assert_equals(ops.id, "ops")
100-
});
101-
test(function() {
102-
assert_equals(ops.id, clonedOps.id)
103-
});
104-
test(function() {
105-
let el = document.getElementById("svg");
106-
assert_equals(el.getAttribute("style"), "");
107-
el.style.background = violetOps.style.background;
108-
assert_not_equals(el.style.background, "");
109-
let clone = el.cloneNode(true);
110-
assert_equals(el.style.background, clone.style.background)
111-
}, "non-HTML namespace");
18+
try {
19+
runTests();
20+
} finally {
21+
done();
22+
}
11223
};
24+
25+
function runTests() {
26+
window.nodes = document.getElementById('nodes');
27+
window.node1 = document.getElementById('node1');
28+
window.node1.style.background = "yellow";
29+
window.node1.style.color = "red";
30+
window.node2 = document.getElementById('node1').cloneNode(true);
31+
window.node2.id = "node2";
32+
window.node3 = document.getElementById('node3');
33+
window.node3.style.background = "blue";
34+
window.node3.style.color = "green";
35+
window.node4 = document.getElementById('node3').cloneNode(false);
36+
window.node4.id = "node4";
37+
window.node4.innerHTML = "Node #4";
38+
nodes.appendChild(node1);
39+
nodes.appendChild(node2);
40+
nodes.appendChild(node3);
41+
nodes.appendChild(node4);
42+
test(function() {
43+
assert_equals(node1.style.background.match(/yellow/)[0], "yellow")
44+
});
45+
test(function() {
46+
assert_equals(node2.style.background.match(/yellow/)[0], "yellow")
47+
});
48+
test(function() {
49+
assert_equals(node3.style.background.match(/blue/)[0], "blue")
50+
});
51+
test(function() {
52+
assert_equals(node4.style.background.match(/blue/)[0], "blue")
53+
});
54+
test(function() {
55+
assert_equals(node1.style.color, "red")
56+
});
57+
test(function() {
58+
assert_equals(node2.style.color, "red")
59+
});
60+
test(function() {
61+
assert_equals(node3.style.color, "green")
62+
});
63+
test(function() {
64+
assert_equals(node4.style.color, "green")
65+
});
66+
test(function() {
67+
assert_equals(window.getComputedStyle(node1).background, window.getComputedStyle(node2).background)
68+
});
69+
test(function() {
70+
assert_equals(window.getComputedStyle(node3).background, window.getComputedStyle(node4).background)
71+
});
72+
test(function() {
73+
assert_equals(window.getComputedStyle(node1).color, window.getComputedStyle(node2).color)
74+
});
75+
test(function() {
76+
assert_equals(window.getComputedStyle(node3).color, window.getComputedStyle(node4).color)
77+
});
78+
window.ops = document.getElementById('ops');
79+
ops.style.color = 'red';
80+
window.clonedOps = ops.cloneNode(true);
81+
window.violetOps = document.getElementById('violetOps');
82+
violetOps.style.background = 'rgb(238, 130, 238)';
83+
document.getElementsByTagName('body')[0].appendChild(clonedOps);
84+
test(function() {
85+
assert_equals(ops.style.background, "")
86+
});
87+
test(function() {
88+
assert_equals(ops.style.color, "red")
89+
});
90+
test(function() {
91+
assert_equals(clonedOps.style.background, "")
92+
});
93+
test(function() {
94+
assert_equals(violetOps.style.background.match(/rgb\(238, 130, 238\)/)[0], "rgb(238, 130, 238)")
95+
});
96+
test(function() {
97+
assert_equals(window.getComputedStyle(clonedOps).background, window.getComputedStyle(ops).background)
98+
});
99+
test(function() {
100+
assert_equals(window.getComputedStyle(clonedOps).color, window.getComputedStyle(ops).color)
101+
});
102+
test(function() {
103+
assert_equals(window.getComputedStyle(ops).background, window.getComputedStyle(violetOps).background)
104+
});
105+
test(function() {
106+
assert_equals(window.getComputedStyle(clonedOps).background, window.getComputedStyle(violetOps).background)
107+
});
108+
test(function() {
109+
assert_equals(ops.id, "ops")
110+
});
111+
test(function() {
112+
assert_equals(ops.id, clonedOps.id)
113+
});
114+
test(function() {
115+
let el = document.getElementById("svg");
116+
assert_equals(el.getAttribute("style"), "");
117+
el.style.background = violetOps.style.background;
118+
assert_not_equals(el.style.background, "");
119+
let clone = el.cloneNode(true);
120+
assert_equals(el.style.background, clone.style.background)
121+
}, "non-HTML namespace");
122+
}
123+
113124
</script>
114125
</head>
115126

‎css/cssom-view/negativeMargins.html

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
Hello
1010
</div>
1111
<script>
12+
setup({explicit_done:true});
1213
window.onload = function () {
1314
var outer = document.getElementById('outer');
1415
var inner = document.getElementById('inner');
@@ -25,6 +26,7 @@
2526
[inner, outer, document.body, document.querySelector('html')],
2627
"elementsFromPoint should get sequence [inner, outer, body, html]");
2728
});
29+
done();
2830
};
2931
</script>
3032
</body>

‎html/browsers/the-windowproxy-exotic-object/windowproxy-prototype-setting-goes-cross-origin-domain.sub.html

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
<script>
1414
"use strict";
15+
setup({ explicit_done: true });
16+
1517
window.onload = () => {
1618
const target = frames[0];
1719
const origProto = Object.getPrototypeOf(target);
@@ -31,5 +33,7 @@
3133
testSettingImmutablePrototypeToNewValueOnly(
3234
"Became cross-origin via document.domain", target, origProto,
3335
"the original value from before going cross-origin", { isSameOriginDomain: false });
36+
37+
done();
3438
};
3539
</script>

‎infrastructure/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ This directory contains a number of tests to ensure test running
22
infrastructure is operating correctly:
33

44
* The tests in assumptions/ are designed to test UA assumptions
5-
documented in [assumptions.md](/docs/writing-tests/assumptions.md).
5+
documented in [assumptions.md](/docs/_writing-tests/assumptions.md).
66

77
* The tests in server/ are designed to test the WPT server configuration
88

‎infrastructure/expected-fail/window-onload-test.html

-28
This file was deleted.

‎resources/testharness.js

-5
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,7 @@
9191
}
9292

9393
on_event(window, 'load', function() {
94-
setTimeout(() => {
9594
this_obj.all_loaded = true;
96-
if (tests.all_done()) {
97-
tests.complete();
98-
}
99-
},0);
10095
});
10196

10297
on_event(window, 'message', function(event) {

0 commit comments

Comments
 (0)