1395421 - Add a notification when get() is called on a JS-implemented maplike

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This is needed in bug 1393306.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Don't codegen JS-implemented-webidl glue for methods that use maplike/setlike/iterable codegen (and hence wouldn't call into that clue anyway)

MozReview-Commit-ID: 6voihCBh6wh

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. When a get() happens on a JS-implemented maplike, notify the JS implementation so it can take some sort of action (e.g. logging or warning)

MozReview-Commit-ID: 9G115wOyzvm

Comment 3

[Boris Zbarsky [:bzbarsky]]

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

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 8902992 [details] [diff] [review]
part 1.  Don't codegen JS-implemented-webidl glue for methods that use maplike/setlike/iterable codegen (and hence wouldn't call into that clue anyway)

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

::: dom/bindings/Codegen.py
@@ +15919,5 @@
>                     for a in attrs if not a.readonly]
>          methods = [m for m in iface.members
> +                   if (m.isMethod() and not m.isStatic() and
> +                       not m.isIdentifierLess() and
> +                       not m.isMaplikeOrSetlikeOrIterableMethod())]

So I think maplike is allowed on callback interfaces according to spec. Not sure that makes sense, but if so should we limit this to iface.isJSImplemented()?

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 8902993 [details] [diff] [review]
part 2.  When a get() happens on a JS-implemented maplike, notify the JS implementation so it can take some sort of action (e.g. logging or warning)

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

::: dom/bindings/Codegen.py
@@ +15953,1 @@
>          if any(m.isAttr() or m.isMethod() for m in iface.members) or (iface.isJSImplemented() and iface.ctor()):

This might not be true even though needOnGetId is? Might make more sense to just fill idList with the additional ids instead of using these booleans and then append initIdsClassMethod if idList is not empty?

@@ +16483,5 @@
> +    happening on a JS-implemented maplike.  This method takes two arguments
> +    (key and value) and returns nothing.
> +    """
> +    def __init__(self, descriptor):
> +        CallbackOperationBase.__init__(

I sort of expected this to be optional, but it'll throw if __onget is missing, right? I guess we'd need to change GetCallableProperty to make that not throw. Not sure it's worth it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> but if so should we limit this to iface.isJSImplemented()?

Yeah, probably we should.  Done.

> This might not be true even though needOnGetId is? 

Oh, good catch.  I should have used my two new booleans, but I like your suggestion more.  Will do that.

> I sort of expected this to be optional, but it'll throw if __onget is missing, right?

Yes.  In practice, we have precisely one non-test JS-implemented maplike and that one is the one that wants __onget, so this is OK, I think.

> I guess we'd need to change GetCallableProperty to make that not throw. Not sure it's worth it.

Exactly my reasoning.  ;)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/196ede21eb28
part 1.  Don't codegen JS-implemented-webidl glue for methods that use maplike/setlike/iterable codegen (and hence wouldn't call into that clue anyway).  r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/dad6d3dd7404
part 2.  When a get() happens on a JS-implemented maplike, notify the JS implementation so it can take some sort of action (e.g. logging or warning).  r=peterv

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/196ede21eb28
https://hg.mozilla.org/mozilla-central/rev/dad6d3dd7404