notifications icon indicating copy to clipboard operation
notifications copied to clipboard

Custom data object [SameObject] extended attribute

Open robertbindar opened this issue 11 years ago • 15 comments

According to the WEBIDL spec [1] we should have an interface type for the custom data object when used with the [SameObject] attribute [2].

[1] http://heycam.github.io/webidl/#SameObject [2] notifications.spec.whatwg.org/#api

robertbindar avatar Jul 28 '14 21:07 robertbindar

@bzbarsky @heycam it would be nice to use that here. I guess the alternative is detailing in prose that the structured clone is only made once so that the same object is returned each time. Any thoughts?

annevk avatar Jul 29 '14 07:07 annevk

I wonder if [SameObject] should just become [Constant], as Gecko's bindings supports. Then we could use it for any type (including the any type!).

heycam avatar Jul 29 '14 07:07 heycam

Works I think. You might want to review usage in http://dom.spec.whatwg.org/ to be sure.

annevk avatar Jul 29 '14 07:07 annevk

In section 5.4, I think:

The data attribute must return a structured clone of notification's data.

should be just:

The data attribute must return the notification's data.

since the constructor already handles the initial assignment of the structured clone to data.

heycam avatar Jul 29 '14 07:07 heycam

No, the "notification" is shared across workers and documents. Notification objects are per-document, per-worker, etc. so need their own copy.

annevk avatar Jul 29 '14 08:07 annevk

Oh I see, the "notification" is the abstract thing that one or more Notification object get created to represent.

Still I don't think there is any need to add any additional wording about making the structured clone once, even if we don't use [SameObject] or [Constant] here, since the step in the constructor only runs once.

heycam avatar Jul 29 '14 08:07 heycam

But otherwise each time you get the data property the getter will run which creates a structured clone of the abstract thing.

annevk avatar Jul 29 '14 08:07 annevk

OK. It's not exactly clear whether "must return a structured clone of notification's data" should mean a new structured clone every time you fetch the IDL attribute or just any structured clone (which therefore could be the same one). With the [SameObject] or [Constant] it would force you to interpret it as "only do the structured clone once" I guess.

heycam avatar Jul 29 '14 12:07 heycam

Yeah that was the idea. Otherwise I'd have to store an object alongside the Notification object that's created the first time you get the data property and then returned on subsequent gets.

annevk avatar Jul 29 '14 12:07 annevk

[SameObject]/[Constant] doesn't do what it sounds like people think it does.

It's just a promise that the implementation will return the same thing each time, for ease of reading/understanding the API. It doesn't actually enforce this in any way.

It sounds like what you're asking for here is more like Gecko's [Cached], which says that after the first time the getter or method is called the result is cached by the Web IDL layer and returned for subsequent calls.

I suppose we could make [SameObject] or [Constant] imply the equivalent of [Cached] at least in spec terms. That would be OK by me; we'd have to clearly define it by saying that these extended attributes lead to the creation of a corresponding internal slot and the exact behavior of that external slot. Fwiw, Gecko's behavior is that if the value in the internal is not JS undefined then it's returned, otherwise the getter/method is called and the return value stored in the internal slot before returning. This does mean that if a method is declared as returning any and actually returns undefined we'll keep calling the method, but in practice no one does that.

bzbarsky avatar Jul 29 '14 12:07 bzbarsky

Technically it's a spec bug if the spec author uses [SameObject] and then requires in prose that a different object is returned at some point. So it is an "enforcement" as long as the spec has no bugs!

heycam avatar Jul 29 '14 12:07 heycam

I see. I would prefer having [Cached] then, indeed. If [SameObject] does not actually help in writing text, I'm not sure why we would have it.

annevk avatar Jul 29 '14 12:07 annevk

https://www.w3.org/Bugs/Public/show_bug.cgi?id=26524

annevk avatar Aug 05 '14 13:08 annevk

By the way, I guess this is working as expected?:

// Prerequisite: a single notification with data is currently showing.
Promises.all([
    serviceWorkerRegistration.getNotifications().then(function(ns) { return ns[0]; }),
    serviceWorkerRegistration.getNotifications().then(function(ns) { return ns[0]; })
]).then(function(notification1, notification2) {
    // notification1 !== notification2
    // notification1.data !== notification2.data
})

It's a little odd that you get different instances of the SameObject for different references to the same underlying notification, but I suppose that's normal for WebIDL...

johnmellor avatar Aug 12 '15 19:08 johnmellor

IDL describes instances and [SameObject] only pertains to an instance. It knows nothing about underlying objects. So yes, that is working as expected.

annevk avatar Aug 13 '15 05:08 annevk