incremental restyle: Hoist most styling functionality from TNode to TElement #13956

bholley · 2016-10-28T07:32:20Z

This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.

This change is

highfive · 2016-10-28T07:32:24Z

Heads up! This PR modifies the following files:

@KiChjang: components/script_layout_interface/wrapper_traits.rs, components/script/layout_wrapper.rs
@fitzgen: components/script_layout_interface/wrapper_traits.rs, components/script/layout_wrapper.rs
@emilio: components/layout/fragment.rs, components/layout/traversal.rs, ports/geckolib/glue.rs, components/style/parallel.rs, components/layout/wrapper.rs, components/style/gecko/traversal.rs, components/style/sequential.rs, components/layout/construct.rs, components/style/dom.rs, components/style/gecko/wrapper.rs, components/layout/query.rs, components/layout/lib.rs, components/style/traversal.rs, components/style/matching.rs

highfive · 2016-10-28T07:32:25Z

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 · 2016-10-28T07:40:36Z

@bors-servo try

bors-servo · 2016-10-28T07:40:38Z

⌛ Trying commit 37f05cc with merge 6f4d957...


        Auto merge of #13956 - bholley:more_telement, r=<try>

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bholley · 2016-10-28T07:40:56Z

r? @emilio

(Review #13951 first)

bors-servo · 2016-10-28T08:11:32Z

💔 Test failed - linux-rel-wpt

emilio

The idea looks good to me, but I need to check with much more care before stamping this.

emilio · 2016-10-28T08:27:54Z

components/layout/wrapper.rs

-            self.init_style_and_layout_data(opaque);
-        }
+impl<T: GetLayoutData> LayoutNodeHelpers for T {
+    fn ensure_layout_data(&self) -> NonOpaqueStyleAndLayoutData {


So I'm slightly scared about this change in the sense that now calling borrow_layout_data can trigger a non thread-safe allocation, so I'll have to check this much more carefully.

Great point. This observation escaped me in my post-midnight haze of getting this over the line. Let me take a crack at fixing this somehow.

emilio · 2016-10-28T08:27:54Z

components/layout_thread/lib.rs

@@ -1111,8 +1111,8 @@ impl LayoutThread {

                        let mut current = node.parent_node();
                        while let Some(node) = current {
-                            if node.has_dirty_descendants() { break; }
-                            unsafe { node.set_dirty_descendants(); }
+                            if node.is_element() && node.as_element().unwrap().has_dirty_descendants() { break; }


Given we're checking on parent_node, this is guaranteed to be an element, right?

Good point, will fix.

emilio · 2016-10-28T08:28:29Z

@bors-servo: retry

bash: tests/wpt/run.sh: No such file or directory?


        Auto merge of #13956 - bholley:more_telement, r=<try>

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bors-servo · 2016-10-28T08:28:32Z

⌛ Trying commit 37f05cc with merge d42c64a...

bors-servo · 2016-10-28T09:27:59Z

💔 Test failed - linux-rel-wpt

bholley · 2016-10-28T16:10:32Z

bash: tests/wpt/run.sh: No such file or directory

@bors-servo retry

bors-servo · 2016-10-28T16:10:35Z

⌛ Trying commit 37f05cc with merge 904c825...


        Auto merge of #13956 - bholley:more_telement, r=<try>

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bors-servo · 2016-10-28T16:41:34Z

💔 Test failed - linux-rel-wpt

larsbergstrom · 2016-10-28T18:29:35Z

@bors-servo retry

Logs were busted, picked up servo/saltfs#525

bors-servo · 2016-10-28T18:30:31Z

⌛ Trying commit 37f05cc with merge 1dcb061...


        Auto merge of #13956 - bholley:more_telement, r=<try>

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bors-servo · 2016-10-28T18:30:38Z

💔 Test failed - mac-dev-unit

bholley · 2016-10-29T20:46:05Z

@bors-servo r=emilio

Thanks for the review!

bors-servo · 2016-10-29T20:46:06Z

📌 Commit 71ee1bd has been approved by emilio

bholley · 2016-10-29T20:46:37Z

@bors-servo p=1

More refactorings on top.

bors-servo · 2016-10-29T20:55:03Z

⌛ Testing commit 71ee1bd with merge 59e651b...


        Auto merge of #13956 - bholley:more_telement, r=emilio

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bors-servo · 2016-10-29T20:58:35Z

💔 Test failed - linux-dev

jdm · 2016-10-29T21:06:56Z

error[E0133]: dereference of raw pointer requires unsafe function or block
   --> /home/servo/buildbot/slave/linux-dev/build/components/script/layout_wrapper.rs:560:14
    |
560 |             &**d.ptr
    |              ^^^^^^^ dereference of raw pointer

error: aborting due to previous error


        Rename NodeData and associated data structures to Element*.

MozReview-Commit-ID: 96VsmsoZtjZ

bholley · 2016-10-29T21:58:33Z

Whoops - I forget it was a raw pointer and not Deref impls.

@bors-servo r=emilio

bors-servo · 2016-10-29T21:58:33Z

📌 Commit be89f73 has been approved by emilio

bors-servo · 2016-10-29T22:14:11Z

⌛ Testing commit be89f73 with merge b611e5c...


        Auto merge of #13956 - bholley:more_telement, r=emilio

incremental restyle: Hoist most styling functionality from TNode to TElement This is a continuation of the work in #13951. I'm separating it out into a separate PR since the aforementioned patch has a green try run and this one doesn't yet.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13956)

bors-servo · 2016-10-29T23:15:21Z

💔 Test failed - linux-rel-wpt

emilio · 2016-10-29T23:29:18Z

@bors-servo: retry

#13360 + an IO error.

bors-servo · 2016-10-30T00:17:24Z

⚡ Previous build results for arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-wpt...

bors-servo · 2016-10-30T00:48:25Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

highfive assigned larsbergstrom Oct 28, 2016

highfive added the S-awaiting-review label Oct 28, 2016

highfive assigned emilio and unassigned larsbergstrom Oct 28, 2016

highfive added the S-tests-failed label Oct 28, 2016

emilio reviewed Oct 28, 2016

View changes

highfive added the S-tests-failed label Oct 28, 2016

bholley mentioned this pull request Oct 28, 2016

incremental restyle: Hoist more stuff to Element #13951

Merged

highfive added the S-tests-failed label Oct 28, 2016

bholley force-pushed the bholley:more_telement branch from 37f05cc to 776f781 Oct 28, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Oct 29, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Oct 29, 2016

Rename NodeData and associated data structures to Element*.

Loading status checks…

be89f73

MozReview-Commit-ID: 96VsmsoZtjZ

bholley force-pushed the bholley:more_telement branch from 71ee1bd to be89f73 Oct 29, 2016

highfive added S-awaiting-review and removed S-tests-failed labels Oct 29, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Oct 29, 2016

jdm mentioned this pull request Oct 29, 2016

Buildbot sometimes checks out the wrong revision for try runs servo/saltfs#530

Closed

highfive added S-tests-failed and removed S-awaiting-merge labels Oct 29, 2016

bors-servo merged commit be89f73 into servo:master Oct 30, 2016
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 deleted the bholley:more_telement branch Oct 30, 2016

servo / servo

incremental restyle: Hoist most styling functionality from TNode to TElement #13956

incremental restyle: Hoist most styling functionality from TNode to TElement #13956

bholley commented Oct 28, 2016 •

edited by larsbergstrom

highfive commented Oct 28, 2016

highfive commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

emilio left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

larsbergstrom commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

jdm commented Oct 29, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

emilio commented Oct 29, 2016

bors-servo commented Oct 30, 2016

bors-servo commented Oct 30, 2016

servo / servo

Join GitHub today

incremental restyle: Hoist most styling functionality from TNode to TElement #13956

incremental restyle: Hoist most styling functionality from TNode to TElement #13956

Conversation

bholley commented Oct 28, 2016 • edited by larsbergstrom

highfive commented Oct 28, 2016

highfive commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

emilio left a comment

This comment has been minimized.

emilio Oct 28, 2016

This comment has been minimized.

bholley Oct 28, 2016

This comment has been minimized.

emilio Oct 28, 2016

This comment has been minimized.

bholley Oct 28, 2016

emilio commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

larsbergstrom commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bors-servo commented Oct 28, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

jdm commented Oct 29, 2016

bholley commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

bors-servo commented Oct 29, 2016

emilio commented Oct 29, 2016

bors-servo commented Oct 30, 2016

bors-servo commented Oct 30, 2016

bholley commented Oct 28, 2016 •

edited by larsbergstrom