Stop implementing JS-exposed QueryInterface on some core DOM objects
Categories
(Core :: DOM: Core & HTML, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox70 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file)
We don't do QI on webidl objects other than Window and Element in Firefox. See try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f45b9e009b38a8df79c04e4e42197e50e04b1a4
This means we can remove QI from Document, DocumentFragment, and Event.
|
||
Comment 1•5 years ago
|
||
On the try push some removals of QueryInterface(Ci.nsIDOMChromeWindow). We have a few of those:
https://searchfox.org/comm-central/search?q=QueryInterface(Ci.nsIDOMChromeWindow)&case=false®exp=false&path=
What exactly will you remove here?
|
Assignee | |
Comment 2•5 years ago
|
||
Like comment 0 says, this will remove QI methdos on Document, DocumentFragment, and Event instances.
I suggest applying https://hg.mozilla.org/try/rev/a3fb3aa2c7e8645c336c23d7bf2c0b97f6dee38d and doing a test run on Thunderbird to see whether this affects you.
|
|
Assignee | |
Comment 3•5 years ago
|
||
|
|
||
Comment 4•5 years ago
|
||
Hmm, the "domparser checkies". But in the other patch you remove QueryInterface(Ci.nsIDOMChromeWindow). Wouldn't we need to do the same? How is DOMParser related to the QI on other DOM objects? Also, isn't DOMParser covered in bug 1568281? I'm confused.
|
|
Assignee | |
Comment 5•5 years ago
|
||
Wouldn't we need to do the same?
That's needed to remove QI on Window objects, but I'm not removing that yet. I'll give you a heads-up of I get close to doing that.
How is DOMParser related to the QI on other DOM objects?
It's not.
Also, isn't DOMParser covered in bug 1568281?
It is.
I'm confused.
You're paying attention to the commit message on a test patch, which has nothing to do with that patch's contents. What that patch actually does is MOZ_CRASH if a QI happens on any Web IDL objects that is not an Element or Window. My current goal with the set of patches I have up is to remove QI for everything that's not Element or Window.
|
|
||
Comment 6•5 years ago
|
||
OK, thanks, here we go:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=66a759acb46876660a75053797cb68e957f69ce0
based on https://hg.mozilla.org/try/rev/e8b7cc3091ffa98ca7ed15242c8e1328fea8ea07
Oh, in case you're looking at it, the Z4 in testCategoryColors.js is currently expected.
|
|
Assignee | |
Comment 7•5 years ago
|
||
OK, you probably want to look at the "MOZ_CRASH(This is not a great thing to be doing)" bits in the debug failure logs. I commented on the one thing I see there in bug 1568285.
|
|
||
Comment 8•5 years ago
|
||
Thanks, let's move the discussion to bug 1568420.
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6598e37c88d2 Stop implementing JS-exposed QueryInterface on some core DOM objects. r=peterv
|
||
Comment 10•5 years ago
|
||
| bugherder | ||
Description
•