Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Look into assert_throws(null, ...) usage #5317

Closed
annevk opened this issue Apr 3, 2017 · 4 comments
Closed

Look into assert_throws(null, ...) usage #5317

annevk opened this issue Apr 3, 2017 · 4 comments
Labels
type:cleanup

Comments

@annevk
Copy link
Member

annevk commented Apr 3, 2017

All exceptions should be well-defined, so any such usage is arguably a bug. Perhaps with some work it can be removed.
annevk added a commit that referenced this issue Apr 5, 2017
@annevk
Reduce assert_throws(null, ...) usage in cors/ and selectors/
a15117d 
Addresses part of #5317.
@annevk annevk mentioned this issue Apr 5, 2017
Merged
@annevk
Copy link
Member Author

annevk commented Apr 5, 2017 •
edited

There's not a lot:

2dcontext/tools/gentestutils.py:                r'assert_throws(null, function() { \1; });',
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.getItem(0)}, "baseVal.getItem() on an empty list should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.initialize(null)}, "baseVal.initialize(null) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.initialize(30)}, "baseVal.initialize(30) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.initialize("someString")}, "initialize('someString') should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.initialize(new Object())}, "initialize(new Object()) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.getItem(30)}, "baseVal.getItem(30) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore(null, 0)}, "baseVal.insertItemBefore(null, 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore("someString", 0)}, "baseVal.insertItemBefore('someString', 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore(new Object(), 0)}, "baseVal.insertItemBefore(new Object(), 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore(t, null)}, "baseVal.insertItemBefore(t, null) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore(t, "someString")}, "baseVal.insertItemBefore(t, 'someString') should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.insertItemBefore(t, new Object())}, "baseVal.insertItemBefore(t, new Object()) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem(null, 0)}, "baseVal.replaceItem(null, 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem("someString", 0)}, "baseVal.replaceItem('someString', 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem(new Object(), 0)}, "baseVal.replaceItem(new Object(), 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem(t, null)}, "baseVal.replaceItem(t, null) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem(t, "someString")}, "baseVal.replaceItem(t, 'someString') should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.replaceItem(t, new Object())}, "baseVal.replaceItem(t, new Object()) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem(null, 0)}, "baseVal.appendItem(null, 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem("someString", 0)}, "baseVal.appendItem('someString', 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem(new Object(), 0)}, "baseVal.appendItem(new Object(), 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem(t, null)}, "baseVal.appendItem(t, null) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem(t, "someString")}, "baseVal.appendItem(t, 'someString') should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.appendItem(t, new Object())}, "baseVal.appendItem(t, new Object()) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.removeItem(10)}, "baseVal.removeItem() should throw an error on invalid index");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.removeItem(-1)}, "baseVal.removeItem() should throw an error on invalid index");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.removeItem(null)}, "baseVal.removeItem() should throw an error on invalid index");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.removeItem("someString")}, "baseVal.removeItem() should throw an error on invalid index");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.removeItem(new Object())}, "baseVal.removeItem() should throw an error on invalid index");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.createSVGTransformFromMatrix(null)}, "baseVal.createSVGTransformFromMatrix(null) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.createSVGTransformFromMatrix("someString")}, "baseVal.createSVGTransformFromMatrix('someString', 0) should throw an error");
css/work-in-progress/adobe/svg-transform/dom/js/SVGTransformListBasicCommon.js:	assert_throws(null, function(){tList.baseVal.createSVGTransformFromMatrix(new Object())}, "baseVal.createSVGTransformFromMatrix(new Object(), 0) should throw an error");
html/browsers/origin/cross-origin-objects/cross-origin-objects.html:  assert_throws(null, func, description);
service-workers/service-worker/client-navigate.https.html:        assert_throws(null, function() { return frame.contentWindow.location.href });

annevk added a commit that referenced this issue Apr 5, 2017
@annevk
Indexed DB: remove assert_throws(null, ...) usage
a082eaf 
Helps with #5317.
@annevk annevk mentioned this issue Apr 5, 2017
Merged
inexorabletash pushed a commit that referenced this issue Apr 6, 2017
@annevk @inexorabletash
Indexed DB: remove assert_throws(null, ...) usage (#5391)
1bcfa6e 
Helps with #5317.
This was referenced Apr 6, 2017
Merged
Merged
Merged
annevk added a commit that referenced this issue Apr 6, 2017
@annevk
CSSOM: make deleteMedium() exception explicit and remove two redundan…
6714c64 
…t tests

Helps with #5317.
jgraham pushed a commit that referenced this issue Apr 7, 2017
@annevk @jgraham
Reduce assert_throws(null, ...) usage in cors/ and selectors/
2db5c6f 
Addresses part of #5317.
@annevk annevk mentioned this issue Apr 10, 2017
Merged
annevk added a commit that referenced this issue Apr 10, 2017
@annevk
CSS svg-transform from Adobe: correct assert_throws usage
7ed7a8f 
Helps with #5317.
@annevk annevk mentioned this issue Apr 10, 2017
Merged
@annevk
Copy link
Member Author

annevk commented Apr 10, 2017

@jakearchibald @wanderview for service-workers/service-worker/client-navigate.https.html do you know what the assert_throws(null, ...) can be replaced with? Is this just cross-origin access that should fail? So should be as easy as s/null/"SecurityError"/?

@wanderview
Copy link
Member

for service-workers/service-worker/client-navigate.https.html do you know what the assert_throws(null, ...) can be replaced with? Is this just cross-origin access that should fail? So should be as easy as s/null/"SecurityError"/?

I'm in the middle of fixing some other problems in this test here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1351935

I just tried updating my patch to also use "SecurityError" here and it passes in FF. And yes, these checks are just to verify the iframe is actually treated as cross-origin.

Do you want me to just fix this while I'm in the middle of changing the other parts of the test? That way we could avoid conflicting.

@annevk
Copy link
Member Author

annevk commented Apr 10, 2017

@wanderview that would be great, thanks!

annevk added a commit that referenced this issue Apr 10, 2017
@annevk
HTML: remove test_throws abstraction from cross-origin-objects
dd95b85 
I realize we added this to make sure stuff would at least throw, but
that shows up even if you request a detailed exception.

Furthermore, somehow we ended up masking a failure in Chrome for
[[PreventExtensions]]. (I’m not sure how, but there’s a difference in
the results.)

Removing this helps fixing #5317.
@annevk annevk mentioned this issue Apr 10, 2017
Merged
annevk added a commit that referenced this issue Apr 11, 2017
@annevk
CSS svg-transform from Adobe: correct assert_throws usage
ea0d6f0 
Helps with #5317.
annevk added a commit that referenced this issue Apr 11, 2017
@annevk
Canvas: no need for assert_throws(null, ...)
1246010 
Helps with #5317.
zcorpan pushed a commit that referenced this issue Apr 12, 2017
@annevk @zcorpan
HTML: remove test_throws abstraction from cross-origin-objects
e3957ce 
I realize we added this to make sure stuff would at least throw, but
that shows up even if you request a detailed exception.

Furthermore, somehow we ended up masking a failure in Chrome for
[[PreventExtensions]]. (I’m not sure how, but there’s a difference in
the results.)

Removing this helps fixing #5317.
annevk added a commit that referenced this issue May 9, 2017
@annevk
Disallow assert_throws(null, ...)
5559b13 
Fixes #5317.
@annevk annevk mentioned this issue May 9, 2017
Merged
@jdm jdm closed this as completed in #5856 May 9, 2017
jdm pushed a commit that referenced this issue May 9, 2017
@annevk @jdm
Disallow assert_throws(null, ...) (#5856)
6a5f3b2 
Fixes #5317.
@qyearsley-zz qyearsley-zz mentioned this issue May 30, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type:cleanup
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@wanderview @annevk