Re: [spec-reviews] Frame Timing API (#56)

So I read through the explainer and the specification (the editor's draft link above), although I haven't looked into the history of the mailing list discussion that led to it, so I'm sure I'm missing a good bit of background.

The thoughts I have are below.  Probably I should eventually file them as issues on the specification in some form, but I figured I'd share them here first.  They probably cross three main areas:
1. general architectural issues (where I have a few comments but am reasonably happy)
2. level of detail in the specification (where it seems rather underspecified in some significant areas)
3. minor nits
although not everything fits precisely into one of those buckets.

# General comments

- This specification seems somewhat specific to the way browsers are built today, which is not the same as the way they were built 5 years ago, and may well not match the way they're built 5 years in the future.  In general, building that sort of architectural dependency into a specification would worry me quite a bit, but I'm substantially less worried here because this specification is a set of performance tools.  If browser architecture changes, those tools may become less useful (or even completely useless), but they seem unlikely to cause Web content to fail to function or to constrain architectural decisions in order to keep this API operating.
  - That said, I think the specification should probably say a little bit more about what an implementation should do if it doesn't use exactly the model that we currently have with one rendering thread and one compositor thread.  It might be an interesting exercise to see if the spec's definitions make sense either with compositing happening on the same thread, or with a pipeline that passes data across more than two threads before it gets rendered.  I think this could probably be addressed as part of making clearer definitions of startTime and duration in sections 4.3 and 4.4.

# 1. Introduction

- "JavaScript provides a mechanism" is misleading given that requestAnimationFrame isn't part of JavaScript itself; maybe something like "can use" instead of "provides" would be clearer
- requestAnimationFrame is also bad because it means more wakeups when there otherwise wouldn't have been one.  I'm not sure if that's worth mentioning, although it might be given the extensive discussion of requestAnimationFrame.  Then again, that extended discussion is perhaps more discussion of requestAnimationFrame than belongs in a spec that's intended to avoid needing to use it as a measurement hack.

# 4.2 Extension to the Performance interface

- Is the wacky indentation of the onframetimingbufferfull line intended?

# 4.2.1 Attributes

- Not clear what it means for the frametimingbufferfull event to bubble; I don't think the Performance object is participating in a tree per the definitions in https://dom.spec.whatwg.org/#dispatching-events and https://dom.spec.whatwg.org/#concept-tree-participate

# 4.2.2 Methods

- The description of how the buffer operates isn't clear.  Is the intent is that by default there is no buffer (or a zero-size buffer) unless setFrameTimingBufferSize is called (so that there isn't recording overhead unless such recording has been requested).  The behavior should be explicitly specified (perhaps by reference if an appropriate reference exists).
- Along similar lines, the specification should say what happens when the buffer fills up:  are old data overwritten, or is recording stopped?  (My intuition would be that stopping recording is better because then you wouldn't get stuck with the overhead of recording forever.)
- The specification should describe what state is cleared by clearFrameTimings().  For example:
  - I would assume that if a PerformanceRenderTiming or PerformanceCompositeTiming has not been returned from any of the methods in http://www.w3.org/TR/performance-timeline/#sec-window.performance-attribute that any data that would be returned is cleared.  But if this is correct, it should still be stated explicitly.
  - If data from the "buffer" has been used as part of the return value of one of those methods, is the object returned modified in any way, or does it retain all its original state?
    - I would assume that after being cleared, it would no longer be returned from getEntries(), etc.  But, again, if this is the case, it should be stated explicitly.
- The description of the methods refers only to PerformanceRenderTiming resources, whereas the definition of the attributes refers to both PerformanceRenderTiming and PerformanceCompositeTiming.  Was it intended that the methods refer to both as well?

# 4.3 (PerformanceRenderTiming) and 4.4 (PerformanceCompositeTiming)

- The descriptions of the name attribute seem unnessarily different; the one in 4.3 seems much clearer.
- The definition of sourceFrameNumber in 4.3 doesn't define "source frame number", and the one in 4.4 is different but similarly vague.  Is it just an incrementing counter?  What makes them correlate between the two types of events?  (When does it increment if things like animations or scrolling are happening entirely on the compositor?)
- Should the time stamps be defined to be related to Performance.now() in some way, or to time stamps in some other specification?  They currently appear to be allowed to be pretty much arbitrary as long as they follow the rules in section 5 (Monotonic Clock).

# 4.3 (PerformanceRenderTiming)

- The description of duration in 4.3 seems to contradict the explainer, in that the explainer says that you can tell how far under your frame budget you are, whereas this definition of duration reports the actual time between frames.

# 4.4 (PerformanceCompositeTiming)

- The definition of startTime doesn't actually define anything; it should say what time is actually used!
- The definition of duration as 0 seems like it might be throwing away information about how long the composite takes that would be of substantial use to authors.  Is that really intended?

# 5 (Monotonic Clock)

- As mentioned above, it seems a little disturbing that this needs to be redefined here, given that it seems like users of this specification will want to compare time stamps from different sources, so it seems like referencing a common definition of what those timestamps mean might be more useful.  Or is the intent that they not be comparable to time stamps from other specifications?  If so, that should be explicitly stated.

# 6 (Privacy and Security)

- I think I agree with the conclusion here, but I find the explanation a little bit confusing.  Maybe it's worth mentioning that much (if not all?) of the timing information that this exposes can already be estimated by script in various (though inefficient) ways?

---
Reply to this email directly or view it on GitHub:
https://github.com/w3ctag/spec-reviews/issues/56#issuecomment-127802871