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

Accumulate parent elements that need selector bits set on the ThreadLocalStyleContext #15462

Merged
merged 3 commits into from Feb 9, 2017

Conversation

@bholley
Copy link
Contributor

bholley commented Feb 9, 2017

Discussion and review in https://bugzilla.mozilla.org/show_bug.cgi?id=1336646


This change is Reviewable

bholley added 2 commits Feb 6, 2017
… a mutable StyleContext into match_element. r=emilio

We need to do something here to avoid a double-borrow when passing a mutable
StyleContext to match_element. After some discussion on IRC, we decided that
building the bloom filter for this case is probably worthwhile.
This makes things more consistent between the parallel and sequential traversal drivers.
@highfive
Copy link

highfive commented Feb 9, 2017

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/node.rs, components/script/layout_wrapper.rs, components/script_layout_interface/wrapper_traits.rs, components/script/dom/bindings/trace.rs, components/script/dom/element.rs
  • @fitzgen: components/script/dom/node.rs, components/script/layout_wrapper.rs, components/script_layout_interface/wrapper_traits.rs, components/script/dom/bindings/trace.rs, components/script/dom/element.rs
  • @emilio: components/style/restyle_hints.rs, components/style/parallel.rs, components/style/bloom.rs, components/style/stylist.rs, components/style/gecko/wrapper.rs, components/layout/query.rs, components/style/matching.rs, components/style/traversal.rs, components/style/context.rs, components/style/dom.rs
@highfive
Copy link

highfive commented Feb 9, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style, layout, and script code, but no tests are modified. Please consider adding a test!
@bholley
Copy link
Contributor Author

bholley commented Feb 9, 2017

Note that this PR touches the in-tree selectors, and the decision on that is not finalized until friday. If the decision ends up being that we keep selectors out-of-tree, I will apply this change upstream.

@bholley
Copy link
Contributor Author

bholley commented Feb 9, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Feb 9, 2017

Trying commit 2c8e346 with merge 818bc7c...

bors-servo added a commit that referenced this pull request Feb 9, 2017
Accumulate parent elements that need selector bits set on the ThreadLocalStyleContext

Discussion and review in https://bugzilla.mozilla.org/show_bug.cgi?id=1336646

<!-- 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/15462)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 9, 2017

💔 Test failed - mac-dev-unit

@bholley bholley force-pushed the bholley:accumulate_selector_flags branch from 2c8e346 to 9e860df Feb 9, 2017
@highfive highfive removed the S-tests-failed label Feb 9, 2017
@bholley
Copy link
Contributor Author

bholley commented Feb 9, 2017

This shrunk Element and all its subclasses by 8 bytes. Adjusting test expectations.

@bholley
Copy link
Contributor Author

bholley commented Feb 9, 2017

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

bors-servo commented Feb 9, 2017

📌 Commit 9e860df has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Feb 9, 2017

Testing commit 9e860df with merge cbfd446...

bors-servo added a commit that referenced this pull request Feb 9, 2017
Accumulate parent elements that need selector bits set on the ThreadLocalStyleContext

Discussion and review in https://bugzilla.mozilla.org/show_bug.cgi?id=1336646

<!-- 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/15462)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 9, 2017

@bors-servo bors-servo merged commit 9e860df into servo:master Feb 9, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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

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