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

[Do Not Merge] Make DOM Manipulation events cancellable #12296

Closed
wants to merge 2 commits into from

Conversation

@cbrewster
Copy link
Member

cbrewster commented Jul 6, 2016

I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #11703 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

highfive commented Jul 6, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/storage.rs, components/script/dom/event.rs, components/script/dom/htmldetailselement.rs, components/script/dom/htmlformelement.rs, components/script/dom/htmlmediaelement.rs, components/script/dom/htmlimageelement.rs, components/script/script_thread.rs, components/script/task_source/dom_manipulation.rs
@highfive
Copy link

highfive commented Jul 6, 2016

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@cbrewster
Copy link
Member Author

cbrewster commented Jul 6, 2016

bors-servo added a commit that referenced this pull request Jul 6, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Trying commit 7bff05e with merge 2185c18...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - mac-rel-wpt

@cbrewster cbrewster force-pushed the cbrewster:dommanip_intermittent branch from 7bff05e to 3248f52 Jul 6, 2016
@highfive highfive removed the S-tests-failed label Jul 6, 2016
@cbrewster cbrewster force-pushed the cbrewster:dommanip_intermittent branch from 3248f52 to aaba316 Jul 6, 2016
@cbrewster
Copy link
Member Author

cbrewster commented Jul 6, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Trying commit aaba316 with merge 1b88d31...

bors-servo added a commit that referenced this pull request Jul 6, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

@cbrewster
Copy link
Member Author

cbrewster commented Jul 6, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Trying commit aaba316 with merge f91a158...

bors-servo added a commit that referenced this pull request Jul 6, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - linux-rel

@highfive
Copy link

highfive commented Jul 6, 2016

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-table-007.htm
  └   → /css-transforms-1_dev/html/transform-table-007.htm a5c014b20ef1363bea6f24eda28c7efb7c45698a
/css-transforms-1_dev/html/reference/transform-blank-ref.htm fa6407b1acbbfea27e27061e7d1bdeca98e4a728
Testing a5c014b20ef1363bea6f24eda28c7efb7c45698a == fa6407b1acbbfea27e27061e7d1bdeca98e4a728

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform3d-translatez-001.htm
  └   → /css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-ref.htm b14318ccfd8a59b7b249d41521e178d617678823
Testing 902d90a8d198625335e738587fdfe81dcc90392d == b14318ccfd8a59b7b249d41521e178d617678823
/css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-notref.htm 902d90a8d198625335e738587fdfe81dcc90392d
Testing 902d90a8d198625335e738587fdfe81dcc90392d != 902d90a8d198625335e738587fdfe81dcc90392d
@cbrewster
Copy link
Member Author

cbrewster commented Jul 6, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Trying commit aaba316 with merge 439128f...

bors-servo added a commit that referenced this pull request Jul 6, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

@notriddle
Copy link
Contributor

notriddle commented Jul 7, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

Trying commit aaba316 with merge 49da17d...

bors-servo added a commit that referenced this pull request Jul 7, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

@cbrewster
Copy link
Member Author

cbrewster commented Jul 7, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

Trying commit aaba316 with merge 34dbe1c...

bors-servo added a commit that referenced this pull request Jul 7, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

💔 Test failed - mac-rel-wpt

@highfive
Copy link

highfive commented Jul 7, 2016

  ▶ TIMEOUT [expected OK] /html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
@cbrewster
Copy link
Member Author

cbrewster commented Jul 7, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

Trying commit aaba316 with merge 93aae44...

bors-servo added a commit that referenced this pull request Jul 7, 2016
[Do Not Merge] Make DOM Manipulation events cancellable

<!-- Please describe your changes on the following line: -->
I can't reproduce the intermittent failure on #11703 so I am running some tests using buildbot.
If this does fix the intermittent, this is blocked on #12292.

---
<!-- 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 #11703 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12296)
<!-- Reviewable:end -->
@cbrewster
Copy link
Member Author

cbrewster commented Jul 7, 2016

I have not found a way to write a test to reproduce this (although I have found many other panics along the way). Any ideas would be appreciated. The test should somehow fire a DOMManipulation event to a target whose script thread is shut down.

@bors-servo
Copy link
Contributor

bors-servo commented Jul 7, 2016

💔 Test failed - mac-rel-wpt

@KiChjang
Copy link
Member

KiChjang commented Jul 7, 2016

Looks like the last try run has been killed by the OS.

@cbrewster cbrewster force-pushed the cbrewster:dommanip_intermittent branch from aaba316 to 0791710 Jul 8, 2016
@highfive highfive removed the S-tests-failed label Jul 8, 2016
@KiChjang
Copy link
Member

KiChjang commented Jul 8, 2016

Can we reduce the change_parentage test down into a minimal case where the bug before this patch can deterministically happen?

@cbrewster
Copy link
Member Author

cbrewster commented Jul 8, 2016

@KiChjang I have not been able to do so, I can't even get change_parentage to crash locally after 400+ test runs.

@cbrewster
Copy link
Member Author

cbrewster commented Jul 12, 2016

Closing in favor of #12404 as it makes all tasks cancellable.

@cbrewster cbrewster closed this Jul 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.