1165807 - Show entries for WeakMap and WeakSet in console

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[ziyunfei]

Just like we did for Map and Set.
And both Chrome and Safari(webkit nightly) already support this.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Bug 1172920 is the same but for non-weak versions.

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Note: For iterating weak map items (it feels gross just saying that), one can use the Cu.getJSTestingFunctions().nondeterministicGetWeakMapKeys function.

Comment 3

[Tom Tromey :tromey]

I'm making them show up in the console and deferring the variables view change
to bug 1172920.

Comment 4

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Comment 5

[Tom Tromey :tromey]

This patch works but I think it should probably have more tests for the new
JS API, not just via the webconsole tests.

Comment 6

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

I added a simple test in xpconnect.

Comment 7

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

git add the test

Comment 8

[Tom Tromey :tromey]

Comment on attachment 8682729 [details] [diff] [review]
display WeakSet and WeakMap contents in console

This adds display of the contents of WeakSet and WeakMap to the console.

To make this work for WeakSet, I had to add a new API to JS to fetch
the keys of the WeakSet object.  I exposed this in xpconnect where the
equivalent for WeakMap is exposed.

Comment 9

[Tom Tromey :tromey]

Comment on attachment 8682729 [details] [diff] [review]
display WeakSet and WeakMap contents in console

Cancelling the review while I work on the precursor bug first.

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8682729 [details] [diff] [review]
display WeakSet and WeakMap contents in console

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

Would prefer attaching nondeterministicBlahBlahBlah to ThreadSafeChromeUtils.webidl since we can use that in workers and we are trying to get the console + debugger properly supported on workers. My hope is that it wouldn't be too difficult, but if it turns out to be then feel free to push it off as a follow up.

The rest of the changes look A+ to me.

::: devtools/client/webconsole/test/test-console-table.html
@@ +45,5 @@
>        mySet.add(null);
>        mySet.add(undefined);
> +
> +      var bunnies = new String("bunnies");
> +      var lizards = new String("lizards");

And we know these will still be around when we preview the weak set because these are global variables and therefore can't be reclaimed by the GC? Might deserve a quick note.

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8682729 [details] [diff] [review]
display WeakSet and WeakMap contents in console

(Sorry to revive from the dead!)

Comment 12

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Updated to follow bug 1221292.

Comment 13

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Comment 14

[Tom Tromey :tromey]

Comment on attachment 8686254 [details] [diff] [review]
display WeakSet and WeakMap contents in console

Updated per initial review, and to follow bug 1221292.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8686254 [details] [diff] [review]
display WeakSet and WeakMap contents in console

r=me on the dom/base and dom/webidl changes.

Comment 16

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8686254 [details] [diff] [review]
display WeakSet and WeakMap contents in console

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

Looks great!

Comment 17

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec5f17b177dc

Comment 18

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Add r= to commit message.

Comment 19

[Tom Tromey :tromey]

Naturally try points out that the test assumes a particular ordering of keys.
Whoops.  I will figure out what to do about the test.

Comment 20

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Update tests to allow "out-of-order" results.

Comment 21

[Tom Tromey :tromey]

Comment on attachment 8687267 [details] [diff] [review]
display WeakSet and WeakMap contents in console

I changed the tests to allow the results to appear in any order.
I wasn't sure if this was big enough to warrant re-review, so
erring on the side of caution.

Comment 22

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c15d8b012ac

Comment 23

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

Typo in one of the expected results.

Comment 24

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b67a145d508f

Comment 25

[Tom Tromey :tromey]

Attachment: display WeakSet and WeakMap contents in console

A silly slip of the keys snuck in.
Someday this will make it through.

One thing that occurred to me that is odd about this patch is that if
a user prints a WeakSet on the console, then that will keep the
elements alive.  This may be unexpected.

Comment 26

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98278885a078

Comment 27

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4e0d0bf80b36

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4e0d0bf80b36