Auto merge of #17279 - emilio:no-viewport, r=bholley

style: Don't try to compute @viewport unnecessarily.

If the viewport rule is not enabled, there's just no point in computing it.

Bug: 1372058

components/style/stylesheets/rule_parser.rs

@@ -30,6 +30,7 @@ use stylesheets::keyframes_rule::parse_keyframe_list;
 use stylesheets::loader::NoOpLoader;
 use stylesheets::stylesheet::{Namespaces, Stylesheet};
 use stylesheets::supports_rule::SupportsCondition;
+use stylesheets::viewport_rule;
 use values::CustomIdent;
 use values::KeyframesName;
 use values::specified::url::SpecifiedUrl;
@@ -325,17 +326,6 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
     }
 }
 
-#[cfg(feature = "servo")]
-fn is_viewport_enabled() -> bool {
-    use servo_config::prefs::PREFS;
-    PREFS.get("layout.viewport.enabled").as_boolean().unwrap_or(false)
-}
-
-#[cfg(not(feature = "servo"))]
-fn is_viewport_enabled() -> bool {
-    false // Gecko doesn't support @viewport.
-}
-
 impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
     type Prelude = AtRulePrelude;
     type AtRule = CssRule;
@@ -380,7 +370,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
                 Ok(AtRuleType::WithBlock(AtRulePrelude::CounterStyle(name)))
             },
             "viewport" => {
-                if is_viewport_enabled() {
+                if viewport_rule::enabled() {
                     Ok(AtRuleType::WithBlock(AtRulePrelude::Viewport))
                 } else {
                     Err(StyleParseError::UnsupportedAtRule(name.clone()).into())

components/style/stylesheets/viewport_rule.rs

@@ -31,6 +31,19 @@ use stylesheets::{Stylesheet, Origin};
 use values::computed::{Context, ToComputedValue};
 use values::specified::{NoCalcLength, LengthOrPercentageOrAuto, ViewportPercentageLength};
 
+/// Whether parsing and processing of `@viewport` rules is enabled.
+#[cfg(feature = "servo")]
+pub fn enabled() -> bool {
+    use servo_config::prefs::PREFS;
+    PREFS.get("layout.viewport.enabled").as_boolean().unwrap_or(false)
+}
+
+/// Whether parsing and processing of `@viewport` rules is enabled.
+#[cfg(not(feature = "servo"))]
+pub fn enabled() -> bool {
+    false // Gecko doesn't support @viewport.
+}
+
 macro_rules! declare_viewport_descriptor {
     ( $( $variant_name: expr => $variant: ident($data: ident), )+ ) => {
          declare_viewport_descriptor_inner!([] [ $( $variant_name => $variant($data), )+ ] 0);

components/style/stylist.rs

@@ -368,14 +368,30 @@ impl Stylist {
 
         self.num_rebuilds += 1;
 
-        let cascaded_rule = ViewportRule {
+        self.viewport_constraints = None;
-            declarations: viewport_rule::Cascade::from_stylesheets(
-                doc_stylesheets.clone(), guards.author, &self.device
-            ).finish(),
-        };
 
-        self.viewport_constraints =
+        if viewport_rule::enabled() {
-            ViewportConstraints::maybe_new(&self.device, &cascaded_rule, self.quirks_mode);
+            // TODO(emilio): This doesn't look so efficient.
+            //
+            // Presumably when we properly implement this we can at least have a
+            // bit on the stylesheet that says whether it contains viewport
+            // rules to skip it entirely?
+            //
+            // Processing it with the rest of rules seems tricky since it
+            // overrides the viewport size which may change the evaluation of
+            // media queries (or may not? how are viewport units in media
+            // queries defined?)
+            let cascaded_rule = ViewportRule {
+                declarations: viewport_rule::Cascade::from_stylesheets(
+                    doc_stylesheets.clone(), guards.author, &self.device
+                ).finish()
+            };
+
+            self.viewport_constraints =
+                ViewportConstraints::maybe_new(&self.device,
+                                               &cascaded_rule,
+                                               self.quirks_mode)
+        }
 
         if let Some(ref constraints) = self.viewport_constraints {
             self.device.account_for_viewport_rule(constraints);