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

Fix an issue with the rooting of animating nodes #26503

Merged
merged 1 commit into from May 13, 2020

Conversation

@mrobinson
Copy link
Member

mrobinson commented May 13, 2020

Make sure nodes removed from map of rooted animating nodes are rooted on
the stack before triggering event handlers. We also make sure not to
call from_untrusted_node_address on nodes that aren't guaranteed to be
rooted.

Fixes #26498.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #26498.
  • There are tests for these changes
Make sure nodes removed from map of rooted animating nodes are rooted on
the stack before triggering event handlers. We also make sure not to
call `from_untrusted_node_address` on nodes that aren't guaranteed to be
rooted.

Fixes #26498.
@highfive
Copy link

highfive commented May 13, 2020

Heads up! This PR modifies the following files:

@highfive
Copy link

highfive commented May 13, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@mrobinson mrobinson requested a review from jdm May 13, 2020
@mrobinson
Copy link
Member Author

mrobinson commented May 13, 2020

This is a flaky crash, so there isn't a solid guarantee that this fixes the issue. I tried to be as careful as possible when confirming this though. Before this change the following test:

./mach test-wpt   /_mozilla/mozilla/transitionend_safety.html --repeat 100

always resulted in at least one crash. After the change, I could not reproduce the crash at all.

@jdm
Copy link
Member

jdm commented May 13, 2020

Thanks for investigating and figuring out a way to manually test it!
@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2020

📌 Commit c4a38be has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2020

Testing commit c4a38be with merge 9eee048...

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 9eee048 to master...

@bors-servo bors-servo merged commit 9eee048 into servo:master May 13, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@mrobinson mrobinson deleted the mrobinson:fix-rooting branch May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.