layout: Do not consider `iframe` fragments as contentful for paint timing by shubhamg13 · Pull Request #42498 · servo/servo

shubhamg13 · 2026-02-10T05:26:20Z

This aligns first-contentful-paint about iframe according to specs, Step: 10.2.1.

NOTE: A parent frame should not be aware of the paint events from its child iframes, and vice versa. This means that a frame that contains just iframes will have first paint (due to the enclosing boxes of the iframes) but no first contentful paint.

Testing: Update WPT tests expectations.

TimvdLippe

LGTM with 1 nit

TimvdLippe · 2026-02-10T07:24:52Z

components/layout/display_list/mod.rs

                let style = iframe.base.style();
                match style.get_inherited_box().visibility {
                    Visibility::Visible => {
-                        builder.mark_is_contentful();


Nit: please add the spec text as comment here to explain why we aren't calling this anymore.

Hmm, I don't have a good feeling about adding a comment for not doing something unless it is TODO or else.

IMHO, it should be other way around mentioning which are contentful.

I think it makes sense for people reading the code that there is a comment explaining this. Consider that without the comment someone will think it is a bug that an <iframe> isn't considered contentful.

I am curious about this though. Is it that <iframe>s are not considered contentful or simply that <iframe>s that haven't loaded yet are not considered contentful?

A parent frame should not be aware of the paint events from its child iframes

iframe should send FCP to its own browsing context, not top level browser context.

Chrome: Reports FCP to iframe's context (not top level browser context).

A parent frame should not be aware of the paint events from its child iframes

iframe should send FCP to its own browsing context, not top level browser context.

The Fragment::IFrame list is not actually the interior contents of the <iframe>. It is a slot that the child frame's display list will be inserted into during the final paint pass. It doesn't have anything to do with paint events that happen in that child iframe, which happen when the child iframe's display list is built for the child iframe's Document.

mrobinson · 2026-02-10T08:20:35Z

@shubhamg13 For what it's worth, it's a bit rude to simply refuse a reviewer's request because you disagree. If a reviewer is mistaken (this isn't unusual), then generally you try to resolve the misunderstanding or miscommunication. To simply say, "I don't want to do this" is, in general, not a great way to interact in the project.

shubhamg13 · 2026-02-10T08:27:25Z

Ahh my intention was not that to refuse that. I didn't mean to sound rude.

Let me correct that comment.

Added a TODO.

mrobinson · 2026-02-10T10:58:27Z

components/layout/display_list/mod.rs

                match style.get_inherited_box().visibility {
                    Visibility::Visible => {
-                        builder.mark_is_contentful();
+                        // TODO: Send FCP for iframe to its own browsing context.


So, is this not happening? Each <iframe> is a pipeline and each pipeline has a display list. Each display list checks whether it is contentful or not and then tells the pipeline when the first contentful paint happened.

As far I remember, currently any FCP reported irrespective of display list is sent to global scope, ideally FCP from iframe should be sent to iframe scope instead.

If that's an issue though, this change is not fixing it. The presence of this display list item is only applicable to the Document that it is found in. It doesn't have anything to do with the display list of the child frame, other than providing a slot for it in the final rendering.

To clarify, this fragment is about the wrapper <iframe> in the parent document and doesn't have anything to do with the display list of the child <iframe>. It is not affected at all by paint events that happen in the child <iframe>.

Your last comment sounds little confusing.

In simplified manner, My motive with this patch is stop firing FCP for any iframe either in parent document or child iframe as of now.

mrobinson · 2026-02-10T11:05:11Z

@shubhamg13 Out of curiosity, is this change solving an actual issue that you are seeing?

shubhamg13 · 2026-02-10T11:25:52Z

@shubhamg13 Out of curiosity, is this change solving an actual issue that you are seeing?

Actually it is solving Intermittent behaviour by not reporting FCP of iframe (to global scope by default).

Yeah, it needs some more efforts and time to properly align as per specs. But as far this seems quite reasonable for now to me. WDYS?

mrobinson · 2026-02-10T11:29:14Z

Actually it is solving Intermittent behaviour by not reporting FCP of iframe (to global scope by default).

When you say intermittent behavior are you referring to tests that are no longer intermittent with this change?

shubhamg13 · 2026-02-10T15:05:53Z

Actually it is solving Intermittent behaviour by not reporting FCP of iframe (to global scope by default).

When you say intermittent behavior are you referring to tests that are no longer intermittent with this change?

Yes, that's what I mean. This will still require a revisit. That's why I added a TODO.

cc: @xiaochengh

mrobinson · 2026-02-10T16:39:49Z

When you say intermittent behavior are you referring to tests that are no longer intermittent with this change?

Yes, that's what I mean. This will still require a revisit. That's why I added a TODO.

Sorry, I meant to also ask which tests those were specifically? Are they WPT tests?

mrobinson

Looks good, with one change:

mrobinson · 2026-02-10T17:07:18Z

components/layout/display_list/mod.rs

                match style.get_inherited_box().visibility {
                    Visibility::Visible => {
-                        builder.mark_is_contentful();
+                        // TODO: Send FCP for iframe to its own browsing context.


I think that the TODO is probably wrong, as it's already the case that paint events are sent to their browsing context, right? That said, I think @TimvdLippe original suggestion was to just put the specification text here, so let's do that:

Suggested change

// TODO: Send FCP for iframe to its own browsing context.

// From <https://www.w3.org/TR/paint-timing/>:

// > A parent frame should not be aware of the paint events from its child iframes, and

// > vice versa. This means that a frame that contains just iframes will have first paint

// > (due to the enclosing boxes of the iframes) but no first contentful paint.

jdm · 2026-02-11T04:06:07Z

components/layout/display_list/mod.rs

+                        // > A parent frame should not be aware of the paint events from its child iframes, and
+                        // > vice versa.
+                        if builder.webrender_display_list_builder.pipeline_id ==
+                            iframe.pipeline_id.into()


This check doesn't make sense to me—an iframe will never contain the pipeline that the current display list builder is building for.

Yeah that's my concern. (But it saves from marking other pipelines as contentful)

You got my intent, any tip how to achieve that.

It looks like you're focusing on this specification text:

A parent frame should not be aware of the paint events from its child iframes, and vice versa. This means that a frame that contains just iframes will have first paint (due to the enclosing boxes of the iframes) but no first contentful paint.

But the reason for making the change in this PR is not related to this text; the current implementation isn't sending FCP notifications for a child frame to the parent frame. Instead, we should focus on this text:

If document should report first contentful paint, then:

which links to this text:

If document contains at least one element that is both paintable and contentful, then return true.

which links to https://www.w3.org/TR/paint-timing/#contentful, which does not include any steps that treat an iframe element as contentful.

Therefore, from my reading, we should never mark the display list as contentful when processing an iframe fragment.

Completely agree with all your explanations.

If I understood completely. Following is summary:

if iframe contains an image, it will mark FCP within its own scope without notifying parent.

As iframe will be treated as a separate document, it should have its own paint timings irrespective of parent document.

That matches my understanding!

Signed-off-by: Shubham Gupta <[email protected]>

`iframe` should be considered for `first-paint`. TODO of: #42975 Synced: web-platform-tests/wpt#58243 Refer for some history: #42498 Testing: - `wpt/paint-timing/first-paint-only/child-painting-first-image.html` - `wpt/paint-timing/first-paint-only/sibling-painting-first-image.html` Fixes: #42455 --------- Signed-off-by: Shubham Gupta <[email protected]>

shubhamg13 requested review from Loirooriol and mrobinson as code owners February 10, 2026 05:26

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 10, 2026

shubhamg13 force-pushed the iframe_fcp branch from 6b13ac5 to ae1fa5d Compare February 10, 2026 05:27

shubhamg13 mentioned this pull request Feb 10, 2026

Feature: LargestContentfulPaint #42000

Open

TimvdLippe approved these changes Feb 10, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 10, 2026

shubhamg13 requested a review from TimvdLippe February 10, 2026 08:02

shubhamg13 force-pushed the iframe_fcp branch from ae1fa5d to 45c4d8c Compare February 10, 2026 08:41

servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Feb 10, 2026

shubhamg13 changed the title ~~layout: iframe should not fire first-contentful-paint~~ layout: iframe should not fire first-contentful-paint in top level browsing context Feb 10, 2026

shubhamg13 force-pushed the iframe_fcp branch from 45c4d8c to 1f07dad Compare February 10, 2026 08:58

mrobinson reviewed Feb 10, 2026

View reviewed changes

mrobinson requested changes Feb 10, 2026

View reviewed changes

servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Feb 10, 2026

shubhamg13 force-pushed the iframe_fcp branch from 1f07dad to 0bcdd2a Compare February 11, 2026 04:00

servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Feb 11, 2026

shubhamg13 changed the title ~~layout: iframe should not fire first-contentful-paint in top level browsing context~~ layout: Mark first-contentful-paint for iframe only in its own pipeline Feb 11, 2026

shubhamg13 requested a review from mrobinson February 11, 2026 04:05

jdm reviewed Feb 11, 2026

View reviewed changes

shubhamg13 marked this pull request as draft February 11, 2026 04:10

shubhamg13 marked this pull request as ready for review February 11, 2026 04:12

shubhamg13 marked this pull request as draft February 11, 2026 05:30

shubhamg13 force-pushed the iframe_fcp branch from 0bcdd2a to 50a8e1c Compare February 11, 2026 05:56

shubhamg13 marked this pull request as ready for review February 11, 2026 05:58

shubhamg13 changed the title ~~layout: Mark first-contentful-paint for iframe only in its own pipeline~~ layout: iframe should not fire first-contentful-paint in top level browsing context Feb 11, 2026

shubhamg13 requested a review from jdm February 11, 2026 06:00

shubhamg13 added 2 commits February 11, 2026 14:04

iframe are not contentful for parent.

1cca82c

Signed-off-by: Shubham Gupta <[email protected]>

Update WPT tests expectations

6d0dd39

Signed-off-by: Shubham Gupta <[email protected]>

shubhamg13 force-pushed the iframe_fcp branch from 50a8e1c to 6d0dd39 Compare February 11, 2026 06:08

mrobinson changed the title ~~layout: iframe should not fire first-contentful-paint in top level browsing context~~ layout: Do not consider iframe fragments at contentful for paint timing Feb 11, 2026

mrobinson approved these changes Feb 11, 2026

View reviewed changes

servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Feb 11, 2026

mrobinson added this pull request to the merge queue Feb 11, 2026

servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 11, 2026

shubhamg13 changed the title ~~layout: Do not consider iframe fragments at contentful for paint timing~~ layout: Do not consider iframe fragments as contentful for paint timing Feb 11, 2026

Merged via the queue into servo:main with commit dbc8a7f Feb 11, 2026
33 checks passed

servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Feb 11, 2026

shubhamg13 deleted the iframe_fcp branch February 11, 2026 10:59

shubhamg13 mentioned this pull request Feb 14, 2026

Feature: PaintTiming #42623

Open

shubhamg13 mentioned this pull request Mar 23, 2026

layout: Consider iframe for first-paint #42149

Merged

-                        // TODO: Send FCP for iframe to its own browsing context.
+                        // From <https://www.w3.org/TR/paint-timing/>:
+                        // > A parent frame should not be aware of the paint events from its child iframes, and
+                        // > vice versa. This means that a frame that contains just iframes will have first paint
+                        // > (due to the enclosing boxes of the iframes) but no first contentful paint.

Uh oh!

Conversation

shubhamg13 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimvdLippe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shubhamg13 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrobinson commented Feb 10, 2026

Uh oh!

shubhamg13 commented Feb 10, 2026

Uh oh!

mrobinson commented Feb 10, 2026

Uh oh!

shubhamg13 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson commented Feb 10, 2026

Uh oh!

mrobinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdm Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubhamg13 Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shubhamg13 commented Feb 10, 2026 •

edited

Loading

shubhamg13 Feb 10, 2026 •

edited

Loading

mrobinson commented Feb 10, 2026 •

edited

Loading

shubhamg13 commented Feb 10, 2026 •

edited

Loading

shubhamg13 Feb 10, 2026 •

edited

Loading

shubhamg13 commented Feb 10, 2026 •

edited

Loading

shubhamg13 Feb 11, 2026 •

edited

Loading

jdm Feb 11, 2026 •

edited

Loading

shubhamg13 Feb 11, 2026 •

edited

Loading