Closed Bug 1245313 Opened 8 years ago Closed 8 years ago

Javascript-navigator-property objects are recreated each time they're accessed over Xrays if they use nsIDOMGlobalPropertyInitializer to replace themselves with some other object

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: myk, Unassigned)

References

Details

(Whiteboard: dom-triaged)

Bug 985827 uses mozApps to confirm that Javascript-navigator-property objects aren't recreated each time they're accessed, but mozApps is a WebIDL NavigatorProperty as of bug 899322, so the tests no longer actually check a Javascript-navigator-property object.

Just in time for bug 1027734 to land, I converted the tests to use mozPay (on Mulet) instead; and in the process I discovered that bug 985827 appears to have regressed for Javascript-navigator-property objects, since the tests fail with the change:

--------------------------------------------------------------------------------
1 INFO TEST-START | dom/base/test/test_navigator_js_resolve_identity_xrays.xul
2 INFO Error: Unable to restore focus, expect failures and timeouts.
3 INFO TEST-PASS | dom/base/test/test_navigator_js_resolve_identity_xrays.xul | Should have an Xray here 
4 INFO TEST-PASS | dom/base/test/test_navigator_js_resolve_identity_xrays.xul | Should have a mozPay function 
5 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_navigator_js_resolve_identity_xrays.xul | Should have gotten the same mozPay function again - got function pay() {
    [native code]
}, expected function pay() {
    [native code]
}

…

25 INFO TEST-START | dom/base/test/test_navigator_js_resolve_identity.html
26 INFO TEST-PASS | dom/base/test/test_navigator_js_resolve_identity.html | Should have a mozPay function 
27 INFO TEST-UNEXPECTED-FAIL | dom/base/test/test_navigator_js_resolve_identity.html | Should have gotten the same mozPay function again - got function pay() {
    [native code]
}, expected function pay() {
    [native code]
}
--------------------------------------------------------------------------------

(Note that the test would pass on desktop Firefox if I enabled it there, but only because mozPay is *null* there, since MOZ_PAY is enabled, but dom.mozPay.enabled is not.  And *null* identity doesn't appear to be affected by the regression.  So you can only see the regression on Mulet.)

My changes are in this branch:

https://github.com/mykmelez/gecko-dev/tree/test-javascript-navigator-property-objects-on-mulet

Here's the diff:

https://github.com/mykmelez/gecko-dev/compare/ab98eed47c4ecc82dadcaea1543d79976f0c5532...test-javascript-navigator-property-objects-on-mulet

Obviously the changes no longer apply because of bug 1027734.  I'm not sure how to update the tests without mozPay, since the only other Javascript-navigator-property object is mozNetworkStats, which is only built on MOZ_B2G targets.

But maybe it doesn't matter, because Javascript-navigator-property objects are going away?
mozNetworkStats is no longer using Javascript-navigator-property either, as of bug 993311 being fixed.  So in general, Javascript-navigator-property should be going away...  There is one extension I'm aware of using it, apart from the various fxos emulators, but maybe we can get them to stop.

But past that, the reason this fails for mozPay is actually quite specific to the mozPay API.  The code after bug 985827 basically does the following:

1)  Create the nsISupports object via the contract registered via Javascript-navigator-property.
2)  QI to nsIDOMGlobalPropertyInitializer.
3)  If QI succeeds, call Init() on the object, which allows it to return a new thing to use as the actual
    value.
4)  If step 3 returned a primitive (esp. undefined), which means "just use the object you already have", then
    cache that object, and create a JS reflector to return.  Otherwise just return the thing from step 3.

Now mozPay does in fact return a different thing from Init(): it returns the function to be called.  So the cache bug 985827 added is skipped in the case of mozPay.  Note also bug 985827 comment 5, which explicitly says the patch in that bug does not fix mozPay because of the way mozPay is set up.

We could try to do something that would handle this case, but I'm more interested in killing off the nsIDOMGlobalPropertyInitializer path altogether for Javascript-navigator-property, even if we don't kill off Javascript-navigator-property itself.  Of course the extension using Javascript-navigator-property also returns a different object, complete with __exposedProps__, from init()...
Summary: Javascript-navigator-property objects are recreated each time they're accessed → Javascript-navigator-property objects are recreated each time they're accessed over Xrays if they use nsIDOMGlobalPropertyInitializer to replace themselves with some other object
Depends on: 1245650
Whiteboard: dom-triaged
Wontfix, since there is no more Javascript-navigator-property as of bug 1245650.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.