1047848 - PerformanceResourceTiming objects can't be JSON.stringify'd

Status: RESOLVED FIXED

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

Description

[Adam Roben]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_4) AppleWebKit/537.77.4 (KHTML, like Gecko) Version/7.0.5 Safari/537.77.4

Steps to reproduce:

JSON.stringify(performance.getEntriesByType("resource")[0])

or

Object.keys(performance.getEntriesByType("resource")[0])


Actual results:

The first call returns "{}".

The second call returns an empty array.

(This happens both in Firefox 31 and a nightly v34 build.)


Expected results:

The first call should return something like:

"{\"initiatorType\":\"link\","redirectStart":141.739439,\"redirectEnd\":141.739439,\"fetchStart\":141.739439,\"domainLookupStart\":141.739439,\"domainLookupEnd\":141.739439,\"connectStart\":141.739439,\"connectEnd\":141.739439,\"secureConnectionStart\":0,\"requestStart\":257.34999400000004,\"responseStart\":276.781895,\"responseEnd\":339.190355,\"name\":\"http://www.example.com/foo.css\",\"entryType\":\"resource\",\"startTime\":141.739439,\"duration\":197.450916}"

The second call should return something like:

["initiatorType","redirectStart","redirectEnd","fetchStart","domainLookupStart","domainLookupEnd","connectStart","connectEnd","secureConnectionStart","requestStart","responseStart","responseEnd","name","entryType","startTime","duration"]

Chrome and IE both return these non-empty results.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Adam, thanks for filing this.

> The first call returns "{}".

This is what the spec currently requires, yes.

> The second call returns an empty array.

Right, because the properties are on the prototype, again as the spec requires.

> Chrome and IE both return these non-empty results.

This is a known spec violation on Chrome's part: it puts the properties on the wrong object.

The claim about IE is wrong, though: IE _does_ put them on the right object.  http://jsfiddle.net/R6KK7/1/ shows that there are no own properties on the PerformanceResourceTiming object.

What IE does is add a non-standard toJSON method which makes JSON.stringify serialize non-own properties in this case.

That said, chances are the spec should just change to specify a toJSON method, since that's a reasonable thing to do with PerformanceResourceTiming objects.  That will make JSON.stringify work like it does in IE.  I raised a spec issue at http://lists.w3.org/Archives/Public/public-web-perf/2014Aug/0000.html but don't hold much hope for it getting anywhere given the past behavior of that working group. :(

So we should probably just ignore the spec here and add a jsonifier to this interface.  Valentin, are you willing to do that?

Also, is there a bug on enabling this API by default that this bug should block?

Comment 2

[Adam Roben]

Thanks for clarifying the underlying issue, Boris, and for raising it with the WG! JSON.stringify is indeed the important issue, as you deduced. I'm sending resource timings to GitHub's servers for performance tracking.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

(In reply to On vacation Aug 5-18.  Please do not request review. from comment #1)
> So we should probably just ignore the spec here and add a jsonifier to this
> interface.  Valentin, are you willing to do that?

Yes, I think adding a jsonifier to the interface is a fair thing to do.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: PerformanceResourceTiming objects can't be JSON.stringify'd

Adding a jsonifier per comment 3

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8467505 [details] [diff] [review]
PerformanceResourceTiming objects can't be JSON.stringify'd

And a test, please?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Yes, we definitely want a test here.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8467505 [details] [diff] [review]
PerformanceResourceTiming objects can't be JSON.stringify'd

Clearing the review request until we have a test.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Yes, we definitely want a test here.

Quick question: if I add a jsonifier to both PerformanceEntry and PerformanceResourceTiming (which extends PerformanceEntry), then serialization a PerformanceResourceTiming should also contain the attributes of PerformanceEntry, right?
This isn't what I'm seeing in the test. Is there a catch?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

I would expect both things to end up being json-fied yes.
I assume both IE and Chrome put properties from both interfaces to the object?
(Well, in Chrome they are there implicitly because of the wrong property handling.)

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Our jsonfier doesn't seem to quite do what the spec's serializer requires.
Sounds like we need serializers. It supports 'inherit'.
http://heycam.github.io/webidl/#idl-serializers

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

(But we might be able to just hack the inheritance support to current jsonfier without full
serializer support.)

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

If a quick look to the codegen is right, somewhere around
http://hg.mozilla.org/mozilla-central/annotate/7f81be7db528/dom/bindings/Codegen.py#l7240
we could check if the ancestor interfaces have jsonfier, and if so, jsonfy also their attributes.

Comment 13

[Boris Zbarsky [:bzbarsky]]

We have no plans to support the Web IDL serializer syntax.  We should do what comment 12 says.

Comment 14

[Valentin Gosu [:valentin] (he/him)]

I'll try to change Codegen.py to serialize the parent attributes as well.

Comment 15

[Valentin Gosu [:valentin] (he/him)]

Attachment: Part 1 - Change Codegen.py to jsonify parent interface attributes as well

Comment 16

[Valentin Gosu [:valentin] (he/him)]

Attachment: Part 2 - PerformanceResourceTiming objects can't be JSON.stringify'd

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8468289 [details] [diff] [review]
Part 1 - Change Codegen.py to jsonify parent interface attributes as well

I'd prefer to have a check that the interface actually has jsonifier.


(It might be nice to have a setup where we don't create similar code several times when child interface needs to jsonify parent too. It could call something in the parent. But this can be done in a followup.)

Comment 18

[Valentin Gosu [:valentin] (he/him)]

Attachment: Part 1 - Change Codegen.py to jsonify parent interface attributes as well

I added a check for the jsonifier for each parent interface

Comment 19

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0556ee388f90
https://hg.mozilla.org/integration/mozilla-inbound/rev/27dd0839f6d2

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0556ee388f90
https://hg.mozilla.org/mozilla-central/rev/27dd0839f6d2