853283 - nsNavigatorSH::NewResolve breaks when resolving over Xrays

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bobby Holley (:bholley)]

The situation with XPCWN Xrays and resolve hooks is super tenuous. In particular, the resolve hooks have to be aware that |obj| might be an Xray wrapper, and need to be sure to define the associated properties on |obj| (which then gets intercepted the second time through by the Xray isResolving machinery) rather than the underlying reflector.

With my GWNOJO patches, I ran into marionette orange related to the fact that marionette actually appears to be running the test code over Xrays. And the nsNavigatorSH NewResolve hook expects |obj| to be the bonafide reflector, because it grabs the global object and expects that to be the window.

It's easy to fix the Resolve hook here, but Marionette also probably shouldn't be using Xrays. I'll file another bug for that.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Grab the window directly from the navigator in nsNavigatorSH::NewResolve. v1

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 727496 [details] [diff] [review]
Grab the window directly from the navigator in nsNavigatorSH::NewResolve. v1

Oof, good catch.

Comment 3

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=b0a7e6aee6d7

Comment 4

[Andrew McCreight [:mccr8]]

D'oh, it looks like this is something I should have noticed in my Xray NewResolve audit.  Oh well.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Backed out because something in Bobby's mega-push caused Windows debug bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/de90200c61e1

Comment 6

[Ryan VanderMeulen [:RyanVM]]

It appears the issue was a needs-clobber. Re-landed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4489ed4085ab

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4489ed4085ab