Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upDispose layout data for every node removed from the tree #6815
Conversation
highfive
commented
Jul 28, 2015
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @glennw (or someone else) soon. |
|
Based way to ensure this is doing what we think right now is to add a println! to the dispose method that prints the pointer being disposed, and see what happens before and after this change when disconnecting a node with a child from the document. |
|
-S-awaiting-review +S-needs-code-changes Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. components/script/dom/node.rs, line 375 [r1] (raw file): Comments from the review on Reviewable.io |
|
Reviewed 1 of 1 files at r1. Comments from the review on Reviewable.io |
2760271
to
072609a
|
Fixed the extraneous code, tested as you suggested and saw the expected results. My test approach is covered in this paste: http://pastebin.com/LqxLxbT3 Thanks! |
|
@bors-servo: r+ Thanks for the patch! Reviewed 1 of 1 files at r2. Comments from the review on Reviewable.io |
|
@bors-servo r=jdm |
|
|
Dispose layout data for every node removed from the tree Fix for #6754. cc @jdm – I believe this is all that's required for the fix, but until I get a better sense of #6813, I'm unsure of the best way to test this. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6815) <!-- Reviewable:end -->
nick-thompson commentedJul 28, 2015
Fix for #6754.
cc @jdm – I believe this is all that's required for the fix, but until I get a better sense of #6813, I'm unsure of the best way to test this.