Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

compositing: Re-enable LCP calculation on WebViews after they have been reloaded/navigated#41169

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
shubhamg13 wants to merge1 commit intoservo:main
base:main
Choose a base branch
Loading
fromshubhamg13:flush_webview_on_reload_lcp

Conversation

@shubhamg13
Copy link
Contributor

@shubhamg13shubhamg13 commentedDec 10, 2025•
edited
Loading

Re-enable LCP calculation on WebViews after they have been reloaded/navigated.

Scenario: LCP calculation is stopped when user interaction is detected. When page is reloaded. No, LCP is observed. So, can't develop the performance or benchmark tools.

Motivation: No explicit specs found for this, but this was missed during implementation from my side. And also observed inconsistent with Chrome.

Testing:components/servo/tests/largest_contentful_paint_calculator.rs
Fixes: N/A (Earlier after reload LCP was not generated)

@servo-highfiveservo-highfive added the S-awaiting-reviewThere is new code that needs to be reviewed. labelDec 10, 2025
@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch fromce2b872 tob780cfdCompareDecember 10, 2025 06:20
@shubhamg13shubhamg13 changed the titleFlush webview from disabled list on reloadFlush webview from disabled list on reload for LCPDec 10, 2025
@mrobinson
Copy link
Member

  1. Why is this necessary? The PR description just repeats what the title and code does.
  2. Is there a specification for this behavior?
  3. Can you add a test for this behavior? TheTesting field is meant as a description of the test added or a justification of why adding a test is not possible at the moment. It should be possible to add unit tests for this.

}

pubfnreload(&self){
self.inner().servo.compositor().notify_reload(self.id());
Copy link
Member

Choose a reason for hiding this comment

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

It isvery odd that this only happens on reloads and not on any other type of navigation. Can you justify this a bit in the PR description? Why is this behavior necessary / useful?

@servo-highfiveservo-highfive added S-needs-code-changesChanges have not yet been made that were requested by a reviewer. and removed S-awaiting-reviewThere is new code that needs to be reviewed. labelsDec 10, 2025
@mrobinsonmrobinson changed the titleFlush webview from disabled list on reload for LCPcompositing: Re-enable LCP calculation on WebViews after they have been reloadedDec 10, 2025
@mrobinson
Copy link
Member

Let's add some tests for LCP. This is a good moment to do that.

Why don't we need this behavior on other types of navigations? Why just reloads?

@shubhamg13
Copy link
ContributorAuthor

Let's add some tests for LCP. This is a good moment to do that.

Why don't we need this behavior on other types of navigations? Why just reloads?

Yeah, You are right. We need this consideration for other navigations too. I will add a follow-up to this one.

@shubhamg13
Copy link
ContributorAuthor

cc:@xiaochengh

@shubhamg13shubhamg13 marked this pull request as draftDecember 11, 2025 10:01
@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch fromb780cfd to86aa138CompareDecember 11, 2025 10:02
@mrobinson
Copy link
Member

Yeah, You are right. We need this consideration for other navigations too. I will add a follow-up to this one.

Let's handle it in this change with tests please.

shubhamg13 reacted with thumbs up emoji

@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch 3 times, most recently from294ef95 to0e85549CompareDecember 16, 2025 04:29
@shubhamg13shubhamg13 changed the titlecompositing: Re-enable LCP calculation on WebViews after they have been reloadedcompositing: Re-enable LCP calculation on WebViews after they have been reloaded/navigatedDec 16, 2025
@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch 3 times, most recently frome51b6f4 toc737182CompareDecember 16, 2025 09:32
@shubhamg13shubhamg13 marked this pull request as ready for reviewDecember 16, 2025 09:33
@servo-highfiveservo-highfive added S-awaiting-reviewThere is new code that needs to be reviewed. and removed S-needs-code-changesChanges have not yet been made that were requested by a reviewer. labelsDec 16, 2025
@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch fromc737182 to6b820c0CompareDecember 16, 2025 09:36
@shubhamg13
Copy link
ContributorAuthor

Yeah, You are right. We need this consideration for other navigations too. I will add a follow-up to this one.

Let's handle it in this change with tests please.

I added for reload and load_url(from URL Bar and navigation). And added test as well. Please help to review.

@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch from6b820c0 to900cf8bCompareDecember 17, 2025 02:26
Signed-off-by: Shubham Gupta <shubham.gupta@chromium.org>
@shubhamg13shubhamg13force-pushed theflush_webview_on_reload_lcp branch from900cf8b to566cb53CompareDecember 17, 2025 07:16
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@mrobinsonmrobinsonAwaiting requested review from mrobinsonmrobinson is a code owner

@LoirooriolLoirooriolAwaiting requested review from Loirooriol

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

S-awaiting-reviewThere is new code that needs to be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shubhamg13@mrobinson@servo-highfive
[8]ページ先頭
©2009-2025 Movatter.jp