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

script: implement AbortController #31361

Merged
merged 6 commits into from
May 4, 2024
Merged

Conversation

syvb
Copy link
Contributor

@syvb syvb commented Feb 16, 2024

Adds basic support for AbortController/AbortSignal, and implements the signal parameter to addEventListener to support cancelling event listeners with AbortSignal.

This PR doesn't implement AbortSignal dependent signals or fetch cancellation.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

Comment on lines -2271 to +2274
extras += [descriptor.path, descriptor.bindingPath]
if current_name != descriptor.ifaceName:
extras += [descriptor.path, descriptor.bindingPath]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug in binding generation - AbortSignal both implements EventTarget and is part of AddEventListenerOptions in EventTarget. This resulted in the binding generation trying to make EventTarget import itself.

@syvb syvb changed the title Implement AbortController script: implement AbortController Feb 21, 2024
@jdm
Copy link
Member

jdm commented Mar 10, 2024

Sorry about the delay here. I've started reviewing these changes and I'll try to finish it up later today!

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

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

I got sick in the meantime, so apologies for the additional delay. This looks good!

components/script/dom/abortcontroller.rs Outdated Show resolved Hide resolved
components/script/dom/abortcontroller.rs Outdated Show resolved Hide resolved
@syvb syvb force-pushed the impl-abort-controller branch 2 times, most recently from 859a637 to 6fba319 Compare March 21, 2024 03:54
@syvb syvb requested a review from jdm March 21, 2024 03:54
@jdm jdm added this pull request to the merge queue Mar 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 21, 2024
@mrobinson
Copy link
Member

It looks like this has modified the behavior of some tests that use the BFCache?

@syvb syvb force-pushed the impl-abort-controller branch 2 times, most recently from 658d888 to e734047 Compare April 2, 2024 22:02
@jdm jdm added the T-linux-wpt-2020 Do a try run of the WPT label Apr 2, 2024
@github-actions github-actions bot removed the T-linux-wpt-2020 Do a try run of the WPT label Apr 2, 2024
Copy link

github-actions bot commented Apr 2, 2024

🔨 Triggering try run (#8530709733) for Linux WPT

@syvb
Copy link
Contributor Author

syvb commented Apr 2, 2024

I just updated the expectations so that the tests should pass now.

bors try


try run in my repo, since I can't run try here: https://github.com/syvb/servo/actions/runs/8531001484

Copy link

github-actions bot commented Apr 3, 2024

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

Flaky unexpected result (16)
  • FAIL [expected PASS] /_mozilla/css/dirty_viewport.html (#13731)
  • PASS [expected FAIL] /css/css-sizing/dynamic-available-size-iframe.html (#31559)
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • PASS [expected FAIL] subtest: listeners are called correct number of times
  • TIMEOUT /fetch/fetch-later/new-window.tentative.https.window.html
    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='0'] can trigger fetchLater.

      Test timed out
      

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='1'] can trigger fetchLater.

      Test timed out
      

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target=''][features='2'] can trigger fetchLater.

      Test timed out
      

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='0'] can trigger fetchLater.

      Test timed out
      

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='1'] can trigger fetchLater.

      Test timed out
      

    • TIMEOUT [expected FAIL] subtest: A same-origin window[target='_blank'][features='2'] can trigger fetchLater.

      Test timed out
      

  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/history-traversal/srcdoc/consecutive-srcdoc.html (#29084)
    • TIMEOUT [expected FAIL] subtest: changing srcdoc to about:srcdoc#yo then another srcdoc does two push navigations and we can navigate back

      Test timed out
      

  • OK /html/browsers/history/the-history-interface/traverse_the_history_2.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected TIMEOUT] subtest: opener of discarded auxiliary browsing context

      assert_equals: opener after removal expected null but got object "[object Window]"
      

  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • TIMEOUT [expected OK] /html/semantics/links/links-created-by-a-and-area-elements/htmlanchorelement_noopener.html (#23205)
    • NOTRUN [expected FAIL] subtest: Check that rel=noopener with target=_self does a normal load
  • OK /html/syntax/parsing/DOMContentLoaded-defer.html (#21550)
    • PASS [expected FAIL] subtest: The end: DOMContentLoaded and defer scripts
  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-cross-origin-bfcache.tentative.window.html
    • FAIL [expected TIMEOUT] subtest: RemoteContextHelper navigation using BFCache

      assert_equals: document unexpectedly BFCached expected (undefined) undefined but got (boolean) true
      

  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-navigation-failure.tentative.window.html
    • FAIL [expected TIMEOUT] subtest: Ensure that navigation failure blocks bfcache and gets recorded.

      assert_equals: document unexpectedly BFCached expected (undefined) undefined but got (boolean) true
      

  • 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.https.html (#25216)
    • PASS [expected FAIL] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)
  • OK /workers/WorkerGlobalScope-close.html (#23064)
    • PASS [expected FAIL] subtest: Test sending a message after closing.
Stable unexpected results that are known to be intermittent (15)
  • TIMEOUT /FileAPI/url/url-in-tags-revoke.window.html (#19978)
    • TIMEOUT [expected FAIL] subtest: Opening a blob URL in a new window immediately before revoking it works.

      Test timed out
      

  • FAIL [expected PASS] /_mozilla/css/iframe/hide_and_show.html (#15265)
  • OK /css/css-fonts/variations/at-font-face-font-matching.html (#20684)
    • PASS [expected FAIL] subtest: Matching font-stretch: '90%' should prefer '60% 70%' over '110% 140%'
    • PASS [expected FAIL] subtest: Matching font-style: 'normal' should prefer 'oblique 10deg 40deg' over 'oblique 20deg 30deg'
    • PASS [expected FAIL] subtest: Matching font-style: 'oblique -10deg' should prefer 'oblique -50deg -40deg' over 'italic'
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • PASS [expected TIMEOUT] subtest: background-image sec-fetch-site - HTTPS downgrade (header not sent)
  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.https.sub.html (#30111)
    • FAIL [expected TIMEOUT] subtest: sec-fetch-site - Cross-site, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

    • TIMEOUT [expected NOTRUN] subtest: sec-fetch-site - Same site, no attributes

      Test timed out
      

  • TIMEOUT /fetch/metadata/generated/element-img-environment-change.sub.html (#30111)
    • FAIL [expected PASS] subtest: sec-fetch-site - Not sent to non-trustworthy same-origin destination, no attributes

      promise_test: Unhandled rejection with value: object "Error: Failed to query for recorded headers."
      

  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • PASS [expected FAIL] subtest: Check execution order on load handler
    • PASS [expected FAIL] subtest: Check execution order from nested timeout
  • 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=''
  • CRASH [expected TIMEOUT] /html/dom/documents/dom-tree-accessors/Document.currentScript.html (#22423)
  • TIMEOUT [expected OK] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/media-elements/track/track-element/no-cuechange-before-play.html (#31014)
    • TIMEOUT [expected FAIL] subtest: Ensure that the 'cuechange' event is not fired before video playback has begun.

      Test timed out
      

  • OK [expected CRASH] /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"
      

  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
  • OK /html/semantics/forms/form-submission-0/text-plain.window.html (#28687)
    • FAIL [expected PASS] subtest: text/plain: 0x00 in value (formdata event)

      assert_equals: expected "a=b\0c\r\n" but got ""
      

    • PASS [expected FAIL] subtest: text/plain: single quote in name (normal form)
  • OK [expected TIMEOUT] /webmessaging/with-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
Stable unexpected results (2)
  • OK /dom/idlharness.window.html?exclude=Node
    • PASS [expected FAIL] subtest: AbortController interface: existence and properties of interface object
    • PASS [expected FAIL] subtest: AbortController interface object length
    • PASS [expected FAIL] subtest: AbortController interface object name
    • PASS [expected FAIL] subtest: AbortController interface: existence and properties of interface prototype object
    • PASS [expected FAIL] subtest: AbortController interface: existence and properties of interface prototype object's "constructor" property
    • PASS [expected FAIL] subtest: AbortController interface: existence and properties of interface prototype object's @@unscopables property
    • PASS [expected FAIL] subtest: AbortController interface: attribute signal
    • PASS [expected FAIL] subtest: AbortController interface: operation abort(optional any)
    • PASS [expected FAIL] subtest: AbortController must be primary interface of new AbortController()
    • PASS [expected FAIL] subtest: Stringification of new AbortController()
    • And 28 more unexpected results...
  • TIMEOUT [expected OK] /fetch/fetch-later/send-on-deactivate.tentative.https.window.html
    • TIMEOUT [expected FAIL] subtest: Call fetchLater() when BFCached with activateAfter=0 sends immediately.

      Test timed out
      

Copy link

github-actions bot commented Apr 3, 2024

⚠️ Try run (#8530709733) failed.

@syvb
Copy link
Contributor Author

syvb commented May 1, 2024

I rebased the PR on main, it should pass tests now.

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

github-actions bot commented May 1, 2024

🔨 Triggering try run (#8906780806) for Linux WPT

Copy link

github-actions bot commented May 1, 2024

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

Flaky unexpected result (19)
  • FAIL [expected PASS] /_mozilla/css/iframe/hide_and_show.html (#15265)
  • CRASH [expected PASS] /_webgl/conformance/glsl/bugs/nested-functions-should-not-crash.html (#30680)
  • TIMEOUT [expected PASS] /encoding/streams/stringification-crash.html
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK [expected TIMEOUT] /html/infrastructure/urls/base-url/document-base-url-window-initiator-is-not-opener.https.window.html (#30970)
  • TIMEOUT [expected CRASH] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-3.html (#24057)
    • TIMEOUT [expected FAIL] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      Test timed out
      

  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
    • 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"
      

  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.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 OK] /html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html (#29634)
    • TIMEOUT [expected PASS] subtest: Verifies that location navigations take precedence when following form submissions.

      Test timed out
      

  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • PASS [expected FAIL] subtest: multipart/form-data: 0x00 in name (normal form)
  • 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
  • CRASH [expected OK] /html/semantics/forms/the-fieldset-element/disabled-003.html (#31730)
  • ERROR [expected OK] /html/semantics/scripting-1/the-script-element/defer-script/async-script.html?reload (#29054)
  • OK /html/webappapis/dynamic-markup-insertion/document-write/module-static-import-delayed.html (#26243)
    • FAIL [expected PASS] subtest: document.write in an imported module

      assert_true: onload must be called expected true got false
      

  • 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
      

  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-same-origin-bfcache.tentative.window.html
    • FAIL [expected TIMEOUT] subtest: RemoteContextHelper navigation using BFCache

      assert_equals: document unexpectedly BFCached expected (undefined) undefined but got (boolean) true
      

  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-same-origin-replace.tentative.window.html
    • FAIL [expected TIMEOUT] subtest: RemoteContextHelper navigation using BFCache

      promise_test: Unhandled rejection with value: object "TypeError: result is undefined"
      

  • OK [expected TIMEOUT] /webmessaging/without-ports/018.html (#24485)
    • PASS [expected TIMEOUT] subtest: origin of the script that invoked the method, javascript:
Stable unexpected results that are known to be intermittent (17)
  • FAIL [expected PASS] /_mozilla/gfx-rs-gecko/descriptor-ranges.html (#23258)
  • OK /css/css-fonts/variations/font-weight-matching.html (#20686)
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 99
    • FAIL [expected PASS] subtest: Test @font-face matching for weight 249

      assert_approx_equals: @font-face should be mapped to CSSTest Weights 900. expected 90 +/- 2 but got 180
      

    • PASS [expected FAIL] subtest: Test @font-face matching for weight 250
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 400
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 500
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 750
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 900
    • PASS [expected FAIL] subtest: Test @font-face matching for weight 1000
  • OK /css/cssom-view/MediaQueryList-addListener-removeListener.html (#24569)
    • PASS [expected FAIL] subtest: listeners are called correct number of times
  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • PASS [expected FAIL] subtest: Check execution order on load handler
    • PASS [expected FAIL] subtest: Check execution order from nested timeout
  • 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=''
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/javascript-url-referrer.window.html (#29081)
    • TIMEOUT [expected PASS] subtest: no-referrer referrer policy used to create the starting page

      Test timed out
      

  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.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])
      

  • TIMEOUT [expected OK] /html/browsers/history/the-history-interface/traverse_the_history_write_onload_1.html (#21581)
    • TIMEOUT [expected PASS] subtest: Traverse the history when a history entry is written in the load event

      Test timed out
      

  • OK [expected CRASH] /html/browsers/windows/embedded-opener-remove-frame.html (#23867)
    • FAIL [expected TIMEOUT] subtest: opener of discarded auxiliary browsing context

      assert_object_equals: property "get" expected function "function opener() {
          [native code]
      }" got function "function opener() {
          [native code]
      }"
      

  • CRASH [expected TIMEOUT] /html/dom/documents/dom-tree-accessors/Document.currentScript.html (#22423)
  • OK [expected TIMEOUT] /html/semantics/embedded-content/media-elements/track/track-element/no-cuechange-before-play.html (#31014)
    • FAIL [expected TIMEOUT] subtest: Ensure that the 'cuechange' event is not fired before video playback has begun.

      assert_true: Not expecting event, but got canplaythrough event expected true got false
      

  • CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html (#24066)
  • OK /html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute-width-1000px.html (#21666)
    • FAIL [expected PASS] subtest: <img srcset="/images/green-1x1.png?e38 50w, /images/green-16x16.png?e38 51w" sizes="(min-width:calc(0)) 1px"> ref sizes="1px" (width:1000px)

      assert_equals: expected "http://web-platform.test:8000/images/green-1x1.png" but got "http://web-platform.test:8000/images/green-16x16.png"
      

  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • FAIL [expected PASS] subtest: application/x-www-form-urlencoded: 0x00 in name (normal form)

      assert_equals: expected "a%00b=c" but got ""
      

    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: \n in name (normal form)
  • TIMEOUT /resource-timing/test_resource_timing.html (#25720)
    • FAIL [expected PASS] subtest: PerformanceEntry has correct name, initiatorType, startTime, and duration (img)

      assert_equals: expected 7321088 but got 7320832
      

  • TIMEOUT [expected OK] /webmessaging/with-ports/017.html (#24486)
    • TIMEOUT [expected PASS] subtest: origin of the script that invoked the method, about:blank

      Test timed out
      

Stable unexpected results (1)
  • OK [expected TIMEOUT] /performance-timeline/not-restored-reasons/performance-navigation-timing-navigation-failure.tentative.window.html
    • FAIL [expected TIMEOUT] subtest: Ensure that navigation failure blocks bfcache and gets recorded.

      assert_equals: document unexpectedly BFCached expected (undefined) undefined but got (boolean) true
      

Copy link

github-actions bot commented May 1, 2024

⚠️ Try run (#8906780806) failed.

@syvb
Copy link
Contributor Author

syvb commented May 2, 2024

That failure is flakey, I created an issue for it: #32213

@jdm jdm added this pull request to the merge queue May 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 3, 2024
@jdm jdm added this pull request to the merge queue May 4, 2024
Merged via the queue into servo:main with commit 7fce850 May 4, 2024
9 checks passed
sagudev added a commit that referenced this pull request May 7, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 7, 2024
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

3 participants