whatwg / xhr

XMLHttpRequest Standard
https://xhr.spec.whatwg.org/
Other
314 stars 129 forks source link

Editorial: Refactor for auto-reporting timing from fetch #347

Closed noamr closed 2 years ago

noamr commented 2 years ago

Depends on https://github.com/whatwg/fetch/pull/1413

This clarifies that a resource timing entry is always for a fetch and not for a request or response.


Preview | Diff

noamr commented 2 years ago

This needs updating to align with the revised approach.

Fixed

annevk commented 2 years ago

So the result here is that for synchronous XMLHttpRequest we don't create a timing entry, right?

noamr commented 2 years ago

So the result here is that for synchronous XMLHttpRequest we don't create a timing entry, right?

No, there is no behavioral change. In sync XHR the task that reports the timing would run on the parallel queue, but that's immaterial because the main-thread queue is waiting on it anyway.

annevk commented 2 years ago

I think there's two problems and they might also have been present with the previous text (though in a different way):

  1. When you're "in parallel" you don't have access to the main thread.
  2. "Pause" means nothing else can happen in the main thread (other than what it allows for). Having state changes happen during pause would be weird.

Perhaps sync XHR needs to invoke "report timing steps" itself?

noamr commented 2 years ago

I think there's two problems and they might also have been present with the previous text (though in a different way):

  1. When you're "in parallel" you don't have access to the main thread.
  2. "Pause" means nothing else can happen in the main thread (other than what it allows for). Having state changes happen during pause would be weird.

Perhaps sync XHR needs to invoke "report timing steps" itself?

Sure, that would be an easy solution. I'll revise the fetch spec to only report if not in a parallel queue

noamr commented 2 years ago

I think there's two problems and they might also have been present with the previous text (though in a different way):

  1. When you're "in parallel" you don't have access to the main thread.
  2. "Pause" means nothing else can happen in the main thread (other than what it allows for). Having state changes happen during pause would be weird.

Perhaps sync XHR needs to invoke "report timing steps" itself?

Fixed in https://github.com/whatwg/fetch/pull/1413/commits/9f0745ab2d82b65071778acaaf24196681031396 and Report timing for sync XHR

noamr commented 2 years ago

@annevk: I believe this is now ready for a re-review as it links to the corrected fetch.

domenic commented 2 years ago

This is blocking https://github.com/whatwg/xhr/pull/353. I will give my own review and ping @annevk about if that's enough.