Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report ResourceTiming for object and embed #7554

Merged
merged 4 commits into from
Feb 27, 2022

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Jan 30, 2022

(See WHATWG Working Mode: Changes for more details.)


/iframe-embed-object.html ( diff )

@noamr
Copy link
Contributor Author

noamr commented Jan 31, 2022

See #6542 for context

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The object spec looks hopeless and I'm happy with leaving what you've done as the best we can do.

The embed spec looks like it would benefit from updating "To process response for the response response:" to use the actual processResponse argument? I think that'd be a small tweak that would avoid mixing old and new styles and is worth the trouble.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK let's think about about this plus #7531 . Both in terms of what to do near-term and what's the best long-term architecture.

Best long-term: object, embed, iframe, and frame all share as much infrastructure as possible. So probably the navigate algorithm, eventually bottoming out in the various "page load processing model for X document" sections, does the resource timing entries, for basically all cases. Ideally they are always nested browsing contexts of some sort, although it's unclear whether we'll be able to get away with that. Even more ideally, they would stop going down the special "navigate to a response" mode, and instead end up in "navigate to a URL".

Shorter-term: embed only uses "navigate" and nested browsing context for image/svg+xml. (That's #513.) object uses navigate/nested browsing contexts more often, but still uses something else for image/ responses, unsupported resource types, and plugins. (Plugins are getting removed in #6315, mostly.) Both of them use "navigate to a response" when they do use navigate, i.e., they do their own separate fetching that is not part of navigate.

Where does that leave us? I guess I see two paths:

  • What you've done across the two CLs, which is basically, only iframe/frame report resource timing as part of navigate; object/embed have separate reporting paths, even when they invoke the navigate algorithm.
  • Try to have resource timing reported as part of navigate, whenever navigate is invoked. Report it separately in other cases. (E.g. image/ in object.)

The second path might set us up for more success long term. But is probably messier in the short term. So I guess I am OK with your current approach.

Let me know if that sounds good to you, and then I can merge this after you fix the couple of typos.

source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
@noamr
Copy link
Contributor Author

noamr commented Feb 17, 2022

OK let's think about about this plus #7531 . Both in terms of what to do near-term and what's the best long-term architecture.

Best long-term: object, embed, iframe, and frame all share as much infrastructure as possible. So probably the navigate algorithm, eventually bottoming out in the various "page load processing model for X document" sections, does the resource timing entries, for basically all cases. Ideally they are always nested browsing contexts of some sort, although it's unclear whether we'll be able to get away with that. Even more ideally, they would stop going down the special "navigate to a response" mode, and instead end up in "navigate to a URL".

Shorter-term: embed only uses "navigate" and nested browsing context for image/svg+xml. (That's #513.) object uses navigate/nested browsing contexts more often, but still uses something else for image/ responses, unsupported resource types, and plugins. (Plugins are getting removed in #6315, mostly.) Both of them use "navigate to a response" when they do use navigate, i.e., they do their own separate fetching that is not part of navigate.

Where does that leave us? I guess I see two paths:

  • What you've done across the two CLs, which is basically, only iframe/frame report resource timing as part of navigate; object/embed have separate reporting paths, even when they invoke the navigate algorithm.

  • Try to have resource timing reported as part of navigate, whenever navigate is invoked. Report it separately in other cases. (E.g. image/ in object.)

The second path might set us up for more success long term. But is probably messier in the short term. So I guess I am OK with your current approach.

Let me know if that sounds good to you, and then I can merge this after you fix the couple of typos.

Yes totally sounds good. I would need to dig in more before doing the bigger refactor and other technical debt should maybe come first before taking on things to do with object/embed

@domenic
Copy link
Member

domenic commented Feb 17, 2022

Great. Also to be clear changing object/embed to be frame/iframe like is more than just a refactor. It'll require potentially-compat-risky browser changes if we really want to go for the most-elegant architecture. If I'm being honest it's mostly more of an aspiration than a plan...

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will merge once the template is filled out (including tests).

@domenic domenic merged commit 9d42d2c into whatwg:main Feb 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants