SYNTAX_ERR exception for markName argument

[plehegar]

From https://lists.w3.org/Archives/Public/public-web-perf/2015Jan/0070.html
[[
In the Exceptions section of the User Timing spec for the mark method, it
states:

"Throws a SYNTAX_ERR exception if the markName argument is the same name as
an attribute in the PerformanceTiming interface."

In Gecko we implemented bug 760851 [1] out-of-spec and added toJSON to
the PerformanceTiming interface to get a JSON version of the timing
attributes. Following this reasoning, then toJSON being a
PerformanceTiming attribute may need to throw a SYNTAX_ERR when used as a
performance marker name:

performance.mark('toJSON');

So we need some clarification of the spec to determine whether all
attributes of the PerformanceTiming interface should throw if used as
marker names, or only those specifically defined by the spec.
[1]
https://bugzilla.mozilla.org/show_bug.cgi?id=760851

]]

[igrigorik]

I believe we discussed this on one of the conf calls and the conclusion was that we should allow any/all metric names. Any objections? /cc @toddreifsteck

[toddreifsteck]

I'm unsure where this bit of the spec originated but I just tested Chrome and IE. Both allow name, duration, startTime, entryType to be used. I believe we should cut this from the spec. Should we follow up with Firefox? I did not check their prerelease builds of Gecko.

[igrigorik]

/cc @mina @marcoscaceres

[plehegar]

@toddreifsteck do you/we have a test somewhere that I can run on firefox?

[plehegar]

We have a test at:
http://w3c-test.org/user-timing/test_user_timing_mark_exceptions.html
if you add the other methods, like toJSON, duration, entryType, or startTime, those will be allowed in FF and Chrome.

[marcoscaceres]

Sorry, I've not been following this spec or it's implementation in Gecko at all :( I don't know if I can be much here.

[plehegar]

So, it looks like we got mixed up in measure() and the fix would impact this issue:

1. we store DOMHighResStamp, ie PerformanceMeasure will return DOMHighResStamp values.
   https://w3c.github.io/user-timing/#performancemeasure
2. if startMark or endMark use is one of the attributes in the PerformanceTiming interface, "the value of that attribute is used as the end DOMHighResTimeStamp time value"
   https://w3c.github.io/user-timing/#h-dom-performance-measure

However, PerformanceTiming doesn't contain DOMHighResTimeStamp values:
http://www.w3.org/TR/navigation-timing-2/#the-performancetiming-interface

imho, this should be changed to refer to PerformanceNavigationTiming attributes instead.

Now, if we do that, can one store the mark "prerenderSwitch" or "nextHopProtocol"?

[plehegar]

Btw, if the answer to my last question is "no, you should raise a SyntaxError instead", then what should we do with performance.measure("myMeasure", "prerenderSwitch", "nextHopProtocol") ?

[igrigorik]

Btw, if the answer to my last question is "no, you should raise a SyntaxError instead", then what should we do with performance.measure("myMeasure", "prerenderSwitch", "nextHopProtocol") ?

Ugh, that's messy. Related, what about measure("domInteractive", ...)

Wondering if we should consider option (3): remove that capability entirely, since it's just syntax sugar for measure('thing', performance.timing.x, performance.timing.y)?

[plehegar]

"domInteractive" is ok as far as I know. We don't raise SYNTAX_ERR on the measureName.
We could indeed change the function prototype from
measure(DOMString, DOMString, DOMString)
to
measure(DOMString, DOMHighResTimeStamp, DOMHighResTimeStamp)
or
measure(DOMString, (DOMString | DOMHighResTimeStamp), (DOMString | DOMHighResTimeStamp))

However, would we break anyone if we remove
measure('thing', "responseStart", "responseEnd") ?

[igrigorik]

Ah, right, apologies about the confusion.

• measure(name) should be measured against time origin - i.e. "from navigationStart to the current time" is incorrect #11.
• On the one hand I understand the desire to have the convenience of referencing PerformanceNavigationTiming attributes, but on the other it creates a bunch of complications (current and moving forward)
  • we have PerformanceNavigationTiming that are not timestamps
  • assuming we unpack redirects into separate entries in the future, there won't be just a single PerformanceNavigationTiming entry; we'd reference the last one? =/
  • there is no PerformanceNavigationTiming in worker

That said, I'm not sure if/what we'd break if we were to remove that functionality at this point.

If we want to keep it backwards compatible, perhaps we should explicitly list the names of "special markNames" that would only be applicable in Window context?

[plehegar]

Note that #13 doesn't solve this issue.

[igrigorik]

After re-reading this thread, my proposal is...

If the global object [HTML51] is a Window object, and markName uses the same name as a read only attribute (excluding toJSON) in PerformanceTiming interface, throw a SyntaxError.

• Limit above behavior to Window contexts; PerformanceTiming doesn't exist for Workers.
• Keep the exceptions scoped to PerformanceTiming (i.e. do not update it to PerformanceNavigationTiming). My hunch is that the whole exception business is not something we would do if we were writing the spec today, but for historical reasons I think we need to maintain this behavior; let's freeze the list of attributes on NT1 and move on.

Thoughts / objections?

[toddreifsteck]

Discussed with Ilya/Yoav. The original purpose of this exception is to protect measure string lookups on nav timing 1 from being corrupted by user timings that are injected.

performance.measure('foo','navigationStart','fetchStart')

A simple spec update to limit the attributes protected to only include the readonly attributes will mean that the following does not need to throw for the spec to be followed:

performance.mark('toJSON')