Merge cross-origin-objects.html and cross-origin-objects-exceptions.html #4690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zcorpan
force-pushed
the
zcorpan/merge-cross-origin-objects-tests
branch
from
f6f856a
to
2d5074b
Compare
Chrome (unstable channel)Testing web-platform-tests at revision dfab1c2 All results/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
|
Firefox (nightly channel)Testing web-platform-tests at revision dfab1c2 All results/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
|
bzbarsky
reviewed
Feb 1, 2017
| "preventExtensions on cross-origin Window should throw"); | ||
| assert_throws(null, function() { Object.preventExtensions(C.location) }, | ||
| test_throws(exception_t, "SecurityError", function() { Object.preventExtensions(C.location) }, |
There was a problem hiding this comment.
Should be new TypeError, no?
There really shouldn't have been. Worth checking the diff between the two before the most recent changes to Thank you for doing this. This is much more maintainable... |
|
The diff between cross-origin-objects.html and cross-origin-objects-exceptions.html on current master: diff --git a/html/browsers/origin/cross-origin-objects/cross-origin-objects.html b/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
index 7560fcd..58c362a 100644
--- a/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
+++ b/html/browsers/origin/cross-origin-objects/cross-origin-objects.html
@@ -1,3 +1,5 @@
+<!-- Once most browsers pass this test it can replace cross-origin-objects.html. It is meant to be
+ identical (please verify), except for also checking that the exceptions are correct. -->
<!doctype html>
<meta charset=utf-8>
<meta name="timeout" content="long">
@@ -12,7 +14,6 @@
<iframe id="B"></iframe>
<iframe id="C"></iframe>
<script>
-
/*
* Setup boilerplate. This gives us a same-origin window "B" and a cross-origin
* window "C".
@@ -59,7 +60,7 @@ addTest(function() {
assert_equals(B.parent, window, "window.parent works same-origin");
assert_equals(C.parent, window, "window.parent works cross-origin");
assert_equals(B.location.pathname, path, "location.href works same-origin");
- assert_throws(null, function() { C.location.pathname; }, "location.pathname throws cross-origin");
+ assert_throws("SecurityError", function() { C.location.pathname; }, "location.pathname throws cross-origin");
assert_equals(B.frames, 'override', "Overrides visible in the same-origin case");
assert_equals(C.frames, C, "Overrides invisible in the cross-origin case");
}, "Basic sanity-checking");
@@ -71,31 +72,26 @@ addTest(function() {
*/
var whitelistedWindowIndices = ['0', '1'];
-var whitelistedWindowPropNames = ['location', 'postMessage', 'window', 'frames', 'self', 'top', 'parent',
- 'opener', 'closed', 'close', 'blur', 'focus', 'length'];
-whitelistedWindowPropNames = whitelistedWindowPropNames.concat(whitelistedWindowIndices);
-whitelistedWindowPropNames.sort();
-var whitelistedLocationPropNames = ['href', 'replace'];
-whitelistedLocationPropNames.sort();
-var whitelistedSymbols = [Symbol.toStringTag, Symbol.hasInstance,
- Symbol.isConcatSpreadable];
-var whitelistedWindowProps = whitelistedWindowPropNames.concat(whitelistedSymbols);
+var whitelistedWindowProps = ['location', 'postMessage', 'window', 'frames', 'self', 'top', 'parent',
+ 'opener', 'closed', 'close', 'blur', 'focus', 'length'];
+whitelistedWindowProps = whitelistedWindowProps.concat(whitelistedWindowIndices);
+whitelistedWindowProps.sort();
addTest(function() {
for (var prop in window) {
if (whitelistedWindowProps.indexOf(prop) != -1) {
C[prop]; // Shouldn't throw.
Object.getOwnPropertyDescriptor(C, prop); // Shouldn't throw.
- assert_true(Object.prototype.hasOwnProperty.call(C, prop), "hasOwnProperty for " + String(prop));
+ assert_true(Object.prototype.hasOwnProperty.call(C, prop), "hasOwnProperty for " + prop);
} else {
- assert_throws(null, function() { C[prop]; }, "Should throw when accessing " + String(prop) + " on Window");
- assert_throws(null, function() { Object.getOwnPropertyDescriptor(C, prop); },
+ assert_throws("SecurityError", function() { C[prop]; }, "Should throw when accessing " + prop + " on Window");
+ assert_throws("SecurityError", function() { Object.getOwnPropertyDescriptor(C, prop); },
"Should throw when accessing property descriptor for " + prop + " on Window");
- assert_throws(null, function() { Object.prototype.hasOwnProperty.call(C, prop); },
+ assert_throws("SecurityError", function() { Object.prototype.hasOwnProperty.call(C, prop); },
"Should throw when invoking hasOwnProperty for " + prop + " on Window");
}
if (prop != 'location')
- assert_throws(null, function() { C[prop] = undefined; }, "Should throw when writing to " + prop + " on Window");
+ assert_throws("SecurityError", function() { C[prop] = undefined; }, "Should throw when writing to " + prop + " on Window");
}
for (var prop in location) {
if (prop == 'replace') {
@@ -104,14 +100,14 @@ addTest(function() {
assert_true(Object.prototype.hasOwnProperty.call(C.location, prop), "hasOwnProperty for " + prop);
}
else {
- assert_throws(null, function() { C[prop]; }, "Should throw when accessing " + prop + " on Location");
- assert_throws(null, function() { Object.getOwnPropertyDescriptor(C, prop); },
+ assert_throws("SecurityError", function() { C[prop]; }, "Should throw when accessing " + prop + " on Location");
+ assert_throws("SecurityError", function() { Object.getOwnPropertyDescriptor(C, prop); },
"Should throw when accessing property descriptor for " + prop + " on Location");
- assert_throws(null, function() { Object.prototype.hasOwnProperty.call(C, prop); },
+ assert_throws("SecurityError", function() { Object.prototype.hasOwnProperty.call(C, prop); },
"Should throw when invoking hasOwnProperty for " + prop + " on Location");
}
if (prop != 'href')
- assert_throws(null, function() { C[prop] = undefined; }, "Should throw when writing to " + prop + " on Location");
+ assert_throws("SecurityError", function() { C[prop] = undefined; }, "Should throw when writing to " + prop + " on Location");
}
}, "Only whitelisted properties are accessible cross-origin");
@@ -128,8 +124,8 @@ addTest(function() {
var protoGetter = Object.getOwnPropertyDescriptor(Object.prototype, '__proto__').get;
assert_true(protoGetter.call(C) === null, "cross-origin Window proto is null");
assert_true(protoGetter.call(C.location) === null, "cross-origin Location proto is null (__proto__)");
- assert_throws(null, function() { C.__proto__; }, "__proto__ property not available cross-origin");
- assert_throws(null, function() { C.location.__proto__; }, "__proto__ property not available cross-origin");
+ assert_throws("SecurityError", function() { C.__proto__; }, "__proto__ property not available cross-origin");
+ assert_throws("SecurityError", function() { C.location.__proto__; }, "__proto__ property not available cross-origin");
}, "[[GetPrototypeOf]] should return null");
@@ -137,14 +133,14 @@ addTest(function() {
* [[SetPrototypeOf]]
*/
addTest(function() {
- assert_throws(null, function() { C.__proto__ = new Object(); }, "proto set on cross-origin Window");
- assert_throws(null, function() { C.location.__proto__ = new Object(); }, "proto set on cross-origin Location");
+ assert_throws("SecurityError", function() { C.__proto__ = new Object(); }, "proto set on cross-origin Window");
+ assert_throws("SecurityError", function() { C.location.__proto__ = new Object(); }, "proto set on cross-origin Location");
var setters = [Object.getOwnPropertyDescriptor(Object.prototype, '__proto__').set];
if (Object.setPrototypeOf)
setters.push(function(p) { Object.setPrototypeOf(this, p); });
setters.forEach(function(protoSetter) {
- assert_throws(null, function() { protoSetter.call(C, new Object()); }, "proto setter |call| on cross-origin Window");
- assert_throws(null, function() { protoSetter.call(C.location, new Object()); }, "proto setter |call| on cross-origin Location");
+ assert_throws(new TypeError, function() { protoSetter.call(C, new Object()); }, "proto setter |call| on cross-origin Window");
+ assert_throws(new TypeError, function() { protoSetter.call(C.location, new Object()); }, "proto setter |call| on cross-origin Location");
});
}, "[[SetPrototypeOf]] should throw");
@@ -160,9 +156,9 @@ addTest(function() {
* [[PreventExtensions]]
*/
addTest(function() {
- assert_throws(null, function() { Object.preventExtensions(C) },
+ assert_throws(new TypeError, function() { Object.preventExtensions(C) },
"preventExtensions on cross-origin Window should throw");
- assert_throws(null, function() { Object.preventExtensions(C.location) },
+ assert_throws(new TypeError, function() { Object.preventExtensions(C.location) },
"preventExtensions on cross-origin Location should throw");
}, "[[PreventExtensions]] should throw for cross-origin objects");
@@ -178,18 +174,9 @@ addTest(function() {
}, "[[GetOwnProperty]] - Properties on cross-origin objects should be reported |own|");
function checkPropertyDescriptor(desc, propName, expectWritable) {
- var isSymbol = (typeof(propName) == "symbol");
- propName = String(propName);
assert_true(isObject(desc), "property descriptor for " + propName + " should exist");
assert_equals(desc.enumerable, false, "property descriptor for " + propName + " should be non-enumerable");
assert_equals(desc.configurable, true, "property descriptor for " + propName + " should be configurable");
- if (isSymbol) {
- assert_true("value" in desc,
- "property descriptor for " + propName + " should be a value descriptor");
- assert_equals(desc.value, undefined,
- "symbol-named cross-origin visible prop " + propName +
- " should come back as undefined");
- }
if ('value' in desc)
assert_equals(desc.writable, expectWritable, "property descriptor for " + propName + " should have writable: " + expectWritable);
else
@@ -205,27 +192,23 @@ addTest(function() {
checkPropertyDescriptor(Object.getOwnPropertyDescriptor(C.location, 'replace'), 'replace', false);
checkPropertyDescriptor(Object.getOwnPropertyDescriptor(C.location, 'href'), 'href', true);
assert_equals(typeof Object.getOwnPropertyDescriptor(C.location, 'href').get, 'undefined', "Cross-origin location should have no href getter");
- whitelistedSymbols.forEach(function(prop) {
- var desc = Object.getOwnPropertyDescriptor(C.location, prop);
- checkPropertyDescriptor(desc, prop, false);
- });
}, "[[GetOwnProperty]] - Property descriptors for cross-origin properties should be set up correctly");
/*
* [[Delete]]
*/
addTest(function() {
- assert_throws(null, function() { delete C[0]; }, "Can't delete cross-origin indexed property");
- assert_throws(null, function() { delete C[100]; }, "Can't delete cross-origin indexed property");
- assert_throws(null, function() { delete C.location; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.parent; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.length; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.document; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.foopy; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.location.href; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.location.replace; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.location.port; }, "Can't delete cross-origin property");
- assert_throws(null, function() { delete C.location.foopy; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C[0]; }, "Can't delete cross-origin indexed property");
+ assert_throws("SecurityError", function() { delete C[100]; }, "Can't delete cross-origin indexed property");
+ assert_throws("SecurityError", function() { delete C.location; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.parent; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.length; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.document; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.foopy; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.location.href; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.location.replace; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.location.port; }, "Can't delete cross-origin property");
+ assert_throws("SecurityError", function() { delete C.location.foopy; }, "Can't delete cross-origin property");
}, "[[Delete]] Should throw on cross-origin objects");
/*
@@ -234,8 +217,8 @@ addTest(function() {
function checkDefine(obj, prop) {
var valueDesc = { configurable: true, enumerable: false, writable: false, value: 2 };
var accessorDesc = { configurable: true, enumerable: false, get: function() {} };
- assert_throws(null, function() { Object.defineProperty(obj, prop, valueDesc); }, "Can't define cross-origin value property " + prop);
- assert_throws(null, function() { Object.defineProperty(obj, prop, accessorDesc); }, "Can't define cross-origin accessor property " + prop);
+ assert_throws("SecurityError", function() { Object.defineProperty(obj, prop, valueDesc); }, "Can't define cross-origin value property " + prop);
+ assert_throws("SecurityError", function() { Object.defineProperty(obj, prop, accessorDesc); }, "Can't define cross-origin accessor property " + prop);
}
addTest(function() {
checkDefine(C, 'length');
@@ -265,44 +248,13 @@ addTest(function() {
*/
addTest(function() {
- assert_array_equals(Object.getOwnPropertyNames(C).sort(),
- whitelistedWindowPropNames,
+ assert_array_equals(Object.getOwnPropertyNames(C).sort(), whitelistedWindowProps.sort(),
"Object.getOwnPropertyNames() gives the right answer for cross-origin Window");
- assert_array_equals(Object.getOwnPropertyNames(C.location).sort(),
- whitelistedLocationPropNames,
+ assert_array_equals(Object.getOwnPropertyNames(C.location).sort(), ['href', 'replace'],
"Object.getOwnPropertyNames() gives the right answer for cross-origin Location");
}, "[[OwnPropertyKeys]] should return all properties from cross-origin objects");
addTest(function() {
- assert_array_equals(Object.getOwnPropertySymbols(C), whitelistedSymbols,
- "Object.getOwnPropertySymbols() should return the three symbol-named properties that are exposed on a cross-origin Window");
- assert_array_equals(Object.getOwnPropertySymbols(C.location),
- whitelistedSymbols,
- "Object.getOwnPropertySymbols() should return the three symbol-named properties that are exposed on a cross-origin Location");
-}, "[[OwnPropertyKeys]] should return the right symbol-named properties for cross-origin objects");
-
-addTest(function() {
- var allWindowProps = Reflect.ownKeys(C);
- indexedWindowProps = allWindowProps.slice(0, whitelistedWindowIndices.length);
- stringWindowProps = allWindowProps.slice(0, -1 * whitelistedSymbols.length);
- symbolWindowProps = allWindowProps.slice(-1 * whitelistedSymbols.length);
- assert_array_equals(indexedWindowProps, whitelistedWindowIndices,
- "Reflect.ownKeys should start with the indices exposed on the cross-origin window.");
- assert_array_equals(stringWindowProps.sort(), whitelistedWindowPropNames,
- "Reflect.ownKeys should continue with the cross-origin window properties for a cross-origin Window.");
- assert_array_equals(symbolWindowProps, whitelistedSymbols,
- "Reflect.ownKeys should end with the cross-origin symbols for a cross-origin Window.");
-
- var allLocationProps = Reflect.ownKeys(C.location);
- stringLocationProps = allLocationProps.slice(0, -1 * whitelistedSymbols.length);
- symbolLocationProps = allLocationProps.slice(-1 * whitelistedSymbols.length);
- assert_array_equals(stringLocationProps.sort(), whitelistedLocationPropNames,
- "Reflect.ownKeys should start with the cross-origin window properties for a cross-origin Location.")
- assert_array_equals(symbolLocationProps, whitelistedSymbols,
- "Reflect.ownKeys should end with the cross-origin symbols for a cross-origin Location.")
-}, "[[OwnPropertyKeys]] should place the symbols after the property names after the subframe indices");
-
-addTest(function() {
assert_true(B.eval('parent.C') === C, "A and B observe the same identity for C's Window");
assert_true(B.eval('parent.C.location') === C.location, "A and B observe the same identity for C's Location");
}, "A and B jointly observe the same identity for cross-origin Window and Location");
@@ -365,11 +317,6 @@ addTest(function() {
checkFunction(set_href_B, B.Function.prototype);
}, "Same-origin observers get different accessors for cross-origin Location");
-addTest(function() {
- assert_equals({}.toString.call(C), "[object Object]");
- assert_equals({}.toString.call(C.location), "[object Object]");
-}, "{}.toString.call() does the right thing on cross-origin objects");
-
// We do a fresh load of the subframes for each test to minimize side-effects.
// It would be nice to reload ourselves as well, but we can't do that without
// disrupting the test harness. |
zcorpan
commented
Feb 2, 2017
| var whitelistedWindowProps = ['location', 'postMessage', 'window', 'frames', 'self', 'top', 'parent', | ||
| 'opener', 'closed', 'close', 'blur', 'focus', 'length']; | ||
| whitelistedWindowProps = whitelistedWindowProps.concat(whitelistedWindowIndices); | ||
| whitelistedWindowProps.sort(); |
There was a problem hiding this comment.
...no, sorry. This is actually whitelistedWindowPropNames. Sorting the symbols throws.
See #4662 (comment) The exception type check is tested in a separate subtest, but it is now in the same file so we don't get out of sync.
zcorpan
force-pushed
the
zcorpan/merge-cross-origin-objects-tests
branch
from
286a62f
to
352f4ce
Compare
|
This looks good to me. |
|
Looks fine to me too. Thanks for working on this. |
|
I'll start aligning WebKit as soon as this lands. |
|
Notifying @Ms2ger, @ayg, @gsnedders, @jdm, @jgraham, @sideshowbarker, and @zqzhang. (Learn how reviewing works.) These tests will be available shortly on w3c-test.org. |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #4662 (comment)
The exception type check is tested in a separate subtest, but it is
now in the same file so we don't get out of sync.
I changed
cross-origin-objects.htmland removedcross-origin-objects-exceptions.html; it's unclear to me if there was some change for the latter that was not in the former, that we want to keep?cc @cdumez @bzbarsky @domenic