Auto merge of #12838 - emilio:viewport, r=SimonSapin

Fix restyling on viewport resize.

<!-- Please describe your changes on the following line: -->

Fixes https://github.com/servo/servo/issues/12835

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/12838)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -1114,6 +1114,14 @@ impl LayoutThread {
                     if node.needs_dirty_on_viewport_size_changed() {
                         // NB: The dirty bit is propagated down the tree.
                         unsafe { node.set_dirty(true); }
+
+                        let mut current = node.parent_node();
+                        while let Some(node) = current {
+                            if node.has_dirty_descendants() { break; }
+                            unsafe { node.set_dirty_descendants(true); }
+                            current = node.parent_node();
+                        }
+
                         next = iter.next_skipping_children();
                     } else {
                         next = iter.next();

components/style/animation.rs

@@ -394,6 +394,7 @@ fn compute_style_for_animation_step(context: &SharedStyleContext,
                                                     false,
                                                     Some(previous_style),
                                                     None,
+                                                    None,
                                                     context.error_reporter.clone());
             computed
         }

components/style/cascade_info.rs

@@ -0,0 +1,78 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+use dom::TNode;
+use properties::{DeclaredValue, PropertyDeclaration};
+use values::HasViewportPercentage;
+
+/// A structure to collect information about the cascade.
+///
+/// This is useful to gather information about what an element is affected by,
+/// and can be used in the future to track optimisations like when a
+/// non-inherited property is explicitly inherited, in order to cut-off the
+/// traversal.
+pub struct CascadeInfo {
+    pub saw_viewport_units: bool,
+    #[cfg(debug_assertions)]
+    finished: bool,
+}
+
+impl CascadeInfo {
+    #[cfg(debug_assertions)]
+    pub fn new() -> Self {
+        CascadeInfo {
+            saw_viewport_units: false,
+            finished: false,
+        }
+    }
+
+    #[cfg(not(debug_assertions))]
+    pub fn new() -> Self {
+        CascadeInfo {
+            saw_viewport_units: false,
+        }
+    }
+
+    /// Called when a property is cascaded.
+    ///
+    /// NOTE: We can add a vast amount of information here.
+    #[inline]
+    pub fn on_cascade_property<T>(&mut self,
+                                  _property_declaration: &PropertyDeclaration,
+                                  value: &DeclaredValue<T>)
+        where T: HasViewportPercentage
+    {
+        // TODO: we can be smarter and keep a property bitfield to keep track of
+        // the last applying rule.
+        if value.has_viewport_percentage() {
+            self.saw_viewport_units = true;
+        }
+    }
+
+    #[cfg(debug_assertions)]
+    fn mark_as_finished_if_appropriate(&mut self) {
+        self.finished = true;
+    }
+
+    #[cfg(not(debug_assertions))]
+    fn mark_as_finished_if_appropriate(&mut self) {}
+
+    #[allow(unsafe_code)]
+    pub fn finish<N: TNode>(mut self, node: &N) {
+        self.mark_as_finished_if_appropriate();
+
+        if self.saw_viewport_units {
+            unsafe {
+                node.set_dirty_on_viewport_size_changed();
+            }
+        }
+    }
+}
+
+#[cfg(debug_assertions)]
+impl Drop for CascadeInfo {
+    fn drop(&mut self) {
+        debug_assert!(self.finished,
+                      "Didn't use the result of CascadeInfo, if you don't need it, consider passing None");
+    }
+}

components/style/custom_properties.rs

@@ -39,6 +39,12 @@ pub struct SpecifiedValue {
     references: HashSet<Name>,
 }
 
+impl ::values::HasViewportPercentage for SpecifiedValue {
+    fn has_viewport_percentage(&self) -> bool {
+        panic!("has_viewport_percentage called before resolving!");
+    }
+}
+
 pub struct BorrowedSpecifiedValue<'a> {
     css: &'a str,
     first_token_type: TokenSerializationType,

components/style/lib.rs

@@ -74,6 +74,7 @@ pub mod animation;
 pub mod attr;
 pub mod bezier;
 pub mod cache;
+pub mod cascade_info;
 pub mod context;
 pub mod custom_properties;
 pub mod data;

components/style/matching.rs

@@ -9,6 +9,7 @@
 use animation;
 use arc_ptr_eq;
 use cache::{LRUCache, SimpleHashCache};
+use cascade_info::CascadeInfo;
 use context::{StyleContext, SharedStyleContext};
 use data::PrivateStyleData;
 use dom::{TElement, TNode, TRestyleDamage};
@@ -443,6 +444,7 @@ trait PrivateMatchMethods: TNode {
                                                             &mut old_style) && cacheable;
         }
 
+        let mut cascade_info = CascadeInfo::new();
         let (this_style, is_cacheable) = match parent_style {
             Some(ref parent_style) => {
                 let cache_entry = applicable_declarations_cache.find(applicable_declarations);
@@ -456,6 +458,7 @@ trait PrivateMatchMethods: TNode {
                         shareable,
                         Some(&***parent_style),
                         cached_computed_values,
+                        Some(&mut cascade_info),
                         shared_context.error_reporter.clone())
             }
             None => {
@@ -464,9 +467,11 @@ trait PrivateMatchMethods: TNode {
                         shareable,
                         None,
                         None,
+                        Some(&mut cascade_info),
                         shared_context.error_reporter.clone())
             }
         };
+        cascade_info.finish(self);
 
         cacheable = cacheable && is_cacheable;
 
@@ -497,7 +502,6 @@ trait PrivateMatchMethods: TNode {
             cacheable = cacheable && !animations_started
         }
 
-
         // Cache the resolved style if it was cacheable.
         if cacheable {
             applicable_declarations_cache.insert(applicable_declarations.to_vec(),

components/style/properties/helpers.mako.rs

@@ -167,6 +167,7 @@
             use parser::{ParserContext, ParserContextExtraData};
             use properties::{CSSWideKeyword, DeclaredValue, Shorthand};
         % endif
+        use cascade_info::CascadeInfo;
         use error_reporting::ParseErrorReporter;
         use properties::longhands;
         use properties::property_bit_field::PropertyBitField;
@@ -185,6 +186,7 @@
                                 context: &mut computed::Context,
                                 seen: &mut PropertyBitField,
                                 cacheable: &mut bool,
+                                cascade_info: &mut Option<<&mut CascadeInfo>,
                                 error_reporter: &mut StdBox<ParseErrorReporter + Send>) {
             let declared_value = match *declaration {
                 PropertyDeclaration::${property.camel_case}(ref declared_value) => {
@@ -200,7 +202,13 @@
                 {
                     let custom_props = context.style().custom_properties();
                     ::properties::substitute_variables_${property.ident}(
-                        declared_value, &custom_props, |value| match *value {
+                        declared_value, &custom_props,
+                    |value| {
+                        if let Some(ref mut cascade_info) = *cascade_info {
+                            cascade_info.on_cascade_property(&declaration,
+                                                             &value);
+                        }
+                        match *value {
                             DeclaredValue::Value(ref specified_value) => {
                                 let computed = specified_value.to_computed_value(context);
                                 context.mutate_style().mutate_${data.current_style_struct.name_lower}()
@@ -226,8 +234,8 @@
                                 context.mutate_style().mutate_${data.current_style_struct.name_lower}()
                                        .copy_${property.ident}_from(inherited_struct);
                             }
-                        }, error_reporter
-                    );
+                        }
+                    }, error_reporter);
                 }
 
                 % if property.custom_cascade:

components/style/properties/properties.mako.rs

@@ -34,6 +34,7 @@ use stylesheets::Origin;
 use values::LocalToCss;
 use values::HasViewportPercentage;
 use values::computed::{self, ToComputedValue};
+use cascade_info::CascadeInfo;
 #[cfg(feature = "servo")] use values::specified::BorderStyle;
 
 use self::property_bit_field::PropertyBitField;
@@ -773,6 +774,19 @@ pub enum DeclaredValue<T> {
     // depending on whether the property is inherited.
 }
 
+impl<T: HasViewportPercentage> HasViewportPercentage for DeclaredValue<T> {
+    fn has_viewport_percentage(&self) -> bool {
+        match *self {
+            DeclaredValue::Value(ref v)
+                => v.has_viewport_percentage(),
+            DeclaredValue::WithVariables { .. }
+                => panic!("DeclaredValue::has_viewport_percentage without resolving variables!"),
+            DeclaredValue::Initial |
+            DeclaredValue::Inherit => false,
+        }
+    }
+}
+
 impl<T: ToCss> ToCss for DeclaredValue<T> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
@@ -797,6 +811,20 @@ pub enum PropertyDeclaration {
     Custom(::custom_properties::Name, DeclaredValue<::custom_properties::SpecifiedValue>),
 }
 
+impl HasViewportPercentage for PropertyDeclaration {
+    fn has_viewport_percentage(&self) -> bool {
+        match *self {
+            % for property in data.longhands:
+                PropertyDeclaration::${property.camel_case}(ref val) => {
+                    val.has_viewport_percentage()
+                },
+            % endfor
+            PropertyDeclaration::Custom(_, ref val) => {
+                val.has_viewport_percentage()
+            }
+        }
+    }
+}
 
 #[derive(Eq, PartialEq, Copy, Clone)]
 pub enum PropertyDeclarationParseResult {
@@ -854,19 +882,6 @@ impl ToCss for PropertyDeclaration {
     }
 }
 
-impl HasViewportPercentage for PropertyDeclaration {
-    fn has_viewport_percentage(&self) -> bool {
-        match *self {
-            % for property in data.longhands:
-                PropertyDeclaration::${property.camel_case}(DeclaredValue::Value(ref val)) => {
-                    val.has_viewport_percentage()
-                },
-            % endfor
-            _ => false
-        }
-    }
-}
-
 impl PropertyDeclaration {
     pub fn name(&self) -> PropertyDeclarationName {
         match *self {
@@ -1622,6 +1637,7 @@ fn cascade_with_cached_declarations(
         parent_style: &ComputedValues,
         cached_style: &ComputedValues,
         custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
+        mut cascade_info: Option<<&mut CascadeInfo>,
         mut error_reporter: StdBox<ParseErrorReporter + Send>)
         -> ComputedValues {
     let mut context = computed::Context {
@@ -1664,7 +1680,12 @@ fn cascade_with_cached_declarations(
                                     let custom_props = context.style().custom_properties();
                                     substitute_variables_${property.ident}(
                                         declared_value, &custom_props,
-                                        |value| match *value {
+                                    |value| {
+                                        if let Some(ref mut cascade_info) = cascade_info {
+                                            cascade_info.on_cascade_property(&declaration,
+                                                                             &value);
+                                        }
+                                        match *value {
                                             DeclaredValue::Value(ref specified_value)
                                             => {
                                                 let computed = specified_value.to_computed_value(&context);
@@ -1688,8 +1709,8 @@ fn cascade_with_cached_declarations(
                                                        .copy_${property.ident}_from(inherited_struct);
                                             }
                                             DeclaredValue::WithVariables { .. } => unreachable!()
-                                        }, &mut error_reporter
-                                    );
+                                        }
+                                    }, &mut error_reporter);
                                 % endif
 
                                 % if property.name in data.derived_longhands:
@@ -1728,6 +1749,7 @@ pub type CascadePropertyFn =
                      context: &mut computed::Context,
                      seen: &mut PropertyBitField,
                      cacheable: &mut bool,
+                     cascade_info: &mut Option<<&mut CascadeInfo>,
                      error_reporter: &mut StdBox<ParseErrorReporter + Send>);
 
 #[cfg(feature = "servo")]
@@ -1760,6 +1782,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
                shareable: bool,
                parent_style: Option<<&ComputedValues>,
                cached_style: Option<<&ComputedValues>,
+               mut cascade_info: Option<<&mut CascadeInfo>,
                mut error_reporter: StdBox<ParseErrorReporter + Send>)
                -> (ComputedValues, bool) {
     let initial_values = ComputedValues::initial_values();
@@ -1794,6 +1817,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
                                                      parent_style,
                                                      cached_style,
                                                      custom_properties,
+                                                     cascade_info,
                                                      error_reporter);
         return (style, false)
     }
@@ -1863,6 +1887,7 @@ pub fn cascade(viewport_size: Size2D<Au>,
                                                      &mut context,
                                                      &mut seen,
                                                      &mut cacheable,
+                                                     &mut cascade_info,
                                                      &mut error_reporter);
                 }
             }

components/style/selector_matching.rs

@@ -230,6 +230,7 @@ impl Stylist {
                                     &declarations, false,
                                     parent.map(|p| &**p),
                                     None,
+                                    None,
                                     Box::new(StdoutErrorReporter));
             Some(Arc::new(computed))
         } else {
@@ -242,8 +243,9 @@ impl Stylist {
                                                   pseudo: &PseudoElement,
                                                   parent: &Arc<ComputedValues>)
                                                   -> Option<Arc<ComputedValues>>
-                                                  where E: Element<Impl=TheSelectorImpl> +
-                                                        PresentationalHintsSynthetizer {
+        where E: Element<Impl=TheSelectorImpl> +
+              PresentationalHintsSynthetizer
+    {
         debug_assert!(TheSelectorImpl::pseudo_element_cascade_type(pseudo).is_lazy());
         if self.pseudos_map.get(pseudo).is_none() {
             return None;
@@ -262,8 +264,10 @@ impl Stylist {
         let (computed, _) =
             properties::cascade(self.device.au_viewport_size(),
                                 &declarations, false,
-                                Some(&**parent), None,
+                                Some(&**parent), None, None,
                                 Box::new(StdoutErrorReporter));
+
+
         Some(Arc::new(computed))
     }
 

components/style/stylesheets.rs

@@ -169,7 +169,8 @@ impl Stylesheet {
             origin: origin,
             rules: rules,
             media: None,
-            dirty_on_viewport_size_change: input.seen_viewport_percentages(),
+            dirty_on_viewport_size_change:
+                input.seen_viewport_percentages(),
         }
     }
 

components/style/traversal.rs

@@ -370,20 +370,6 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C,
     // NB: flow construction updates the bloom filter on the way up.
     put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context());
 
-    // Mark the node as DIRTY_ON_VIEWPORT_SIZE_CHANGE is it uses viewport
-    // percentage units.
-    if !node.needs_dirty_on_viewport_size_changed() {
-        if let Some(element) = node.as_element() {
-            if let Some(ref property_declaration_block) = *element.style_attribute() {
-                if property_declaration_block.declarations().any(|d| d.0.has_viewport_percentage()) {
-                    unsafe {
-                        node.set_dirty_on_viewport_size_changed();
-                    }
-                }
-            }
-        }
-    }
-
     if nonincremental_layout {
         RestyleResult::Continue
     } else {

components/style/values/specified/mod.rs

@@ -197,6 +197,7 @@ impl HasViewportPercentage for Length {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
             Length::ViewportPercentage(_) => true,
+            Length::Calc(ref calc) => calc.has_viewport_percentage(),
             _ => false
         }
     }
@@ -208,7 +209,7 @@ impl ToCss for Length {
             Length::Absolute(length) => write!(dest, "{}px", length.to_f32_px()),
             Length::FontRelative(length) => length.to_css(dest),
             Length::ViewportPercentage(length) => length.to_css(dest),
-            Length::Calc(calc) => calc.to_css(dest),
+            Length::Calc(ref calc) => calc.to_css(dest),
             Length::ServoCharacterWidth(_)
             => panic!("internal CSS values should never be serialized"),
         }
@@ -467,6 +468,7 @@ pub struct CalcLengthOrPercentage {
     pub rem: Option<FontRelativeLength>,
     pub percentage: Option<Percentage>,
 }
+
 impl CalcLengthOrPercentage {
     fn parse_sum(input: &mut Parser, expected_unit: CalcUnit) -> Result<CalcSumNode, ()> {
         let mut products = Vec::new();
@@ -751,6 +753,13 @@ impl CalcLengthOrPercentage {
     }
 }
 
+impl HasViewportPercentage for CalcLengthOrPercentage {
+    fn has_viewport_percentage(&self) -> bool {
+        self.vw.is_some() || self.vh.is_some() ||
+            self.vmin.is_some() || self.vmax.is_some()
+    }
+}
+
 impl ToCss for CalcLengthOrPercentage {
     #[allow(unused_assignments)]
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
@@ -823,7 +832,8 @@ pub enum LengthOrPercentage {
 impl HasViewportPercentage for LengthOrPercentage {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
-            LengthOrPercentage::Length(length) => length.has_viewport_percentage(),
+            LengthOrPercentage::Length(ref length) => length.has_viewport_percentage(),
+            LengthOrPercentage::Calc(ref calc) => calc.has_viewport_percentage(),
             _ => false
         }
     }
@@ -882,7 +892,8 @@ pub enum LengthOrPercentageOrAuto {
 impl HasViewportPercentage for LengthOrPercentageOrAuto {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
-            LengthOrPercentageOrAuto::Length(length) => length.has_viewport_percentage(),
+            LengthOrPercentageOrAuto::Length(ref length) => length.has_viewport_percentage(),
+            LengthOrPercentageOrAuto::Calc(ref calc) => calc.has_viewport_percentage(),
             _ => false
         }
     }
@@ -941,7 +952,8 @@ pub enum LengthOrPercentageOrNone {
 impl HasViewportPercentage for LengthOrPercentageOrNone {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
-            LengthOrPercentageOrNone::Length(length) => length.has_viewport_percentage(),
+            LengthOrPercentageOrNone::Length(ref length) => length.has_viewport_percentage(),
+            LengthOrPercentageOrNone::Calc(ref calc) => calc.has_viewport_percentage(),
             _ => false
         }
     }
@@ -950,9 +962,9 @@ impl HasViewportPercentage for LengthOrPercentageOrNone {
 impl ToCss for LengthOrPercentageOrNone {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
-            LengthOrPercentageOrNone::Length(length) => length.to_css(dest),
-            LengthOrPercentageOrNone::Percentage(percentage) => percentage.to_css(dest),
-            LengthOrPercentageOrNone::Calc(calc) => calc.to_css(dest),
+            LengthOrPercentageOrNone::Length(ref length) => length.to_css(dest),
+            LengthOrPercentageOrNone::Percentage(ref percentage) => percentage.to_css(dest),
+            LengthOrPercentageOrNone::Calc(ref calc) => calc.to_css(dest),
             LengthOrPercentageOrNone::None => dest.write_str("none"),
         }
     }
@@ -997,7 +1009,7 @@ pub enum LengthOrNone {
 impl HasViewportPercentage for LengthOrNone {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
-            LengthOrNone::Length(length) => length.has_viewport_percentage(),
+            LengthOrNone::Length(ref length) => length.has_viewport_percentage(),
             _ => false
         }
     }
@@ -1051,6 +1063,7 @@ impl HasViewportPercentage for LengthOrPercentageOrAutoOrContent {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
             LengthOrPercentageOrAutoOrContent::Length(length) => length.has_viewport_percentage(),
+            LengthOrPercentageOrAutoOrContent::Calc(ref calc) => calc.has_viewport_percentage(),
             _ => false
         }
     }

tests/wpt/mozilla/meta/MANIFEST.json

@@ -1356,6 +1356,18 @@
             "url": "/_mozilla/css/direction_style_caching.html"
           }
         ],
+        "css/dirty_viewport.html": [
+          {
+            "path": "css/dirty_viewport.html",
+            "references": [
+              [
+                "/_mozilla/css/dirty_viewport_ref.html",
+                "=="
+              ]
+            ],
+            "url": "/_mozilla/css/dirty_viewport.html"
+          }
+        ],
         "css/empty_cells_a.html": [
           {
             "path": "css/empty_cells_a.html",
@@ -10576,6 +10588,18 @@
           "url": "/_mozilla/css/direction_style_caching.html"
         }
       ],
+      "css/dirty_viewport.html": [
+        {
+          "path": "css/dirty_viewport.html",
+          "references": [
+            [
+              "/_mozilla/css/dirty_viewport_ref.html",
+              "=="
+            ]
+          ],
+          "url": "/_mozilla/css/dirty_viewport.html"
+        }
+      ],
       "css/empty_cells_a.html": [
         {
           "path": "css/empty_cells_a.html",

tests/wpt/mozilla/tests/css/dirty_viewport.html

@@ -0,0 +1,11 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Test elements subject to viewport units are restyled correctly</title>
+<link rel="match" href="dirty_viewport_ref.html">
+<iframe seamless src="dirty_viewport_inner.html" width="500" height="500"></iframe>
+<script>
+  window.onload = function() {
+    var iframe = document.querySelector('iframe');
+    iframe.width = iframe.height = 100;
+  }
+</script>

tests/wpt/mozilla/tests/css/dirty_viewport_inner.html

@@ -0,0 +1,11 @@
+<!doctype html>
+<title>Dirty viewport test (inner)</title>
+<style>
+  div {
+    background: green;
+    width: 50vw;
+    height: 50vh;
+  }
+  html, body { margin: 0; padding: 0; }
+</style>
+<div></div>

tests/wpt/mozilla/tests/css/dirty_viewport_ref.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+  div {
+    background: green;
+    width: 50px;
+    height: 50px;
+  }
+</style>
+<div></div>