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

rule tree: Avoid yet another dumb assertion when dropping the rule tree. #14233

Closed
wants to merge 1 commit into from

Conversation

@emilio
Copy link
Member

emilio commented Nov 15, 2016

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth


This change is Reviewable

@highfive
Copy link

highfive commented Nov 15, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/rule_tree/mod.rs
@highfive
Copy link

highfive commented Nov 15, 2016

warning Warning warning

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

SimonSapin commented Nov 15, 2016

Could you copy that comment next to this let me = … ? r=me with that.

@emilio emilio force-pushed the emilio:rule-tree-assert branch from 8cff46c to 667b362 Nov 15, 2016
@emilio
Copy link
Member Author

emilio commented Nov 15, 2016

@bors-servo r=SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Nov 15, 2016

📌 Commit 667b362 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Nov 17, 2016

Testing commit 667b362 with merge 702e807...

bors-servo added a commit that referenced this pull request Nov 17, 2016
rule tree: Avoid yet another dumb assertion when dropping the rule tree.

<!-- Please describe your changes on the following line: -->

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth

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

bors-servo commented Nov 17, 2016

💔 Test failed - linux-rel-wpt

@heycam
Copy link
Member

heycam commented Nov 17, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Nov 17, 2016

Testing commit 667b362 with merge e750743...

bors-servo added a commit that referenced this pull request Nov 17, 2016
rule tree: Avoid yet another dumb assertion when dropping the rule tree.

<!-- Please describe your changes on the following line: -->

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth

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

bors-servo commented Nov 17, 2016

💔 Test failed - linux-rel-css

@heycam
Copy link
Member

heycam commented Nov 17, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Nov 17, 2016

Testing commit 667b362 with merge be09c0a...

bors-servo added a commit that referenced this pull request Nov 17, 2016
rule tree: Avoid yet another dumb assertion when dropping the rule tree.

<!-- Please describe your changes on the following line: -->

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth

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

bors-servo commented Nov 17, 2016

💔 Test failed - mac-rel-wpt1

@SimonSapin
Copy link
Member

SimonSapin commented Nov 17, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 17, 2016

Testing commit 667b362 with merge e4089c1...

bors-servo added a commit that referenced this pull request Nov 17, 2016
rule tree: Avoid yet another dumb assertion when dropping the rule tree.

<!-- Please describe your changes on the following line: -->

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth

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

bors-servo commented Nov 17, 2016

The build was interrupted to prioritize another pull request.

@Manishearth
Copy link
Member

Manishearth commented Nov 17, 2016

bors-servo added a commit that referenced this pull request Nov 17, 2016
rule tree: Avoid yet another dumb assertion when dropping the rule tree.

<!-- Please describe your changes on the following line: -->

There's a comment in the caller (where we do the same) saying why this can assert (if this runs from the rule tree root destructor).

This doesn't happen in servo's ci because we gc all the time, but can happen when abruptly shutting down the document.

r? @SimonSapin or @heycam or @Manishearth

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

bors-servo commented Nov 17, 2016

Testing commit 667b362 with merge 9728b18...

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 18, 2016

@bors-servo force

@Ms2ger Ms2ger closed this Nov 18, 2016
@heycam
Copy link
Member

heycam commented Nov 18, 2016

Will try landing this commit as part of #14256.

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.

None yet

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