style: Don't look for viewport units in stylesheets. #18267

emilio · 2017-08-28T09:26:41Z

Use whether we've computed any viewport unit instead.

This is more accurate (we avoid restyling unnecessarily if we've found anything
ever on the stylesheet, but that hasn't matched).

This has the benefit of also matching Gecko, and simplify some code and
fishyness around, and also hopefully speeding up stylesheet parsing.

This change is

highfive · 2017-08-28T09:26:45Z

Heads up! This PR modifies the following files:

@bholley: components/style/servo/media_queries.rs, components/style/stylesheets/stylesheet.rs
@canaltinova: components/style/servo/media_queries.rs, components/style/stylesheets/stylesheet.rs
@fitzgen: components/script/dom/htmlmetaelement.rs, components/script/stylesheet_loader.rs
@KiChjang: components/script/dom/htmlmetaelement.rs, components/script/stylesheet_loader.rs

highfive · 2017-08-28T09:26:46Z

Warning

These commits modify style and script code, but no tests are modified. Please consider adding a test!

emilio · 2017-08-28T09:26:52Z

r? @SimonSapin

@bors-servo try

bors-servo · 2017-08-28T09:26:57Z

⌛ Trying commit 3e49a34 with merge b7e28fb...


        Auto merge of #18267 - emilio:dirty-on-viewport-size-change, r=<try>

style: Don't look for viewport units in stylesheets. Use whether we've computed any viewport unit instead. This is more accurate (we avoid restyling unnecessarily if we've found anything ever on the stylesheet, but that hasn't matched). This has the benefit of also matching Gecko, and simplify some code and fishyness around, and also hopefully speeding up stylesheet parsing.

emilio · 2017-08-28T09:27:24Z

After this land we can remove support for the look_for_viewport_percentages on cssparser.


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=<try>

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)

emilio · 2017-08-28T09:47:36Z

components/style/servo/media_queries.rs

@@ -48,21 +48,27 @@ pub struct Device {
    /// by using rem units.
    #[ignore_heap_size_of = "Pure stack type"]
    used_root_font_size: AtomicBool,
+    /// Whether any styles computed in the document relied on the root font-size
+    /// by using rem units.


Err, this comment needs to change :)

bors-servo · 2017-08-28T09:52:09Z

💔 Test failed - linux-dev


        style: Don't look for viewport units in stylesheets.

Use whether we've computed any viewport unit instead. This is more accurate (we avoid restyling unnecessarily if we've found anything ever on the stylesheet, but that hasn't matched). This has the benefit of also matching Gecko, and simplify some code and fishyness around, and also hopefully speeding up stylesheet parsing.

emilio · 2017-08-28T10:05:47Z

@bors-servo try

Unit tests


        Auto merge of #18267 - emilio:dirty-on-viewport-size-change, r=<try>

style: Don't look for viewport units in stylesheets. Use whether we've computed any viewport unit instead. This is more accurate (we avoid restyling unnecessarily if we've found anything ever on the stylesheet, but that hasn't matched). This has the benefit of also matching Gecko, and simplify some code and fishyness around, and also hopefully speeding up stylesheet parsing.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18267)

bors-servo · 2017-08-28T10:05:52Z

⌛ Trying commit 15e71c5 with merge ce6500c...

bors-servo · 2017-08-28T11:35:16Z

☀️ 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

SimonSapin · 2017-08-28T12:17:22Z

@bors-servo r+ p=1

Reviewed 4 of 5 files at r1, 6 of 6 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

bors-servo · 2017-08-28T12:17:23Z

📌 Commit 15e71c5 has been approved by SimonSapin

bors-servo · 2017-08-28T12:34:08Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: #18261

bors-servo · 2017-08-28T12:34:09Z

📌 Commit 15e71c5 has been approved by SimonSapin


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=<try>

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)


        parser: Remove ability to look for viewport percentages.

The only user of it is going away in servo/servo#18267.


        Auto merge of #187 - servo:bye-looking-for-viewport-percentages, r=Si…

…monSapin parser: Remove ability to look for viewport percentages. The only user of it is going away in servo/servo#18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/187)


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=<try>

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)

bors-servo · 2017-08-28T13:48:49Z

⌛ Testing commit 15e71c5 with merge 96b4e06...


        Auto merge of #18267 - emilio:dirty-on-viewport-size-change, r=SimonS…

…apin style: Don't look for viewport units in stylesheets. Use whether we've computed any viewport unit instead. This is more accurate (we avoid restyling unnecessarily if we've found anything ever on the stylesheet, but that hasn't matched). This has the benefit of also matching Gecko, and simplify some code and fishyness around, and also hopefully speeding up stylesheet parsing.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18267)

bors-servo · 2017-08-28T15:13:09Z

☀️ 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 96b4e06 to master...


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=<try>

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=SimonSapin

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=SimonSapin

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)


        Auto merge of #18268 - emilio:dirty-viewport-followup, r=SimonSapin

style: Recascade the document instead of using the dirty_on_viewport_size_change bit This allows us to simplify a lot of code. On top of #18267.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18268)

highfive assigned metajack Aug 28, 2017

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

highfive assigned SimonSapin and unassigned metajack Aug 28, 2017

emilio mentioned this pull request Aug 28, 2017

style: Recascade the document instead of using the dirty_on_viewport_size_change bit #18268

Merged

emilio reviewed Aug 28, 2017

View changes

highfive added the S-tests-failed label Aug 28, 2017

emilio force-pushed the emilio:dirty-on-viewport-size-change branch from 3e49a34 to 15e71c5 Aug 28, 2017

highfive removed the S-tests-failed label Aug 28, 2017

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

emilio mentioned this pull request Aug 28, 2017

parser: Remove ability to look for viewport percentages. servo/rust-cssparser#187

Merged

bors-servo merged commit 15e71c5 into servo:master Aug 28, 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 28, 2017

servo / servo

style: Don't look for viewport units in stylesheets. #18267

style: Don't look for viewport units in stylesheets. #18267

emilio commented Aug 28, 2017 •

edited by Manishearth

highfive commented Aug 28, 2017

highfive commented Aug 28, 2017

emilio commented Aug 28, 2017

bors-servo commented Aug 28, 2017

emilio commented Aug 28, 2017

This comment has been minimized.

bors-servo commented Aug 28, 2017

emilio commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

SimonSapin commented Aug 28, 2017 •

edited

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

servo / servo

Join GitHub today

style: Don't look for viewport units in stylesheets. #18267

style: Don't look for viewport units in stylesheets. #18267

Conversation

emilio commented Aug 28, 2017 • edited by Manishearth

highfive commented Aug 28, 2017

highfive commented Aug 28, 2017

emilio commented Aug 28, 2017

bors-servo commented Aug 28, 2017

emilio commented Aug 28, 2017

This comment has been minimized.

emilio Aug 28, 2017

bors-servo commented Aug 28, 2017

emilio commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

SimonSapin commented Aug 28, 2017 • edited

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

bors-servo commented Aug 28, 2017

emilio commented Aug 28, 2017 •

edited by Manishearth

SimonSapin commented Aug 28, 2017 •

edited