Closed Bug 1060521 Opened 5 years ago Closed 5 years ago

Remove infrastructure for Xrayed NewResolve, GetProperty, and SetProperty on XPCWrappedNatives

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: Ms2ger, Assigned: bholley)

References

Details

Attachments

(1 file)

It seems somewhat likely this can happen when WebIDL Window sticks.
Assignee: nobody → bobbyholley
Won't this cause issues with nsDOMClassInfo::NewResolve/nsDOMClassInfo::ResolveConstructor?
Flags: needinfo?(bobbyholley)
(In reply to Peter Van der Beken [:peterv] from comment #3)
> Won't this cause issues with
> nsDOMClassInfo::NewResolve/nsDOMClassInfo::ResolveConstructor?

Yeah - but my thinking was that it seemed hard to imagine any of the remaining XPCWrappedNatives actually being used over Xrays in a way that would cause them to break here. Most of the ones from [1] are chrome-only. Of the ones that aren't:

Blob: we've never had a PreCreate hook for Blob, so we can't use it over Xrays
ModalContentWindow: Not actually on the prototype chain at this point.
Various b2g APIs: Not worried about addons using them.

So that leaves XPathNSResolver, XSLTProcessor, and the CSS Rules. It seems to me that the chances of an addon using those over Xrays and expecting .constructor to work are vanishingly small, but you never know. What do you think?

[1] https://etherpad.mozilla.org/classinfo
Flags: needinfo?(bobbyholley) → needinfo?(peterv)
> Blob: we've never had a PreCreate hook for Blob, so we can't use it over Xrays

And we're about to land Web IDL bindings for it anyway.

> ModalContentWindow: Not actually on the prototype chain at this point.

And gone in general, no?
(In reply to Boris Zbarsky [:bz] from comment #5)
> > ModalContentWindow: Not actually on the prototype chain at this point.
> 
> And gone in general, no?

Yeah, appears to be. It's still on the etherpad, which is why I assumed it was around in some vestigial capacity.
Comment on attachment 8486087 [details] [diff] [review]
Remove infrastructure for Xrayed NewResolve, GetProperty, and SetProperty on XPCWrappedNatives. v1

Review of attachment 8486087 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, let's try it.

::: js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1130,1 @@
>              if (retval)

Please also remove holder_get.
Attachment #8486087 - Flags: review?(peterv) → review+
Flags: needinfo?(peterv)
Summary: Remove XrayUtils::IsXrayResolving → Remove infrastructure for Xrayed NewResolve, GetProperty, and SetProperty on XPCWrappedNatives
https://hg.mozilla.org/mozilla-central/rev/681e48b5fdcd
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.