1505511 - WeakMap lacks X-ray support

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P3)

Description

[hanu6]

Attachment: test-0.2.zip example addon

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.

Comment 1

[Marcos Caceres [:marcosc]]

I wasn't able to reproduce in 65 Nightly. I get:
```
function get()
``` 

hanu6, could you please try again in 65 Nightly?

Comment 2

[hanu6]

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

Comment 3

[Marcos Caceres [:marcosc]]

Hi Bobby, see OP. Could it be that that X-Ray wrappers don't for some reason on WeakMap in a web extension context?

Comment 4

[Bobby Holley (:bholley)]

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

Comment 5

[Tooru Fujisawa [:arai]]

I'll look into it this weekend

Comment 6

[Tooru Fujisawa [:arai]]

almost there I guess
https://treeherder.mozilla.org/#/jobs?repo=try&revision=45052a87b10f37230a1e868c3520bcaa44cccdb6&selectedJob=212421895

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Use ClassSpec in WeakMapObject.

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

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Use ClassSpec in WeakSetObject.

same for WeakSet.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Support WeakMap and WeakSet in Xray.

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.

Comment 10

[Tom S [:evilpie]]

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.

Comment 11

[Bobby Holley (:bholley)]

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?

Comment 12

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5d9fa8dc136d167e53b1ae4a7b494d2e691dd60
Bug 1505511 - Part 1: Use ClassSpec in WeakMapObject. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/32915637657a5e92dbdd8abec557c1f229b70cd1
Bug 1505511 - Part 2: Use ClassSpec in WeakSetObject. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/754cc91c9d3b50207679ef25861d662496811c64
Bug 1505511 - Part 3: Support WeakMap and WeakSet in Xray. r=bholley

Comment 13

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/mozilla-central/rev/a5d9fa8dc136
https://hg.mozilla.org/mozilla-central/rev/32915637657a
https://hg.mozilla.org/mozilla-central/rev/754cc91c9d3b