libservo: Add a WebView::take_screenshot() API and use it for reftests#39583
Merged
mrobinson merged 1 commit intoservo:mainfrom Sep 30, 2025
Merged
libservo: Add a WebView::take_screenshot() API and use it for reftests#39583mrobinson merged 1 commit intoservo:mainfrom
WebView::take_screenshot() API and use it for reftests#39583mrobinson merged 1 commit intoservo:mainfrom
Conversation
|
🔨 Triggering try run (#18121131681) for Linux (WPT) |
delan
reviewed
Sep 30, 2025
| self.set_needs_repaint(RepaintReason::ReadyForScreenshot); | ||
| } | ||
| CompositorMsg::NewWebRenderFrameReady(..) => { | ||
| unreachable!("New WebRender frames should be handled in the caller."); |
Member
There was a problem hiding this comment.
can we add a comment in handle_messages() explaining why we set these messages aside, and maybe s/the caller/handle_messages()/ here?
yezhizhen
reviewed
Sep 30, 2025
Comment on lines
+1073
to
1076
| let img = self | ||
| .compositor | ||
| .borrow_mut() | ||
| .render_to_shared_memory(webview_id, page_rect); |
Member
There was a problem hiding this comment.
I wonder if we can do something similar here and completely remove render_to_shared_memory. This is also taking webview screenshot if page_rect == None.
If provided, it takes the screenshot of the rectangle.
Member
Author
There was a problem hiding this comment.
Yep! That's will be the next change in the series.
Member
There was a problem hiding this comment.
Nice! render_to_shared_memory is used as for WebDriver screenshot test, and should provide test coverage for the change.
|
Test results for linux-wpt from try job (#18121131681): Flaky unexpected result (29)
Stable unexpected results that are known to be intermittent (33)
|
|
✨ Try run (#18121131681) succeeded. |
delan
approved these changes
Sep 30, 2025
mrobinson
force-pushed
the
add-a-take-screenshot-api
branch
from
c1b2f31 to
a6d1f8a
Compare
servo-highfive
added
the
S-awaiting-review
mrobinson
force-pushed
the
add-a-take-screenshot-api
branch
from
a6d1f8a to
134a7d5
Compare
servo-highfive
added
the
S-awaiting-merge
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
servo-highfive
added
S-tests-failed
mrobinson
force-pushed
the
add-a-take-screenshot-api
branch
from
134a7d5 to
f072b4e
Compare
servo-highfive
removed
the
S-tests-failed
mrobinson
added
T-android
|
🔨 Triggering try run (#18126654002) for Android |
|
🔨 Triggering try run (#18126655985) for OpenHarmony |
|
|
|
|
This change adds a new API to the `WebView` for capturing screenshots. This makes it: - Possible to use the reftest waiting infrastructure via the API easily. - Possible to take more than a single screenshot in one Servo run. In addition, the previous infrastructure, the `wait_for_stable_image` option is removed completely. Co-authored-by: Delan Azabani <[email protected]> Signed-off-by: Martin Robinson <[email protected]>
mrobinson
force-pushed
the
add-a-take-screenshot-api
branch
from
f072b4e to
fd12589
Compare
mrobinson
added
T-android
github-actions
bot
removed
T-android
|
🔨 Triggering try run (#18127549061) for Android, OpenHarmony |
Bencher Report
|
✨ Try run (#18127549061) succeeded. |
servo-highfive
added
the
S-awaiting-merge
servo-highfive
removed
the
S-awaiting-merge
delan
added a commit
that referenced
this pull request
Oct 6, 2025
…sts (#39583) (bencher) {"fail_fast": false, "matrix": [{"name": "Linux (Bencher)", "workflow": "linux", "wpt": false, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": true, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}, {"name": "MacOS (Bencher)", "workflow": "macos", "wpt": false, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": true, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}, {"name": "Windows (Bencher)", "workflow": "windows", "wpt": false, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": true, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}, {"name": "Android (Bencher)", "workflow": "android", "wpt": false, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": true, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}, {"name": "OpenHarmony (Bencher)", "workflow": "ohos", "wpt": false, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": true, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}]}
mrobinson
added a commit
to mrobinson/servo
that referenced
this pull request
Oct 6, 2025
When processing new WebRender frames after handling messages in the compositor, only do that when we actually receive `NewWebRenderFrameReady` messages. This isn't an issue unless there are active animations. When there are, every spin of the event loop was triggering a repaint. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Delan Azabani <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 6, 2025
When processing new WebRender frames after handling messages in the compositor, only do that when we actually receive `NewWebRenderFrameReady` messages. This isn't an issue unless there are active animations. When there are, every spin of the event loop was triggering a repaint. Testing: This should fix the performance regression on the Speedometer Bencher benchmarks. Fixes: #39641 Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Delan Azabani <[email protected]>
mrobinson
added a commit
to mrobinson/servo
that referenced
this pull request
Oct 17, 2025
…ts.ready` This fixes a regression (likely from servo#39583) that made the result of many font loading tests intermittent. The issues here is that the count of fonts loading is decremented synchronously in the `FontContext`, but the notification to the `ScriptThread` happens asynchronously. In between the time the decrement happens and the `ScriptThread` is notified, a rendering update could happen which could mark a screenshot as ready to take too soon. The solution here is to wait until the `fonts.ready` promise is resolved, which is guaranteed to fire after all fonts have loaded. In addition, a rendering update is queued after this happens. This means that the screenshot will wait until that final rendering update to be ready. This should remove the flakiness of font load tests. Signed-off-by: Martin Robinson <[email protected]>
mrobinson
added a commit
to mrobinson/servo
that referenced
this pull request
Oct 17, 2025
…ts.ready` This fixes a regression (likely from servo#39583) that made the result of many font loading tests intermittent. The issues here is that the count of fonts loading is decremented synchronously in the `FontContext`, but the notification to the `ScriptThread` happens asynchronously. In between the time the decrement happens and the `ScriptThread` is notified, a rendering update could happen which could mark a screenshot as ready to take too soon. The solution here is to wait until the `fonts.ready` promise is resolved, which is guaranteed to fire after all fonts have loaded. In addition, a rendering update is queued after this happens. This means that the screenshot will wait until that final rendering update to be ready. This should remove the flakiness of font load tests. Signed-off-by: Martin Robinson <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 17, 2025
…ts.ready` (#39963) This fixes a regression (likely from #39583) that made the result of many font loading tests intermittent. The issues here is that the count of fonts loading is decremented synchronously in the `FontContext`, but the notification to the `ScriptThread` happens asynchronously. In between the time the decrement happens and the `ScriptThread` is notified, a rendering update could happen which could mark a screenshot as ready to take too soon. The solution here is to wait until the `fonts.ready` promise is resolved, which is guaranteed to fire after all fonts have loaded. In addition, a rendering update is queued after this happens. This means that the screenshot will wait until that final rendering update to be ready. This should remove the flakiness of font load tests. Testing: This should fix many flaky tests. Fixes: #39953. Fixes: #39951. Fixes #38956. Fixes #39408. Fixes #39429. Fixes #39592. Fixes #39636. Fixes #39650. Fixes #39666. Fixes #39667. Fixes #39706. Fixes #39750. Fixes #39801. Fixes #39853. Fixes #39881. Fixes #39950. Signed-off-by: Martin Robinson <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a new API to the
WebViewfor capturing screenshots.This makes it possible to:
easily.
WebViewnormally priorto the moment that the screenshot is ready, instead of preventing
all non-screenshot-ready paints while taking a screenshot.
In addition, the previous infrastructure, the
wait_for_stable_imageoption is removed completely.
Testing: This change is tested by the passing of the WPT tests,
as they commonly use the screenshot feature.