Closed Bug 903873 Opened 7 years ago Closed 6 years ago
Convert Web Activities to Web
No description provided.
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.
> 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. :(
(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. :-)
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. ;)
(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?
> 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].
Thanks for the great explanation, filed bug 985812. Just as a clarification, do you think we should wait on that bug before landing this?
I don't think we should, no. What you have here is fine at first glance; I'll read more carefully tomorrow.
Comment on attachment 8393885 [details] [diff] [review] Finish porting Web Activities to WebIDL; r=bzbarsky r=me
Attachment #8393885 - Flags: review?(bzbarsky) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.