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

compositor: Improve the way we wait for frames #31523

Merged
merged 2 commits into from Mar 7, 2024

Conversation

mrobinson
Copy link
Member

In the newest version of WebRender it will be harder to make the
distinction between frame queued for scrolling and other kinds of
pending frames. This change makes it so that we queue frames for both
kinds of changes the same way and keeps a counting of pending frames.
This is conceptually a lot simpler.

In addition, do queue a composite even when recomposite isn't necessary
for a WebRender frame when there are active requestAnimationFrame
callbacks. Doing a composite is what triggers the callbacks to actually
run in the script thread! I believe this was a bug, but the WebRender
upgrade made it much more obvious.

These changes are in preparation for the WebRender upgrade.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because there is no reliable behavior change, but we notice less flakes when running tests.

@mrobinson mrobinson added the T-linux-wpt-2020 Do a try run of the WPT label Mar 6, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

🔨 Triggering try run (#8174165456) for Linux WPT

Copy link

github-actions bot commented Mar 6, 2024

⚠️ Try run (#8174165456) failed.

In the newest version of WebRender it will be harder to make the
distinction between frame queued for scrolling and other kinds of
pending frames. This change makes it so that we queue frames for both
kinds of changes the same way and keeps a counting of pending frames.
This is conceptually a lot simpler.

In addition, do queue a composite even when recomposite isn't necessary
for a WebRender frame when there are active requestAnimationFrame
callbacks. Doing a composite is what triggers the callbacks to actually
run in the script thread! I believe this was a bug, but the WebRender
upgrade made it much more obvious.

These changes are in preparation for the WebRender upgrade.
@mrobinson mrobinson force-pushed the improve-frame-waiting-compositor branch from c4122ea to 4380e77 Compare March 6, 2024 15:09
@mrobinson mrobinson added the T-linux-wpt-2020 Do a try run of the WPT label Mar 6, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

🔨 Triggering try run (#8175329291) for Linux WPT

Copy link

github-actions bot commented Mar 6, 2024

Test results for linux-wpt-layout-2020 from try job (#8175329291):

Flaky unexpected result (18)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • TIMEOUT [expected OK] /_webgl/conformance/uniforms/out-of-bounds-uniform-array-access.html (#26225)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected FAIL] /css/css-position/sticky/position-sticky-fractional-offset.html
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • PASS [expected FAIL] subtest: listeners are called correct number of times
  • TIMEOUT /css/cssom-view/MediaQueryList-extends-EventTarget.html (#25269)
    • FAIL [expected PASS] subtest: onchange adds listener promise_test: Unhandled rejection with value: object "TypeError: _event is undefined"
  • TIMEOUT [expected OK] /custom-elements/reactions/customized-builtins/HTMLMediaElement.html (#31014)
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/005.html (#27062)
  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • FAIL [expected PASS] subtest: Check execution order from nested timeout assert_equals: Expected nested setTimeout to run second expected true but got false
    • FAIL [expected PASS] subtest: Check execution order on load handler assert_equals: Expected onload to run first expected false but got true
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-cross-origin.sub.window.html (#29056)
    • PASS [expected FAIL] subtest: Cross-origin navigation started from unload handler must be ignored
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • FAIL [expected PASS] subtest: Multiple history traversals, last would be aborted assert_array_equals: Pages opened during history navigation expected property 1 to be 5 but got 3 (expected array [6, 5] got [6, 3])
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • PASS [expected FAIL] subtest: text/plain: 0x00 in name (normal form)
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html (#29137)
    • FAIL [expected PASS] subtest: document.write in an imported module assert_true: onload must be called expected true got false
  • CRASH [expected OK] /referrer-policy/gen/iframe.http-rp/origin-when-cross-origin/iframe-tag.http.html
  • TIMEOUT [expected OK] /resource-timing/nested-context-navigations-iframe.html (#24311)
    • TIMEOUT [expected PASS] subtest: Test that iframe navigations are not observable by the parent, even after history navigations by the parent Test timed out
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent, even after history navigations by the parent
    • NOTRUN [expected PASS] subtest: Test that iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe navigations are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that iframe refreshes are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that crossorigin iframe refreshes are not observable by the parent
    • NOTRUN [expected PASS] subtest: Test that cross-site iframe refreshes are not observable by the parent
  • TIMEOUT /resource-timing/test_resource_timing.html (#25720)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img) assert_equals: expected 8702720 but got 8702976
  • OK [expected TIMEOUT] /webmessaging/without-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank
  • TIMEOUT [expected OK] /webmessaging/without-ports/018.html (#24485)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, javascript: Test timed out
Stable unexpected results that are known to be intermittent (16)
  • FAIL [expected PASS] /_mozilla/css/iframe/hide_and_show.html (#15265)
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-uniform-packing-restrictions.html (#28103)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected OK] /_webgl/conformance/glsl/misc/shader-with-non-reserved-words.html (#16216)
    • NOTRUN [expected PASS] subtest: Overall test
  • TIMEOUT [expected PASS] /css/css-color/animation/opacity-animation-ending-correctly-001.html (#29215)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-weight: '400' should prefer '400' over '450 460'
    • PASS [expected FAIL] subtest: Matching font-weight: '400' should prefer '450 460' over '500'
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '350 399' over '340 398'
    • PASS [expected FAIL] subtest: Matching font-weight: '430' should prefer '340 398' over '501 550'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '350 399' over '340 360'
    • PASS [expected FAIL] subtest: Matching font-weight: '399' should prefer '450 460' over '500 501'
    • PASS [expected FAIL] subtest: Matching font-style: 'italic' should prefer 'oblique 5deg 10deg' over 'oblique 5deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 20deg' should prefer 'oblique 20deg' over 'oblique 30deg 60deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 20deg' should prefer 'italic' over 'oblique 0deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique 21deg' should prefer 'oblique 21deg' over 'oblique 30deg 60deg'
    • And 3 more unexpected results...
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • FAIL [expected TIMEOUT] subtest: sec-fetch-site - Not sent to non-trustworthy same-site destination, no attributes promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
    • TIMEOUT [expected NOTRUN] subtest: sec-fetch-site - Not sent to non-trustworthy cross-site destination, no attributes Test timed out
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html (#28681)
    • PASS [expected FAIL] subtest: load & pageshow events do not fire on contentWindow of <iframe> element created with src='about:blank'
  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-window-open.html (#28691)
    • FAIL [expected PASS] subtest: load event does not fire on window.open('about:blank') assert_unreached: load should not be fired Reached unreachable code
  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
    • FAIL [expected TIMEOUT] subtest: Check that popups from a sandboxed iframe escape the sandbox if
      allow-popups-to-escape-sandbox is used assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
    • FAIL [expected NOTRUN] subtest: Check that popups from a sandboxed iframe do not escape the sandbox assert_equals: It came from a sandboxed iframe expected "null" but got "http://web-platform.test:8000"
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK [expected TIMEOUT] /html/semantics/forms/form-submission-0/reparent-form-during-planned-navigation-task.html (#29724)
    • PASS [expected TIMEOUT] subtest: reparent-form-during-planned-navigation-task
  • TIMEOUT /resource-timing/test_resource_timing.https.html (#25216)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img) assert_equals: expected 11902976 but got 11902720
  • OK [expected TIMEOUT] /webmessaging/with-ports/017.html (#24486)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, about:blank

Copy link

github-actions bot commented Mar 6, 2024

✨ Try run (#8175329291) succeeded.

Copy link
Member Author

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Remove spurious println

Copy link
Member

@atbrakhi atbrakhi left a comment

Choose a reason for hiding this comment

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

Thank you for walking me through it. It looks good

@mrobinson mrobinson added this pull request to the merge queue Mar 7, 2024
@mrobinson mrobinson removed this pull request from the merge queue due to a manual request Mar 7, 2024
@mrobinson mrobinson enabled auto-merge March 7, 2024 11:13
@mrobinson mrobinson added this pull request to the merge queue Mar 7, 2024
Merged via the queue into servo:main with commit 6005049 Mar 7, 2024
9 checks passed
@mrobinson mrobinson deleted the improve-frame-waiting-compositor branch March 7, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants