Closed Bug 1505511 Opened 10 months ago Closed 9 months ago

WeakMap lacks X-ray support

Categories

(Core :: XPConnect, defect, P3)

63 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: hanu6, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

Load up the example addon in about:debugging.

Go to example.org or whatever.

Open console (ctrl+shift+K).

paste in:
(new document.defaultView.WeakMap).get





Actual results:

the example addon gives "undefined", whereas console gives "function get()"


Expected results:

They should be the same.

Some npm packages don't work for extensions because of this.

happens on 63 and dev edition, at least.
Summary: Webextension content script defaultView returns undefined when it shouldn't → Webextension content script (new document.defaultView.WeakMap).get returns undefined when it shouldn't
Product: Firefox → WebExtensions
Component: Untriaged → XPConnect
Product: WebExtensions → Core
Summary: Webextension content script (new document.defaultView.WeakMap).get returns undefined when it shouldn't → WeakMap lacks X-ray support
Blocks: XrayToJS
I wasn't able to reproduce in 65 Nightly. I get:
```
function get()
``` 

hanu6, could you please try again in 65 Nightly?
Tried in 65 Nightly, no change.

content.js:
```
console.log('Expecting "function get()". ',(new document.defaultView.WeakMap).get)
console.log('Expecting "function get()". ',(new window.WeakMap).get)
console.log('Expecting "function random()". ',(window.Math).random)
```

webextension console result:
Expecting "function get()".  undefined content.js:1:1
Expecting "function get()".  undefined content.js:2:1
Expecting "function random()".  undefined content.js:3:1

copy pasted into the console manually:
Expecting "function get()".  function get() debugger eval code:1:1
Expecting "function get()".  function get() debugger eval code:2:1
Expecting "function random()".   function random() debugger eval code:3:1
Hi Bobby, see OP. Could it be that that X-Ray wrappers don't for some reason on WeakMap in a web extension context?
Flags: needinfo?(bobbyholley)
Priority: -- → P3
They are not currently supported, per [1].

Now that we have Xray support for regular map/set (added in bug 1155700), it might be straightforward to add for WeakMap. arai, wdyt?

[1] https://searchfox.org/mozilla-central/rev/7f7c353e969e61a6a85201cc8ad3c3de12ac30d8/js/xpconnect/wrappers/XrayWrapper.cpp#75
Flags: needinfo?(bobbyholley) → needinfo?(arai.unmht)
I'll look into it this weekend
almost there I guess
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45052a87b10f37230a1e868c3520bcaa44cccdb6&selectedJob=212421895
Assignee: nobody → arai.unmht
Status: UNCONFIRMED → NEW
Ever confirmed: true
Modified WeakMapObject to follow the style that MapObject uses
  * use class static fields for
    * Class class_
    * Class protoClass_
    * ClassSpec classSpec_
    * JSPropertySpec properties[]
    * JSFunctionSpec methods[]
    * construct()
    * is()
    * all prototype methods, and _impl for them
Flags: needinfo?(arai.unmht)
Attachment #9025919 - Flags: review?(evilpies)
same for WeakSet.
Attachment #9025920 - Flags: review?(evilpies)
we can support class that has appropriate ClassSpec (added in Part 1 and Part 2) in Xray,
added WeakMap and WeakSet to XrayWrapper.cpp's list.
also added basic test to test_xrayToJS.xul.

then, test_bug1042436.xul and test_xrayToJS.xul were using WeakMap as an example of non-Xrayable object.
Changed them to use MapIterator instead.
Attachment #9025921 - Flags: review?(bobbyholley)
Comment on attachment 9025919 [details] [diff] [review]
Part 1: Use ClassSpec in WeakMapObject.

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

Looks good. Thanks for also doing WeakSet.
Attachment #9025919 - Flags: review?(evilpies) → review+
Attachment #9025920 - Flags: review?(evilpies) → review+
Comment on attachment 9025921 [details] [diff] [review]
Part 3: Support WeakMap and WeakSet in Xray.

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

Thanks!

::: js/xpconnect/tests/unit/test_bug856067.js
@@ +1,4 @@
>  function run_test() {
>    var sb = new Cu.Sandbox('http://www.example.com');
>    let w = Cu.evalInSandbox('var w = new WeakMap(); w.__proto__ = new Set(); w.foopy = 12; w', sb);
> +  Assert.equal(Object.getPrototypeOf(w), sb.WeakMap.prototype);

This test was trying to test an un-Xrayable object. Maybe do the same trick you do in the other test?
Attachment #9025921 - Flags: review?(bobbyholley) → review+
You need to log in before you can comment on or make changes to this bug.