Bug 1375383 - style: Ensure we generate a ReconstructFrame hint when -moz-binding changes on a display:none root. r?emilio draft
authorCameron McCormack <cam@mcc.id.au>
Tue, 25 Jul 2017 11:11:47 +0800
changeset 614870 151fdd06d2201973e45a5e4464f78c827ef9d145
parent 614015 5928d905c0bc0b28f5488b236444c7d7991cf8d4
child 638996 bab39983109293476db54ad839fb5a256e0436bd
push id70157
push userbmo:cam@mcc.id.au
push dateTue, 25 Jul 2017 04:33:08 +0000
reviewersemilio
bugs1375383
milestone56.0a1
Bug 1375383 - style: Ensure we generate a ReconstructFrame hint when -moz-binding changes on a display:none root. r?emilio MozReview-Commit-ID: DJ5QKdsAMOS
servo/components/style/gecko/restyle_damage.rs
servo/components/style/matching.rs
servo/components/style/servo/restyle_damage.rs
--- a/servo/components/style/gecko/restyle_damage.rs
+++ b/servo/components/style/gecko/restyle_damage.rs
@@ -1,17 +1,17 @@
 /* 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/. */
 
 //! Gecko's restyle damage computation (aka change hints, aka `nsChangeHint`).
 
 use gecko_bindings::bindings;
 use gecko_bindings::structs;
-use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
+use gecko_bindings::structs::{nsChangeHint, nsStyleContext, nsStyleStructID};
 use matching::{StyleChange, StyleDifference};
 use properties::ComputedValues;
 use servo_arc::Arc;
 use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
 
 /// The representation of Gecko's restyle damage is just a wrapper over
 /// `nsChangeHint`.
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -57,16 +57,44 @@ impl GeckoRestyleDamage {
                                                 new_style,
                                                 source.mBits,
                                                 &mut any_style_changed)
         };
         let change = if any_style_changed { StyleChange::Changed } else { StyleChange::Unchanged };
         StyleDifference::new(GeckoRestyleDamage(hint), change)
     }
 
+    /// Computes the `StyleDifference` between the two `ComputedValues` objects
+    /// for the case where the old and new style are both `display: none`.
+    ///
+    /// In general we don't need to generate damage for such elements, but we
+    /// do need to generate a frame reconstruction for `-moz-binding` changes,
+    /// so that we can start loading the new binding.
+    pub fn compute_undisplayed_style_difference(
+        old_style: &ComputedValues,
+        new_style: &ComputedValues,
+    ) -> StyleDifference {
+        let mut any_style_changed: bool = false;
+
+        // Just compute the Display struct's difference.
+        let display_struct_bit = 1 << (nsStyleStructID::eStyleStruct_Display as u32);
+        let hint = unsafe {
+            bindings::Gecko_CalcStyleDifference(old_style,
+                                                new_style,
+                                                display_struct_bit,
+                                                &mut any_style_changed)
+        };
+
+        // Only pay attention to a reconstruct change hint.
+        let damage = GeckoRestyleDamage(hint) & Self::reconstruct();
+
+        let change = if damage.is_empty() { StyleChange::Changed } else { StyleChange::Unchanged };
+        StyleDifference::new(damage, change)
+    }
+
     /// Returns true if this restyle damage contains all the damage of |other|.
     pub fn contains(self, other: Self) -> bool {
         self & other == other
     }
 
     /// Gets restyle damage to reconstruct the entire frame, subsuming all
     /// other damage.
     pub fn reconstruct() -> Self {
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -778,18 +778,21 @@ pub trait MatchMethods : TElement {
         let old_style_is_display_none = old_display == display::T::none;
 
         // If there's no style source, that likely means that Gecko couldn't
         // find a style context.
         //
         // This happens with display:none elements, and not-yet-existing
         // pseudo-elements.
         if new_style_is_display_none && old_style_is_display_none {
-            // The style remains display:none. No need for damage.
-            return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
+            // The style remains display:none.  The only case we need to care
+            // about is if -moz-binding changed, and to generate a reconstruct
+            // so that we can start the binding load.  Otherwise, there is no
+            // need for damage.
+            return RestyleDamage::compute_undisplayed_style_difference(old_values, new_values);
         }
 
         if pseudo.map_or(false, |p| p.is_before_or_after()) {
             let old_style_generates_no_pseudo =
                 old_style_is_display_none ||
                 old_values.ineffective_content_property();
 
             let new_style_generates_no_pseudo =
--- a/servo/components/style/servo/restyle_damage.rs
+++ b/servo/components/style/servo/restyle_damage.rs
@@ -64,16 +64,27 @@ impl ServoRestyleDamage {
                                     old: &ComputedValues,
                                     new: &ComputedValues)
                                     -> StyleDifference {
         let damage = compute_damage(old, new);
         let change = if damage.is_empty() { StyleChange::Unchanged } else { StyleChange::Changed };
         StyleDifference::new(damage, change)
     }
 
+    /// Computes the `StyleDifference` between the two `ComputedValues` objects
+    /// for the case where the old and new style are both `display: none`.
+    ///
+    /// For Servo we never need to generate any damage for such elements.
+    pub fn compute_undisplayed_style_difference(
+        _old_style: &ComputedValues,
+        _new_style: &ComputedValues,
+    ) -> StyleDifference {
+        StyleDifference::new(Self::empty(), StyleChange::Unchanged)
+    }
+
     /// Returns a bitmask that represents a flow that needs to be rebuilt and
     /// reflowed.
     ///
     /// FIXME(bholley): Do we ever actually need this? Shouldn't
     /// RECONSTRUCT_FLOW imply everything else?
     pub fn rebuild_and_reflow() -> ServoRestyleDamage {
         REPAINT | REPOSITION | STORE_OVERFLOW | BUBBLE_ISIZES | REFLOW_OUT_OF_FLOW | REFLOW |
             RECONSTRUCT_FLOW