Don't hold back the delivery of pending animation events
The script event loop can block for an indefinite period despite having pending animation-related events that it needs to deliver immediately.
The bug can cause a test timeout in `/css/css-transitions/events-001.html` but (to my knowledge) never manifested in `master` because any irrelevant messages received by the script thread afterward would unblock the event loop and cause it to process any pending animation-related events. We can reproduce the bug in `master` by removing [the `maybe_observe_paint_time` call][2] in `LayoutThread::compute_abs_pos_and_build_display_list`. This also explains the test failure [seen in #28708][1] because the same call is [commented out][3].
[1]: https://github.com/servo/servo/pull/28708#issuecomment-1080003778
[2]: 5448528279/components/layout_thread/lib.rs (L1156-L1157)
[3]: 03a41ffe32/components/layout_thread/lib.rs (L1186-L1187)
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
---
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___
Use crates.io release of raqote
<!-- Please describe your changes on the following line: -->
The latest raqote version has been published to crates.io now.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Update gst plugin for videoconvert & videoscale elements
GStreamer 1.22 has [merged](d11f13f476 (0f8c6b125485c13b682ebcb8fd2f7266439ac317)) the 'gstvideoscale' and 'gstvideoconvert' plugins into a new 'gstvideoconvertscale' plugin. Since we hardcode the plugin names [here](https://github.com/servo/servo/blob/master/python/servo/gstreamer.py#L68) and [here](https://github.com/servo/servo/blob/master/python/servo/gstreamer.py#L71), to support packaging them with the binary, the builds on MacOS have started to fail since Homebrew upgraded to 1.22.
This PR is a temporary fix to allow CI builds to succeed & PRs to be merged.
I see code path in the packaging logic for Windows msvc also relies on these plugin names when media-stack is set to 'gstreamer' (I believe it is 'dummy' by default and on nightlies). Ideally, we should fix the packaging logic so that it can handle both older versions of GST (<1.22) and both mac & win platforms. I'll follow up on this with an new issue.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix#29344
<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because we are only changing the release packaging logic.
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Fixes the test case `/_mozilla/css/css-transition-cancel-event
.html`, which was failing under a specific circumstance.
The observed sequence of events during the failing test run looks like
this:
1. Transitions start in `div1` and `div2`.
2. `div1` generates a `transitionend` event.
3. The `transitionend` event handler removes `div2` from DOM, cancelling
its ongoing transition.
4. `div2` is supposed to generate a `transitioncancel` event in a timely
manner, which it does not. The test fails as a result.
What is going on here? Here's a possible explaination:
1. During one invocation of `ScriptThread::handle_msgs`...
2. In step 2, `ScriptThread::update_animations_send_events` -> `Document
::update_for_new_timeline_value` detects the completion of the
transition, and in response, pends the `transitionend` event.
3. In step 3, `ScriptThread::update_animations_send_events` ->
`Animations::send_pending_events` calls the `transitionend` handler.
4. The `transitionend` event handler removes `div2`, thereby cancelling
its ongoing transition and triggering a reflow.
5. Reflow takes place. During this, `Animations::do_post_reflow_update`
-> `Animations::handle_canceled_animations` pends the
`transitioncancel` event (precursor to step 4).
6. Having discovering that there was no running animation, `Animations::
do_post_reflow_update` calls `self.update_running_animation_presence
(_, false)`, which sends `AnimationState::NoAnimationsPresent`.
7. The invocation of `ScriptThread::handle_msgs` ends, and another
starts. It blocks waiting for events.
8. Meanwhile, the compositor receives `AnimationState::
NoAnimationsPresent` and stops further generation of animation ticks.
9. With no events to wake it up, the script thread is stuck waiting
despite having the pending `transitioncancel` event (step 4).
The HTML specification [says][1] that "an event loop must continually
run [...] as long as it exists" and does not say it can block if there
is nothing to do. Blocking is merely optimization in a user agent
implementation. Pending animation-related events must be processed every
time a "rendering opportunity" arises unless the user agent has a reason
to believe that it "would have no visible effect".
Skipping the processing of animation-related events would have visible
effect if such events are indeed present. The correct implementation in
Servo, therefore, would be to request more animation ticks so that such
events are processed in a subsequent tick.
[1]: https://html.spec.whatwg.org/multipage/#event-loop-processing-model
This commit reverses the order of the `send_pending_events` and
`update_animation_timeline` calls in `ScriptThread::update_animations_
send_events` so that animation-related events pended by the latter are
processed by the former.
The new calling order is more compliant with the "update animations and
send events" algorithm steps from [the Web Animations specification][1].
The old implementation was prone to blocking for an indefinite period
while holding pending events. Due to complex interaction with other
events and timing behavior, I was only able to reproduce the problem
under the following conditions:
- *The `maybe_observe_paint_time` call in `LayoutThread::compute_abs_
pos_and_build_display_list` is removed from the code*. While
performance events may seem irrelevant to the issue, they would
bombard the script thread with events. *Any* extra messages received
would unblock the event loop and prevent the manifestation of the
issue. (But, of course, we aren't supposed to count on that to avoid
the issue.)
- Servo is running in a headless mode, which somehow makes it less
likely for the script thread to receive a `TickAllAnimations` event
after sending `AnimationState::NoAnimationsPresent`.
With the above conditions met and the stars aligned, you can reproduce
the problem by running the WPT test `/css/css-transitions/events-001.
html`.
[1]: https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events
Bump euclid to 0.22
Major changes:
- All matrices are now stored in row major order. This means that parameters to rotation functions should no longer be negated.
- `post_...()` functions are now named `then()`. `pre_transform()` is removed, so `then()` is used and the order of operations changed.
In addition, this PR updates lyon_geom to the latest version.
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix#27424
- [x] There are tests for these changes
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Output test results as GitHub comments
After filtering intermittents, output the results as JSON. Update the
GitHub workflow to aggregate this JSON data into an artifact and use the
aggregated data to generate a GitHub comment with details about the try
run. The idea here is that this comment will make it easier to track
intermittent tests and notice when a change affects a test marked as
intermittent -- either causing it to permanently fail or fixing it.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because they modify the CI infrastructure.
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
After filtering intermittents, output the results as JSON. Update the
GitHub workflow to aggregate this JSON data into an artifact and use the
aggregated data to generate a GitHub comment with details about the try
run. The idea here is that this comment will make it easier to track
intermittent tests and notice when a change affects a test marked as
intermittent -- either causing it to permanently fail or fixing it.
wpt: Make WPT result formatting logic independent of ServoHandler
This will allow results to be formatted by other parts of the code (such as the intermittent filtering code). Previously formatting was handled in ServoHandler, which was a bit strange as it's really only necessary for GroupingFormatter and the intermittent filtering code. This also allows the results to be properly typed by the Python typing system.
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because they are part of the testing infrastructure that is generally untested.
This will allow results to be formatted by other parts of the code (such
as the intermittent filtering) code. Previously, formatting was handled
in ServoHandler, which was a bit strange as it's really only necessary
for GroupingFormatter and the intermittent filtering code. This also
allows the results to be properly typed by the Python typing system.
ci: Produce a single WPT log artifact
GitHub supports adding files to an artifact in parallel, as long as the filenames are unique. This makes it easier to download build results when more than a single builder fails.
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because this is a small change to the CI.
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Bump unicode-segmentation from 1.9.0 to 1.10.1
Bumps [unicode-segmentation](https://github.com/unicode-rs/unicode-segmentation) from 1.9.0 to 1.10.1.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a href="https://github.com/unicode-rs/unicode-segmentation/commits/v1.10.1">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
</details>
Run `test-tidy` sooner for pull requests
This lets the builder fail sooner when there is an issue with the style.
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because they are a small change to the CI.
GitHub supports adding files to an artifact in parallel, as long as the
filenames are unique. This makes it easier to download build results
when more than a single builder fails.
Upgrade the Rust toolchain to 'nightly-2023-02-01'
<!-- Please describe your changes on the following line: -->
This change should address the failing nightly [rustc test jobs](https://github.com/servo/servo/actions/workflows/nightly-rust.yml)
For reference, these are the [relevant](https://github.com/rust-lang/rust/pull/107206) [PRs](https://github.com/rust-lang/rust/pull/104170) in rustc that I could find.
Signed-off-by: Mukilan Thiyagarajan <mukilanthiagarajan@gmail.com>
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because there are existing unit tests for script_plugins that do pass.
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
wpt: retain raw results for unexpected tests and subtests
ServoHandler currently processes the raw results for unexpected tests and subtests on the fly, discarding the test_end (test) or test_status (subtest) event data once it has generated output for the events.
This patch retains that event data in ServoHandler.test_failures and ServoHandler.subtest_failures, allowing servowpt.py to send structured data about unexpected tests and subtests to the dashboard (servo/intermittent-tracker#5), e.g.
```diff
diff --git a/tests/wpt/servowpt.py b/tests/wpt/servowpt.py
index fee6dcf2df..ac3e46e36d 100644
--- a/tests/wpt/servowpt.py
+++ b/tests/wpt/servowpt.py
@@ -27,6 +27,7 @@ import update # noqa: F401,E402
TRACKER_API = "https://build.servo.org/intermittent-tracker"
TRACKER_API_ENV_VAR = "INTERMITTENT_TRACKER_API"
+TRACKER_DASHBOARD_SECRET_ENV_VAR = "INTERMITTENT_TRACKER_DASHBOARD_SECRET"
GITHUB_API_TOKEN_ENV_VAR = "INTERMITTENT_TRACKER_GITHUB_API_TOKEN"
@@ -145,6 +146,48 @@ def run_tests(**kwargs):
logger.add_handler(handler)
wptrunner.run_tests(**kwargs)
+
+ if TRACKER_DASHBOARD_SECRET_ENV_VAR in os.environ:
+ body = []
+ for failure in handler.test_failures:
+ # print(f'>>> {repr(failure)}')
+ body.append({
+ 'path': failure['test'],
+ 'subtest': None,
+ 'expected': failure['expected'],
+ 'actual': failure['status'],
+ 'time': failure['time'] // 1000,
+ 'message': failure.get('message'),
+ 'stack': failure.get('stack'),
+ 'branch': os.environ.get('SERVO_BRANCH'),
+ 'build_url': os.environ.get('SERVO_BUILD_URL'),
+ 'pull_url': os.environ.get('SERVO_PULL_URL'),
+ })
+ for (path, failures) in handler.subtest_failures.items():
+ for failure in failures:
+ # print(f'>>> {repr(failure)}')
+ body.append({
+ 'path': path,
+ 'subtest': failure['subtest'],
+ 'expected': failure['expected'],
+ 'actual': failure['status'],
+ 'time': failure['time'] // 1000,
+ 'message': failure.get('message'),
+ 'stack': failure.get('stack'),
+ 'branch': os.environ.get('SERVO_BRANCH'),
+ 'build_url': os.environ.get('SERVO_BUILD_URL'),
+ 'pull_url': os.environ.get('SERVO_PULL_URL'),
+ })
+ request = urllib.request.Request(
+ f'{os.environ.get(TRACKER_API_ENV_VAR, TRACKER_API)}/dashboard/attempts',
+ method='POST',
+ data=json.dumps(body).encode('utf-8'),
+ headers={
+ 'Authorization': f'Bearer {os.environ[TRACKER_DASHBOARD_SECRET_ENV_VAR]}',
+ 'Content-Type': 'application/json',
+ })
+ urllib.request.urlopen(request)
+
if handler.unexpected_results and filter_intermittents_output:
all_filtered = filter_intermittents(
handler.unexpected_results,
```
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] These changes fix #___ (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
