#326: Element Timing API for images

Visit on Github.

Opened Nov 16, 2018

こんにちはTAG!

I'm requesting a TAG review of:

Further details (optional):

  • Relevant time constraints or deadlines: Chrome has implemented this feature but we haven't fleshed out the spec yet. We'd probably like to launch early 2019.
  • We have a Chrome launch bug (sorry, Googlers only), which received privacy and security approvals.

Security and Privacy Questionnaire: https://docs.google.com/document/d/15T51ya-7GCtDQGF_ukGAPXBkygI6WFZBV9e9Bc8mjic/edit

You should also know that...

The current proposal is for ElementTiming for <img> elements, but we received feedback at TPAC that including background images in our first iteration could also be very helpful. So we may add that. Later versions of this API would like to expose other elements, but for most we will need to think about text rendering timing. We are deferring that to a future version of this API.

We'd prefer the TAG provide feedback as (please select one):

  • leave review feedback as a comment in this issue and @-notify @npm1 and @tdresser

Discussions

Comment by @cynthia Feb 6, 2019 (See Github)

It's a bit difficult to judge whether or not this is safe based on just the fact that it was "approved" under the privacy and security review process within Google. Could you fill out the S&P questionnaire or at least share us the review report at the very least?

Comment by @travisleithead Feb 6, 2019 (See Github)

Hey thanks for the review request! Some initial thoughts:

  • I assume only elements that belong to the top-level browsing context are considered for auto-inclusion in the timings? E.g., many ad frameworks manipulate iframes to fill considerable space in the viewport and which oclude other content. Since iframes are not being considered (at this point) in this spec (just img elements and maybe elements with background images), this is additional variability that would potentially decrease reporting value.
  • The auto-inclusion seems nice, but will add a tremendous amount of variability to the feature that seems like it will be challenging to rationalize and get interop across user agents--thus if an origin wants to compare data across user agents, the correlation may be difficult, leading to uncertainty and lack of confidence in the feature.
  • 'elementtiming' as an attribute on an element seems quite redundant. Perhaps something a bit shorter like 'timingname' or 'timingid'?
  • Would like additional clarity on the cross-origin availability of this data. E.g., is it proposed that all "hero" timings (including the auto-added items) be available in the performance entry lists for all browsing contexts or just the top browsing context?
Comment by @travisleithead Feb 6, 2019 (See Github)

Some additional thoughts:

  • We suppose that the elementtiming attribute, if present in a closed ShadowRoot, would still be included in the Performance timeline? This might break an invarient of the closed ShadowDOM principles?
Comment by @cynthia Feb 6, 2019 (See Github)

This would be a interesting feature for larger scale projects (with disciplined development practices and a strict review process), but feels like (in the average case) this is a property that could easily be "leaked" out into the wild unwillingly by busy developers on a tight schedule. Given that there is potential that this could also leak out in templates/libraries/components, it feels like there should be a mechanism that acts as a global off switch (equivalent of #ifdef DEBUG 0) so that you don't have to go in and patch up a bunch of third party code.

Comment by @npm1 Feb 8, 2019 (See Github)

Replying to the comments:

It's a bit difficult to judge whether or not this is safe based on just the fact that it was "approved" under the privacy and security review process within Google. Could you fill out the S&P questionnaire or at least share us the review report at the very least?

Sure! Here it is https://docs.google.com/document/d/15T51ya-7GCtDQGF_ukGAPXBkygI6WFZBV9e9Bc8mjic/edit

  • I assume only elements that belong to the top-level browsing context are considered for auto-inclusion in the timings? E.g., many ad frameworks manipulate iframes to fill considerable space in the viewport and which oclude other content. Since iframes are not being considered (at this point) in this spec (just img elements and maybe elements with background images), this is additional variability that would potentially decrease reporting value.

Yes, only resources in the current browsing context are measured. An iframe can still measure its own resources though (every window has its own performance object). And the entry creation will not be based strictly on visibility, so elements occluded by transparent iframes would still be reported.

  • The auto-inclusion seems nice, but will add a tremendous amount of variability to the feature that seems like it will be challenging to rationalize and get interop across user agents--thus if an origin wants to compare data across user agents, the correlation may be difficult, leading to uncertainty and lack of confidence in the feature.

We’re thinking of adding some way to identify the element due to this problem (and to help make the API more actionable), for example the URL. Do you think that would help with this concern? Note that entries that are auto-included can be distinguished from entries that are registered via the ‘name’ attribute.

  • 'elementtiming' as an attribute on an element seems quite redundant. Perhaps something a bit shorter like 'timingname' or 'timingid'?

Yea we could go with those, but we wanted to choose something that would not collide with other usage. We’ll discuss these options with the WebPerf WG.

  • Would like additional clarity on the cross-origin availability of this data. E.g., is it proposed that all "hero" timings (including the auto-added items) be available in the performance entry lists for all browsing contexts or just the top browsing context?

Hero timings will be available in the browsing context that contains the element. If a top-level browsing context contains an <img>, then the entry will show up there. If this context has an iframe which contains another <img>, then the entry for this image will be available only in the iframe. This is consistent with how it is handled in ResourceTiming.

  • We suppose that the elementtiming attribute, if present in a closed ShadowRoot, would still be included in the Performance timeline? This might break an invarient of the closed ShadowDOM principles?

I agree that a closed ShadowRoot should not report any timing information to DOM trees containing it.

This would be a interesting feature for larger scale projects (with disciplined development practices and a strict review process), but feels like (in the average case) this is a property that could easily be "leaked" out into the wild unwillingly by busy developers on a tight schedule. Given that there is potential that this could also leak out in templates/libraries/components, it feels like there should be a mechanism that acts as a global off switch (equivalent of #ifdef DEBUG 0) so that you don't have to go in and patch up a bunch of third party code.

Not sure I understand what you mean by ‘leak’ here? Javascript should intentionally query for entries or use PerformanceObserver in order to obtain the entries.

Comment by @npm1 Feb 8, 2019 (See Github)

Oops forgot to mention, we recently decided a modification for reporting the timing. I modified the explainer Google Doc but here is a summary of the change:

  • For images that pass the timing allow check (same origin or with Timin-Allow-Origin headers), report the load time, display time (after image has loaded), and calculate the intersectionRect when the image is displayed.
  • For images that do not pass the timing allow check, report only the load time (not the display time). In this case, we compute the intersectionRect when image has loaded so that we can deliver the entry at this time. Otherwise we'd have to wait until display time to compute the rect and deliver the entry and we'd leak some of this display timing information unintentionally.
Comment by @cynthia Feb 9, 2019 (See Github)

Not sure I understand what you mean by ‘leak’ here? Javascript should intentionally query for entries or use PerformanceObserver in order to obtain the entries.

Leak was a poor choice of words. The concern is the potential overhead of having this enabled on a element, and libraries/components sloppily being released in the equivalent of debug mode. It feels like the top level should be able to flip the switch off to reduce overhead.

Comment by @npm1 Feb 11, 2019 (See Github)

Leak was a poor choice of words. The concern is the potential overhead of having this enabled on a element, and libraries/components sloppily being released in the equivalent of debug mode. It feels like the top level should be able to flip the switch off to reduce overhead.

Ah, so this is a performance concern? In a scenario where a website has a lot of components which spam elements containing the elementtiming attribute, I would expect the large DOM size to by far outweigh the performance cost of elementtiming. That is, all else being equal, I suspect that having/not having the elementtiming attribute in this situation won't cause significant performance differences. I could run an experiment to see if that's the case, assuming that I understood your concern correctly?

Comment by @npm1 Mar 19, 2019 (See Github)

I'd like to have TAG review for Element Timing for text as well (see explainer). Should I file a new TAG review or can it be done in this one?

Comment by @npm1 Apr 26, 2019 (See Github)

Ping, I have not heard an update from this review in months.

Also want to update: regarding closed shadow DOM, I would say that such elements should still be exposed because they also affect the performance of a website. The need to measure performance is separate from the desire to hide tree structure information within components.

Comment by @annevk May 17, 2019 (See Github)

FWIW, as I noted elsewhere, it's not acceptable to include shadow trees (of any kind) without a much wider discussion on w3c/webcomponents. We have a couple specific holes for open shadow roots, but they don't generalize to any new feature.

Comment by @npm1 May 17, 2019 (See Github)

Yes, I submitted the issues to clarify what is acceptable. For now, for the purpose of this review, we can assume shadow elements are not exposed. https://github.com/WICG/element-timing/issues/3 https://github.com/w3c/webcomponents/issues/816

Comment by @annevk May 20, 2019 (See Github)

To be clear, exposing the mere existence of a shadow tree is considered an encapsulation violation.

Comment by @dbaron May 22, 2019 (See Github)

@kenchris and I are looking at this in a breakout in the TAG face-to-face in Reykjavík.

I think we're both OK with using this review to cover both images and text -- though based on a quick skim it seems like text is currently covered only in the explainer but not in the text. Is that correct? That said, it seems like it might also make sense to separate if they're going to progress at substantially different rates. It seems like the text parts probably need to be developed a good bit more before we can really review them, since it's not yet clear what they are.

A few thoughts:

  • one thing I'm a little concerned about is implicit registration. It seems like it might have substantial performance overhead since finding out whether something should be implicitly registered seems like it would happen relatively late in the loading process, which would therefore require everything to be tracked up until that point, which might have a bit of overhead. Is that the case? (I notice there's a "TODO: fix implicit registration.") in the explainer.)
  • does the API support vector images, or is it just raster images? It's a little unclear.
  • defining when images are fully loaded requires a little care for animated images. Is the concern about the first frame of an animated raster image?
  • there should probably be a formal-ish definition of the elementtiming attribute somewhere that is intended to be moved into the HTML spec eventually (and when that happens, this spec could link to it).
  • How much discussion has there been of the naming here? Is it better to call a bunch of different things "element" or to call image specific information "image", etc.?
  • This seems like it's tightly tied to the work that's done in the Web Performance Working Group -- to what extent has it been discussed with the other participants of that working group, and is there a plan to move it there? (There were some points above that you said you would discuss there.)
  • We're also curious what the level of urgency is -- are there plans to ship this soon? Are those plans separate for images versus text? (And if so, that also increases the urgency of bringing into the Web Performance WG and getting it discussed effectively there...)
Comment by @npm1 May 22, 2019 (See Github)

I think we're both OK with using this review to cover both images and text -- though based on a quick skim it seems like text is currently covered only in the explainer but not in the text. Is that correct?

That is true - text is currently covered in the explainer but not in the spec (I imagine that's what you meant).

That said, it seems like it might also make sense to separate if they're going to progress at substantially different rates. It seems like the text parts probably need to be developed a good bit more before we can really review them, since it's not yet clear what they are.

I'll try to speed up the process of adding text to the spec draft, I'm just a bit bogged down in work right now :)

  • one thing I'm a little concerned about is implicit registration. It seems like it might have substantial performance overhead since finding out whether something should be implicitly registered seems like it would happen relatively late in the loading process, which would therefore require everything to be tracked up until that point, which might have a bit of overhead. Is that the case? (I notice there's a "TODO: fix implicit registration.") in the explainer.)

Yep implicit registration is a bit of a problem right now, and we believe that the current approach (implicitly register elements that occupy a large portion of the viewport) is not good enough because many websites do not contain any element that occupies a significant portion of the viewport... The performance overhead concern is valid, but I do believe it would be very valuable to expose something here. The overhead would come from keeping track of all elements that have painted, and when the first paint occurs computing the intersectionRect and other attributes. I believe the overhead is acceptable, but can only know when we attempt to ship. An alternative would be to ship without any implicit registration - I'm hoping to avoid that.

  • does the API support vector images, or is it just raster images? It's a little unclear.

It supports both. As long as the it's in an <img>, <image> inside <svg>, or background-image.

  • defining when images are fully loaded requires a little care for animated images. Is the concern about the first frame of an animated raster image?

Not sure I follow this question... can you clarify?

  • there should probably be a formal-ish definition of the elementtiming attribute somewhere that is intended to be moved into the HTML spec eventually (and when that happens, this spec could link to it).

It is already defined in the DOM section

  • How much discussion has there been of the naming here? Is it better to call a bunch of different things "element" or to call image specific information "image", etc.?

Not a lot... I think it's plausible to have an entryType for image and one for text. Filed an issue

  • This seems like it's tightly tied to the work that's done in the Web Performance Working Group -- to what extent has it been discussed with the other participants of that working group, and is there a plan to move it there? (There were some points above that you said you would discuss there.)

This API has also been discussed with the group and there is consensus that it is useful. We do intend to move it from incubation into the group eventually. I'll take a note to followup on the promised discussion on the 'elementtiming' attribute name, as well as maybe the entryType issue.

  • We're also curious what the level of urgency is -- are there plans to ship this soon? Are those plans separate for images versus text? (And if so, that also increases the urgency of bringing into the Web Performance WG and getting it discussed effectively there...)

We'd like to ship in September. I'd like to point out: this has been presented to the group. I can share meeting notes with you if that would be helpful, but what other type of discussion are you envisioning? In any case, the spec is not ready, so I'll send a ping to members of the group from other browser vendors to provide feedback if they wish, once the spec is in better shape.

Comment by @kenchris May 23, 2019 (See Github)

Not sure I follow this question... can you clarify?

I believe David was referring to whether you track when the first frame loaded or when all the frames did (think .GIF file)

Comment by @ylafon May 23, 2019 (See Github)

It can also refer to progressive PNG as you have a first blurry paint before the complete image is loaded.

Comment by @ylafon May 23, 2019 (See Github)

Also could it be used as a way to detect resource blockers, like if the measured time is too short, it is likely that the original picture was replaced by a 1x1 transparent gif, or something like that. Is there a mitigation against this?

Comment by @kenchris May 23, 2019 (See Github)

Have you thought about rate limiting the feature? So that you cannot use it on each load, or by introducing some time before you can measure again?

Comment by @npm1 May 23, 2019 (See Github)

I believe David was referring to whether you track when the first frame loaded or when all the frames did (think .GIF file)

First paint after the image is loaded and fully decoded.

Also could it be used as a way to detect resource blockers, like if the measured time is too short, it is likely that the original picture was replaced by a 1x1 transparent gif, or something like that. Is there a mitigation against this?

A resource blocker returning a 1x1 GIF is easily detectable by looking at the intrinsic size of the image. And even if it returns the same size, the image onload can be used to time whether it is the trivial image or the expensive image. In other words, Element Timing is not needed to detect the resource blocker. The mitigation is that we return a rounded rendering timestamp (nearest 8ms).

Have you thought about rate limiting the feature? So that you cannot use it on each load, or by introducing some time before you can measure again?

No. As a developer, I would expect that an element with elementtiming attribute will be exposed always. Then there is the question of whether entries from implicit registration (right now, occupying a large portion of the viewport) are expensive to compute. For that, we need further investigation to determine whether or not this causes any performance problem. If it does, I'd prefer changing the criteria for implicit registration so that it is faster to compute, rather than rate limiting.

Discussed Jul 3, 2019 (See Github)

(And Event Timing API)

Dan: Was there a breakout..?

David: I was supposed to and I didn't :(

... I have been being productive and ignoring my to-do list.

Dan: Will you be able to get one organised?

David: Let's bump this two weeks because I suspect holidays will interfere this week.

... Same goes for the next topic (Event Timing API - @dbaron, @kenchris)

Dan: Are they blocked on us?

David: Not that I'm aware of

Discussed Jul 17, 2019 (See Github)

David: the order has the harder one first. there was a bunch of stuff related to text that wasn't fleshed out last time we looked. we should probably split images and texts into different reivews

kenneth: they also had questions re shadow dom, and wanted to move this to web performance

David: (currently WICG)

torgo: doing something useful and specific sounds like the right approach to me.

David: Yes. We should probably open the other review ourselves for text and close this one.

Originally this wasn't meant to include text... we tried to add it in, but that probably wasn't ideal.

Assuming Kenneth is okay with the images part

Kenneth: Yes

Comment by @dbaron Jul 17, 2019 (See Github)

So in hindsight, my response to https://github.com/w3ctag/design-reviews/issues/326#issuecomment-474503006 . At this point we're done reviewing the Element Timing API for images, but we should probably look a bit more at the text bits since we didn't really look at those much yet.

So the current state of this review is that I'm going to retitle it back to what it was originally, and say that at this point we're satisfied with the responses so far... and then I'm going to file a separate issue for text, unless you'd prefer to do that.

Thanks for requesting TAG review, and for all your responses to our concerns.