style: Move the StyleSheetSet into the Stylist. #18170

emilio · 2017-08-21T13:19:01Z

This will allow tracking whether there have been only additions to the
stylesheet set, and in that case don't destroy and completely rebuild the
invalidation map.

This is on top of #18143.

This change is

emilio · 2017-08-21T13:19:08Z

@bors-servo try

bors-servo · 2017-08-21T13:19:13Z

⌛ Trying commit 6f7bedf with merge 68aead0...


        Auto merge of #18170 - emilio:stylist-stylesheet-set, r=<try>

style: Move the StyleSheetSet into the Stylist. This will allow tracking whether there have been only additions to the stylesheet set, and in that case don't destroy and completely rebuild the invalidation map. This is on top of #18143.

highfive · 2017-08-21T13:21:35Z

Heads up! This PR modifies the following files:

@bholley: components/style/gecko/data.rs, components/style/stylesheets/stylesheet.rs, components/style/stylesheet_set.rs, components/style/stylist.rs, components/style/animation.rs and 1 more
@canaltinova: components/style/gecko/data.rs, components/style/stylesheets/stylesheet.rs, components/style/stylesheet_set.rs, components/style/stylist.rs, components/style/animation.rs and 1 more

highfive · 2017-08-21T13:24:07Z

Warning

These commits modify unsafe code. Please review it carefully!

bors-servo · 2017-08-21T13:47:15Z

💔 Test failed - linux-dev

emilio · 2017-08-21T14:07:01Z

@bors-servo try

bors-servo · 2017-08-21T14:07:05Z

⌛ Trying commit ed30bbe with merge 2aed2a0...


        Auto merge of #18170 - emilio:stylist-stylesheet-set, r=<try>

style: Move the StyleSheetSet into the Stylist. This will allow tracking whether there have been only additions to the stylesheet set, and in that case don't destroy and completely rebuild the invalidation map. This is on top of #18143.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18170)

emilio · 2017-08-21T14:10:58Z

r? @SimonSapin (this is green on Gecko, and I expect too on Stylo)

emilio · 2017-08-21T14:11:04Z

(or, @heycam, too)

bors-servo · 2017-08-21T16:09:39Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True


        style: Move DocumentStyleSheet to style.

emilio · 2017-08-21T18:09:00Z

This is now rebased on top of #18143, and ready for review.

emilio · 2017-08-21T23:04:18Z

Since @SimonSapin asked about the unsafe impl Sync for GeckoStyleSheet, and I didn't have a better answer than "our usage happens to be fine", I pushed another commit to isolate the unsafety harder.

Thanks Simon :)

SimonSapin · 2017-08-22T14:50:54Z

I’d still like a couple sentences on that impl to say how it’s used etc.

Reviewed 3 of 3 files at r1, 7 of 7 files at r2, 2 of 2 files at r3, 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

components/style/stylist.rs, line 54 at r2 (raw file):

/// The type of the stylesheets that the stylist contains.
///
/// FIXME(emilio): It'd be pretty nice to make the stylist generic, but that's

I think cfg over generics is fine.

components/style/stylist.rs, line 330 at r2 (raw file):

/// This structure holds all the selectors and device characteristics
/// for a given document. The selectors are converted into `Rule`s
/// (defined in rust-selectors), and sorted into `SelectorMap`s keyed

Rule is not in rust-selectors anymore.

components/style/stylist.rs, line 333 at r4 (raw file):

#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
struct StylistStylesheetSet(StylesheetSet<StylistSheet>);
unsafe impl Sync for StylistStylesheetSet {}

Please add a doc-comment here to explain why this impl is ok.

Comments from Reviewable


        style: Move the StyleSheetSet into the Stylist.

This will allow tracking whether there have been only additions to the stylesheet set, and in that case don't destroy and completely rebuild the invalidation map.


        style: Add docs about the lack of device in the stylesheet set.

Also add a few comments and similar about pending invalidation work, and avoid passing a device in script, since the work it performs scanning the stylesheet is just thrown away in the `flush_without_invalidations` call.


        style: Avoid blanket Sync for GeckoStyleSheet, use a more isolated wr…

…apper.

SimonSapin · 2017-08-22T14:59:43Z

@bors-servo r+

Reviewed 2 of 2 files at r6, 2 of 2 files at r7.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

Comments from Reviewable

bors-servo · 2017-08-22T14:59:44Z

📌 Commit c73ec5e has been approved by SimonSapin

emilio · 2017-08-22T16:10:15Z

@bors-servo p=1

More patches incoming.


        Auto merge of #18191 - emilio:incremental-rebuilds, r=<try>

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)

bors-servo · 2017-08-22T16:45:44Z

⌛ Testing commit c73ec5e with merge 019b125...


        Auto merge of #18170 - emilio:stylist-stylesheet-set, r=SimonSapin

style: Move the StyleSheetSet into the Stylist. This will allow tracking whether there have been only additions to the stylesheet set, and in that case don't destroy and completely rebuild the invalidation map. This is on top of #18143.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18170)

bors-servo · 2017-08-22T18:42:35Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: SimonSapin
Pushing 019b125 to master...


        Auto merge of #18191 - emilio:incremental-rebuilds, r=<try>

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)


        Auto merge of #18191 - emilio:incremental-rebuilds, r=<try>

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)


        Auto merge of #18191 - emilio:incremental-rebuilds, r=<try>

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)


        Auto merge of #18191 - emilio:incremental-rebuilds, r=<try>

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)


        Auto merge of #18191 - emilio:incremental-rebuilds, r=SimonSapin,heycam

style: Implement finer-grained stylist rebuilds. This is on top of #18170, and aims to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=1386045.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18191)

highfive assigned pcwalton Aug 21, 2017

highfive added the S-awaiting-review label Aug 21, 2017

emilio force-pushed the emilio:stylist-stylesheet-set branch from 6f7bedf to ed30bbe Aug 21, 2017

highfive assigned SimonSapin and unassigned pcwalton Aug 21, 2017

style: Move DocumentStyleSheet to style.

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

02b2038

emilio force-pushed the emilio:stylist-stylesheet-set branch from ed30bbe to 1700ff3 Aug 21, 2017

emilio added 3 commits Aug 21, 2017

style: Avoid blanket Sync for GeckoStyleSheet, use a more isolated wr…

Unverified

This user has not uploaded their public key yet.

GPG key ID: 056B727BB9C1027C Learn about signing commits

Loading status checks…

c73ec5e

…apper.

emilio force-pushed the emilio:stylist-stylesheet-set branch from c5f4670 to c73ec5e Aug 22, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Aug 22, 2017

emilio mentioned this pull request Aug 22, 2017

style: Implement finer-grained stylist rebuilds. #18191

Merged

bors-servo merged commit c73ec5e into servo:master Aug 22, 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

highfive removed the S-awaiting-merge label Aug 22, 2017

emilio deleted the emilio:stylist-stylesheet-set branch Aug 24, 2017

servo / servo

style: Move the StyleSheetSet into the Stylist. #18170

style: Move the StyleSheetSet into the Stylist. #18170

emilio commented Aug 21, 2017 •

edited by SimonSapin

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

highfive commented Aug 21, 2017

highfive commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

emilio commented Aug 21, 2017

SimonSapin commented Aug 22, 2017

SimonSapin commented Aug 22, 2017

bors-servo commented Aug 22, 2017

emilio commented Aug 22, 2017

bors-servo commented Aug 22, 2017

bors-servo commented Aug 22, 2017

servo / servo

Join GitHub today

style: Move the StyleSheetSet into the Stylist. #18170

style: Move the StyleSheetSet into the Stylist. #18170

Conversation

emilio commented Aug 21, 2017 • edited by SimonSapin

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

highfive commented Aug 21, 2017

highfive commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

emilio commented Aug 21, 2017

bors-servo commented Aug 21, 2017

emilio commented Aug 21, 2017

emilio commented Aug 21, 2017

SimonSapin commented Aug 22, 2017

SimonSapin commented Aug 22, 2017

bors-servo commented Aug 22, 2017

emilio commented Aug 22, 2017

bors-servo commented Aug 22, 2017

bors-servo commented Aug 22, 2017

emilio commented Aug 21, 2017 •

edited by SimonSapin