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

script: Move the layout_wrapper outside of script. #17744

Merged
merged 1 commit into from Jul 15, 2017

Conversation

@emilio
Copy link
Member

emilio commented Jul 15, 2017

This allows us to have ensure_data() and clear_data() functions on the TElement
trait, instead of hacking around it adding methods in random traits.

This also allows us to do some further cleanup, which I'd rather do in a
followup.


This change is Reviewable

@highfive
Copy link

highfive commented Jul 15, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/dom.rs, components/style/gecko/traversal.rs, components/style/gecko/wrapper.rs, components/style/traversal.rs
  • @canaltinova: components/style/dom.rs, components/style/gecko/traversal.rs, components/style/gecko/wrapper.rs, components/style/traversal.rs
  • @fitzgen: components/script/lib.rs, components/script/Cargo.toml, components/script/test.rs, components/script_layout_interface/wrapper_traits.rs, components/script/layout_wrapper.rs
  • @KiChjang: components/script/lib.rs, components/script/Cargo.toml, components/script/test.rs, components/script_layout_interface/wrapper_traits.rs, components/script/layout_wrapper.rs
@highfive
Copy link

highfive commented Jul 15, 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!
@emilio
Copy link
Member Author

emilio commented Jul 15, 2017

r? @bholley for the layout data bits, @nox for the script bits.

@highfive highfive assigned bholley and unassigned cbrewster Jul 15, 2017
@emilio
Copy link
Member Author

emilio commented Jul 15, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

Trying commit 9a933ef with merge 48ddb09...

bors-servo added a commit that referenced this pull request Jul 15, 2017
script: Move the layout_wrapper outside of script.

This allows us to have ensure_data() and clear_data() functions on the TElement
trait, instead of hacking around it adding methods in random traits.

This also allows us to do some further cleanup, which I'd rather do in a
followup.

<!-- 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/17744)
<!-- Reviewable:end -->
@emilio
Copy link
Member Author

emilio commented Jul 15, 2017

@bors-servo r=nox

  • Nox said this looked good to him, and the layout data side of things are just moving code around and the removal of the DomTraversal::ensure_element_data/clear_element_data, which were a hack.
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit 9a933ef has been approved by nox

@highfive highfive assigned nox and unassigned bholley Jul 15, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

Testing commit 9a933ef with merge 9b10aa89f5460919d8386077ffd55ad8381b9d44...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

💔 Test failed - mac-dev-unit

This allows us to have ensure_data() and clear_data() functions on the TElement
trait, instead of hacking around it adding methods in random traits.

This also allows us to do some further cleanup, which I'd rather do in a
followup.
@emilio emilio force-pushed the emilio:ensure-data branch from 9a933ef to bf9369b Jul 15, 2017
@emilio
Copy link
Member Author

emilio commented Jul 15, 2017

@bors-servo r=nox

  • Unit tests
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

📌 Commit bf9369b has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2017

Testing commit bf9369b with merge 3497bbb...

bors-servo added a commit that referenced this pull request Jul 15, 2017
script: Move the layout_wrapper outside of script.

This allows us to have ensure_data() and clear_data() functions on the TElement
trait, instead of hacking around it adding methods in random traits.

This also allows us to do some further cleanup, which I'd rather do in a
followup.

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

bors-servo commented Jul 15, 2017

@bors-servo bors-servo merged commit bf9369b into servo:master Jul 15, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bholley
Copy link
Contributor

bholley commented Jul 15, 2017

This is great. It also means that the servo-side fixup for style crate changes is more likely to happen in layout_thread, which has a much faster iteration time than script. Thanks emilio!

@emilio emilio deleted the emilio:ensure-data branch Jul 15, 2017
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

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