903873 - Convert Web Activities to WebIDL

Status: RESOLVED FIXED

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

Comment 1

[(no longer active)]

Attachment: Finish porting Web Activities to WebIDL; r=bzbarsky

This patch ports over the ActivityRequestHandler interface.

Unfortunately the original design of the interface specifies that
ActivityRequestHandler.source should be of type ActivityOptions which is
a dictionary.  This is not expressible in WebIDL, so for now we're just
using the type 'object' for that.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> This is not expressible in WebIDL

It depends.  It looks like the current API is just badly designed in that it returns a new object every time you call the getter.  Are we OK with returning the same object every time the getter is called?  I guess that should be a followup bug.  :(

Comment 3

[(no longer active)]

(In reply to comment #2)
> > This is not expressible in WebIDL
> 
> It depends.  It looks like the current API is just badly designed in that it
> returns a new object every time you call the getter.  Are we OK with returning
> the same object every time the getter is called?  I guess that should be a
> followup bug.  :(

A quick look at the gaia code seems to suggest that the existing callers do not care about the object identity.  If you want, I can file  follow-up, but I'm not sure what I would put into it.  :-)

Comment 4

[Boris Zbarsky [:bzbarsky]]

That this should be a dictionary type in the IDL and should be a [Cached, Pure] property.  And ask the people who own the code whether it should be [Frozen] too.  ;)

Comment 5

[(no longer active)]

(In reply to comment #4)
> That this should be a dictionary type in the IDL and should be a [Cached, Pure]
> property.  And ask the people who own the code whether it should be [Frozen]
> too.  ;)

Wait, out of curiosity, can [Cached, Pure] properties be a dictionary?  If yes, do you mind explaining the rationale please?

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Wait, out of curiosity, can [Cached, Pure] properties be a dictionary?

Yes.

The reason WebIDL doesn't allow dictionary attributes is that converting a WebIDL dictionary to JS creates a new object each time.  So allowing them would mean that people would create APIs like this one, where each time you get the property you get a new object, which is viewed as an antipattern.

Sequences have the same issue, which is why WebIDL does not allow sequence-valued attributes.  But as Jonas pointed out in https://www.w3.org/Bugs/Public/show_bug.cgi?id=23682 its reasonable to want attributes whose value is the same array (or object, for the dictionary case) every time.  You can already do it if you declare the attribute as an "object" and then manually cache the value in the callee, but that's moderately annoying.

What our [Cached] extension to WebIDL implements is that the binding itself caches the value.  So the underlying getter is called once, then the binding keeps returning the same object without calling the getter.

Obviously this is only OK for cases when the getter has no side-effects, which is why we only allow [Cached] on attributes that are [Pure] or [Constant].

Comment 7

[(no longer active)]

Thanks for the great explanation, filed bug 985812.

Just as a clarification, do you think we should wait on that bug before landing this?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't think we should, no.  What you have here is fine at first glance; I'll read more carefully tomorrow.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8393885 [details] [diff] [review]
Finish porting Web Activities to WebIDL; r=bzbarsky

r=me

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8613d9d677d

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f8613d9d677d