w3c / performance-timeline

Performance Timeline
https://w3c.github.io/performance-timeline/
Other
111 stars 34 forks source link

Cross-origin timeline support #207

Open clelland opened 1 year ago

clelland commented 1 year ago

https://github.com/w3c/performance-timeline/pull/202 has been open for a while, but we should really have an issue for larger discussion :)

The overall plan is roughly as specified in Cross-frame Performance Timeline, which is:

The current concrete proposal for each of these is:

  1. PerformanceObserverInit gains one new key: includeFrames. This is a boolean, defaulting to false, and can be used like this:
const observer = new PerformanceObserver(entryList => {/* callback */});
observer.observe({entryTypes: ["mark", "measure"],
                  includeFrames: true});
  1. We add a Document Policy configuration point, share-performance-timeline-with, which takes a list of origins (possibly *) with which the framed document consents to share its timeline entries. Examples of its usage would look like:
Document-Policy: share-performance-timeline-with=*
Document-Policy: share-performance-timeline-with="https://example.com"
Document-Policy: share-performance-timeline-with=("https://example.com/", "https://embedder.com")
  1. The source member of PerformanceEntry refers to the window of the frame in which the entry was generated.

It is an open question whether we want to, or can support this functionality on the synchronous timeline API methods getEntries, getEntriesByName and getEntriesByType. Because it requires cross-origin communication, this almost certainly would have to block while waiting for a response from child frames, possibly for a long time.

Those APIs may be useful, and are often easier to use than the PerformanceObserver API, but it may not be worth making them slower to support cross-origin frames. People who are writing new code to gather that data could just be encouraged to migrate to PerformanceObserver at the same time.

nicjansma commented 1 year ago

It seems to me that not exposing these entries to getEntries() etc (and only via an observer) is a fair trade-off for having access to those cross-origin entries.

One of the main reasons we (RUM provider / boomerang.js) still rely on .getEntriesByType() is for gathering ResourceTiming data from all accessible (same-origin) frames and subframes on the page. The ergonomics of using a PerformanceObserver when trying to gather the "complete" tree are tricky without includeFrames: one would have to crawl the entire DOM for iframes and add an observer to each frame anyways (and wait for their async callbacks), so we might as well just call .getEntriesByType() on each frame as we traverse the DOM instead.

Once includeFrames is available we could just register a top-level Observer and avoid the DOM crawling. In practice this may be tricky as not all customers / 3P frames may have Document-Policy, but we can bridge that gap when we get there. (For example, we can encourage our customers/3Ps to include the Document-Policy if they want a more efficient gathering of ResourceTiming data on their page, as the gathering today is a bit expensive).

clelland commented 1 year ago

I've moved the synchronous getEntries* methods to an "Alternatives considered" section in the document. I don't think we can implement them in a performant way :(

sefeng211 commented 1 year ago

Just to reillustrate my question in the call - Looks like in the current proposal, documents can broadcast any entries which means entries that were broadcasted from a different window can also be broadcasted out.

Consider a case where A embeds B and B embeds C, B can broadcast entries from C to A, even if C only specifies B explicitly. I think this needs be handled by the proposal such that A can only receive entries from subdocuments that specifies A explicitly.

yoavweiss commented 1 year ago

If C exposes its entries to B, it trusts B to not abuse that privilege. B can then communicate this data by many means to A, even if we block broadcasting in this particular API.

clelland commented 1 year ago

That's true, but this still might be a footgun if developers have to remember to not rebroadcast entries that they received from their own children, in order to not cause a cascade of entries up the tree.

It might make sense to just drop any entries from the list which have a source attribute that doesn't match the current window object.

yoavweiss commented 1 year ago

That's true, but this still might be a footgun if developers have to remember to not rebroadcast entries that they received from their own children, in order to not cause a cascade of entries up the tree.

What I meant to say is that given that the information can be shared by other means, there's no security benefit from blocking its broadcast. (but there may be ergonomic ones - e.g. enable C to only broadcast to B and not elsewhere)

nicjansma commented 1 year ago

For posterity, we had discussed this in the April 13th, 2023 W3C WebPerf call.

Notes: https://docs.google.com/document/d/1fIgfd7ONz-sES55qRFHQI_eQUnynnyX2Asrs-cwAQ_Q/edit#

sefeng211 commented 1 year ago

What I meant to say is that given that the information can be shared by other means, there's no security benefit from blocking its broadcast. (but there may be ergonomic ones - e.g. enable C to only broadcast to B and not elsewhere)

Yeah, so effectively once you allow someone to read your timeline, then it's up to the other player to not doing evil things. And this sounds bad...