servo: Merge #18070 - stylo: only clear relevant origins when medium features change (from emilio:orig-medium); r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 14 Aug 2017 06:42:44 -0500
changeset 646054 684c47edc522ecf9231195cfc484c110d5b1a3f8
parent 646053 7b923ee134c52030eb05ca68e5eb5b9b729f357e
child 646055 a5a29d09a1c769e9f6511497bd00c2a85592ee31
push id73981
push userbmo:emilio+bugs@crisal.io
push dateMon, 14 Aug 2017 19:19:09 +0000
reviewersheycam
milestone57.0a1
servo: Merge #18070 - stylo: only clear relevant origins when medium features change (from emilio:orig-medium); r=heycam Bug: 1389871 Reviewed-by: heycam Source-Repo: https://github.com/servo/servo Source-Revision: 1573309b868bc971b4cd1fe7153c57c4bf37c6cc
servo/components/style/gecko/generated/bindings.rs
servo/components/style/gecko_bindings/sugar/mod.rs
servo/components/style/gecko_bindings/sugar/origin_flags.rs
servo/components/style/stylesheets/mod.rs
servo/components/style/stylesheets/origin.rs
servo/components/style/stylist.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/gecko/generated/bindings.rs
+++ b/servo/components/style/gecko/generated/bindings.rs
@@ -1985,17 +1985,17 @@ extern "C" {
     pub fn Servo_StyleSet_Clear(set: RawServoStyleSetBorrowed);
 }
 extern "C" {
     pub fn Servo_StyleSet_RebuildCachedData(set: RawServoStyleSetBorrowed);
 }
 extern "C" {
     pub fn Servo_StyleSet_MediumFeaturesChanged(set: RawServoStyleSetBorrowed,
                                                 viewport_units_used:
-                                                    *mut bool) -> bool;
+                                                    *mut bool) -> OriginFlags;
 }
 extern "C" {
     pub fn Servo_StyleSet_CompatModeChanged(raw_data:
                                                 RawServoStyleSetBorrowed);
 }
 extern "C" {
     pub fn Servo_StyleSet_AppendStyleSheet(set: RawServoStyleSetBorrowed,
                                            gecko_sheet:
--- a/servo/components/style/gecko_bindings/sugar/mod.rs
+++ b/servo/components/style/gecko_bindings/sugar/mod.rs
@@ -8,12 +8,12 @@ mod ns_com_ptr;
 mod ns_compatibility;
 mod ns_css_shadow_array;
 mod ns_css_shadow_item;
 pub mod ns_css_value;
 mod ns_style_auto_array;
 pub mod ns_style_coord;
 mod ns_t_array;
 mod ns_timing_function;
-mod origin_flags;
+pub mod origin_flags;
 pub mod ownership;
 pub mod refptr;
 mod style_complex_color;
--- a/servo/components/style/gecko_bindings/sugar/origin_flags.rs
+++ b/servo/components/style/gecko_bindings/sugar/origin_flags.rs
@@ -3,48 +3,29 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Helper to iterate over `OriginFlags` bits.
 
 use gecko_bindings::structs::OriginFlags;
 use gecko_bindings::structs::OriginFlags_Author;
 use gecko_bindings::structs::OriginFlags_User;
 use gecko_bindings::structs::OriginFlags_UserAgent;
-use stylesheets::Origin;
+use stylesheets::OriginSet;
 
-impl OriginFlags {
-    /// Returns an iterator over the origins present in the `OriginFlags`,
-    /// in order from highest priority (author) to lower (user agent).
-    pub fn iter(self) -> OriginFlagsIter {
-        OriginFlagsIter {
-            origin_flags: self,
-            cur: 0,
-        }
+/// Checks that the values for OriginFlags are the ones we expect.
+pub fn assert_flags_match() {
+    use stylesheets::origin::*;
+    debug_assert_eq!(OriginFlags_UserAgent.0, ORIGIN_USER_AGENT.bits());
+    debug_assert_eq!(OriginFlags_Author.0, ORIGIN_AUTHOR.bits());
+    debug_assert_eq!(OriginFlags_User.0, ORIGIN_USER.bits());
+}
+
+impl From<OriginFlags> for OriginSet {
+    fn from(flags: OriginFlags) -> Self {
+        Self::from_bits_truncate(flags.0)
     }
 }
 
-/// Iterates over the origins present in an `OriginFlags`, in order from
-/// highest priority (author) to lower (user agent).
-pub struct OriginFlagsIter {
-    origin_flags: OriginFlags,
-    cur: usize,
-}
-
-impl Iterator for OriginFlagsIter {
-    type Item = Origin;
-
-    fn next(&mut self) -> Option<Origin> {
-        loop {
-            let (bit, origin) = match self.cur {
-                0 => (OriginFlags_Author, Origin::Author),
-                1 => (OriginFlags_User, Origin::User),
-                2 => (OriginFlags_UserAgent, Origin::UserAgent),
-                _ => return None,
-            };
-
-            self.cur += 1;
-
-            if (self.origin_flags & bit).0 != 0 {
-                return Some(origin);
-            }
-        }
+impl From<OriginSet> for OriginFlags {
+    fn from(set: OriginSet) -> Self {
+        OriginFlags(set.bits())
     }
 }
--- a/servo/components/style/stylesheets/mod.rs
+++ b/servo/components/style/stylesheets/mod.rs
@@ -9,17 +9,17 @@ mod document_rule;
 mod font_face_rule;
 pub mod font_feature_values_rule;
 pub mod import_rule;
 pub mod keyframes_rule;
 mod loader;
 mod media_rule;
 mod memory;
 mod namespace_rule;
-mod origin;
+pub mod origin;
 mod page_rule;
 mod rule_list;
 mod rule_parser;
 mod rules_iterator;
 mod style_rule;
 mod stylesheet;
 pub mod supports_rule;
 pub mod viewport_rule;
@@ -39,17 +39,17 @@ pub use self::font_feature_values_rule::
 pub use self::import_rule::ImportRule;
 pub use self::keyframes_rule::KeyframesRule;
 pub use self::loader::StylesheetLoader;
 pub use self::media_rule::MediaRule;
 pub use self::memory::{MallocSizeOf, MallocSizeOfFn, MallocSizeOfWithGuard};
 #[cfg(feature = "gecko")]
 pub use self::memory::{MallocSizeOfWithRepeats, SizeOfState};
 pub use self::namespace_rule::NamespaceRule;
-pub use self::origin::{Origin, PerOrigin, PerOriginClear};
+pub use self::origin::{Origin, OriginSet, PerOrigin, PerOriginClear};
 pub use self::page_rule::PageRule;
 pub use self::rule_parser::{State, TopLevelRuleParser};
 pub use self::rule_list::{CssRules, CssRulesHelpers};
 pub use self::rules_iterator::{AllRules, EffectiveRules, NestedRuleIterationCondition, RulesIterator};
 pub use self::stylesheet::{Namespaces, Stylesheet, StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
 pub use self::style_rule::StyleRule;
 pub use self::supports_rule::SupportsRule;
 pub use self::viewport_rule::ViewportRule;
--- a/servo/components/style/stylesheets/origin.rs
+++ b/servo/components/style/stylesheets/origin.rs
@@ -1,30 +1,109 @@
 /* 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/. */
 
-///! [CSS cascade origins](https://drafts.csswg.org/css-cascade/#cascading-origins).
+//! [CSS cascade origins](https://drafts.csswg.org/css-cascade/#cascading-origins).
 
 use std::marker::PhantomData;
+use std::ops::BitOrAssign;
 
 /// Each style rule has an origin, which determines where it enters the cascade.
 ///
 /// https://drafts.csswg.org/css-cascade/#cascading-origins
 #[derive(Clone, PartialEq, Eq, Copy, Debug)]
+#[repr(u8)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum Origin {
-    /// https://drafts.csswg.org/css-cascade/#cascade-origin-us
-    UserAgent,
+    /// https://drafts.csswg.org/css-cascade/#cascade-origin-user-agent
+    UserAgent = 1 << 0,
 
     /// https://drafts.csswg.org/css-cascade/#cascade-origin-user
-    User,
+    User = 1 << 1,
 
     /// https://drafts.csswg.org/css-cascade/#cascade-origin-author
-    Author,
+    Author = 1 << 2,
+}
+
+impl Origin {
+    /// Returns an origin that goes in order for `index`.
+    ///
+    /// This is used for iterating across origins.
+    fn from_index(index: i8) -> Option<Self> {
+        Some(match index {
+            0 => Origin::Author,
+            1 => Origin::User,
+            2 => Origin::UserAgent,
+            _ => return None,
+        })
+    }
+}
+
+bitflags! {
+    /// A set of origins. This is equivalent to Gecko's OriginFlags.
+    pub flags OriginSet: u8 {
+        /// https://drafts.csswg.org/css-cascade/#cascade-origin-user-agent
+        const ORIGIN_USER_AGENT = Origin::UserAgent as u8,
+        /// https://drafts.csswg.org/css-cascade/#cascade-origin-user
+        const ORIGIN_USER = Origin::User as u8,
+        /// https://drafts.csswg.org/css-cascade/#cascade-origin-author
+        const ORIGIN_AUTHOR = Origin::Author as u8,
+    }
+}
+
+impl OriginSet {
+    /// Returns an iterator over the origins present in this `OriginSet`.
+    ///
+    /// See the `OriginSet` documentation for information about the order
+    /// origins are iterated.
+    pub fn iter(&self) -> OriginSetIterator {
+        OriginSetIterator {
+            set: *self,
+            cur: 0,
+        }
+    }
+}
+
+impl From<Origin> for OriginSet {
+    fn from(origin: Origin) -> Self {
+        Self::from_bits_truncate(origin as u8)
+    }
+}
+
+impl BitOrAssign<Origin> for OriginSet {
+    fn bitor_assign(&mut self, origin: Origin) {
+        *self |= OriginSet::from(origin);
+    }
+}
+
+/// Iterates over the origins present in an `OriginSet`, in order from
+/// highest priority (author) to lower (user agent).
+pub struct OriginSetIterator {
+    set: OriginSet,
+    cur: i8,
+}
+
+impl Iterator for OriginSetIterator {
+    type Item = Origin;
+
+    fn next(&mut self) -> Option<Origin> {
+        loop {
+            let origin = match Origin::from_index(self.cur) {
+                Some(origin) => origin,
+                None => return None,
+            };
+
+            self.cur += 1;
+
+            if self.set.contains(origin.into()) {
+                return Some(origin)
+            }
+        }
+    }
 }
 
 /// An object that stores a `T` for each origin of the CSS cascade.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Debug, Default)]
 pub struct PerOrigin<T> {
     /// Data for `Origin::UserAgent`.
     pub user_agent: T,
@@ -113,24 +192,24 @@ pub struct PerOriginIter<'a, T: 'a> {
     cur: i8,
     rev: bool,
 }
 
 impl<'a, T> Iterator for PerOriginIter<'a, T> where T: 'a {
     type Item = (&'a T, Origin);
 
     fn next(&mut self) -> Option<Self::Item> {
-        let result = match self.cur {
-            0 => (&self.data.author, Origin::Author),
-            1 => (&self.data.user, Origin::User),
-            2 => (&self.data.user_agent, Origin::UserAgent),
-            _ => return None,
+        let origin = match Origin::from_index(self.cur) {
+            Some(origin) => origin,
+            None => return None,
         };
+
         self.cur += if self.rev { -1 } else { 1 };
-        Some(result)
+
+        Some((self.data.borrow_for_origin(&origin), origin))
     }
 }
 
 /// Like `PerOriginIter<T>`, but iterates over mutable references to the
 /// per-origin data.
 ///
 /// We must use unsafe code here since it's not possible for the borrow
 /// checker to know that we are safely returning a different reference
@@ -140,18 +219,18 @@ pub struct PerOriginIterMut<'a, T: 'a> {
     cur: i8,
     _marker: PhantomData<&'a mut PerOrigin<T>>,
 }
 
 impl<'a, T> Iterator for PerOriginIterMut<'a, T> where T: 'a {
     type Item = (&'a mut T, Origin);
 
     fn next(&mut self) -> Option<Self::Item> {
-        let result = match self.cur {
-            0 => (unsafe { &mut (*self.data).author }, Origin::Author),
-            1 => (unsafe { &mut (*self.data).user }, Origin::User),
-            2 => (unsafe { &mut (*self.data).user_agent }, Origin::UserAgent),
-            _ => return None,
+        let origin = match Origin::from_index(self.cur) {
+            Some(origin) => origin,
+            None => return None,
         };
+
         self.cur += 1;
-        Some(result)
+
+        Some((unsafe { (*self.data).borrow_mut_for_origin(&origin) }, origin))
     }
 }
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -35,17 +35,17 @@ use selectors::visitor::SelectorVisitor;
 use servo_arc::{Arc, ArcBorrow};
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use smallvec::VecLike;
 use std::fmt::Debug;
 use style_traits::viewport::ViewportConstraints;
 #[cfg(feature = "gecko")]
 use stylesheets::{CounterStyleRule, FontFaceRule};
 use stylesheets::{CssRule, StyleRule};
-use stylesheets::{StylesheetInDocument, Origin, PerOrigin, PerOriginClear};
+use stylesheets::{StylesheetInDocument, Origin, OriginSet, PerOrigin, PerOriginClear};
 use stylesheets::UserAgentStylesheets;
 use stylesheets::keyframes_rule::KeyframesAnimation;
 use stylesheets::viewport_rule::{self, MaybeNew, ViewportRule};
 use thread_state;
 
 pub use ::fnv::FnvHashMap;
 
 /// This structure holds all the selectors and device characteristics
@@ -972,51 +972,59 @@ impl Stylist {
             device.account_for_viewport_rule(constraints);
         }
 
         self.device = device;
         let features_changed = self.media_features_change_changed_style(
             stylesheets.iter().map(|s| &**s),
             guard
         );
-        self.is_device_dirty |= features_changed;
+        self.is_device_dirty |= !features_changed.is_empty();
     }
 
     /// Returns whether, given a media feature change, any previously-applicable
-    /// style has become non-applicable, or vice-versa.
+    /// style has become non-applicable, or vice-versa for each origin.
     pub fn media_features_change_changed_style<'a, I, S>(
         &self,
         stylesheets: I,
         guard: &SharedRwLockReadGuard,
-    ) -> bool
+    ) -> OriginSet
     where
         I: Iterator<Item = &'a S>,
         S: StylesheetInDocument + ToMediaListKey + 'static,
     {
         use invalidation::media_queries::PotentiallyEffectiveMediaRules;
 
         debug!("Stylist::media_features_change_changed_style");
 
-        for stylesheet in stylesheets {
+        let mut origins = OriginSet::empty();
+
+        'stylesheets_loop: for stylesheet in stylesheets {
             let effective_now =
                 stylesheet.is_effective_for_device(&self.device, guard);
 
             let origin = stylesheet.origin(guard);
+
+            if origins.contains(origin.into()) {
+                continue;
+            }
+
             let origin_cascade_data =
                 self.cascade_data.borrow_for_origin(&origin);
 
             let effective_then =
                 origin_cascade_data
                     .effective_media_query_results
                     .was_effective(stylesheet);
 
             if effective_now != effective_then {
                 debug!(" > Stylesheet changed -> {}, {}",
                        effective_then, effective_now);
-                return true
+                origins |= origin;
+                continue;
             }
 
             if !effective_now {
                 continue;
             }
 
             let mut iter =
                 stylesheet.iter_rules::<PotentiallyEffectiveMediaRules>(
@@ -1046,17 +1054,18 @@ impl Stylist {
                                 .is_effective_for_device(&self.device, guard);
                         let effective_then =
                             origin_cascade_data
                                 .effective_media_query_results
                                 .was_effective(import_rule);
                         if effective_now != effective_then {
                             debug!(" > @import rule changed {} -> {}",
                                    effective_then, effective_now);
-                            return true;
+                            origins |= origin;
+                            continue 'stylesheets_loop;
                         }
 
                         if !effective_now {
                             iter.skip_children();
                         }
                     }
                     CssRule::Media(ref lock) => {
                         let media_rule = lock.read_with(guard);
@@ -1065,28 +1074,29 @@ impl Stylist {
                             mq.evaluate(&self.device, self.quirks_mode);
                         let effective_then =
                             origin_cascade_data
                                 .effective_media_query_results
                                 .was_effective(media_rule);
                         if effective_now != effective_then {
                             debug!(" > @media rule changed {} -> {}",
                                    effective_then, effective_now);
-                            return true;
+                            origins |= origin;
+                            continue 'stylesheets_loop;
                         }
 
                         if !effective_now {
                             iter.skip_children();
                         }
                     }
                 }
             }
         }
 
-        return false;
+        return origins
     }
 
     /// Returns the viewport constraints that apply to this document because of
     /// a @viewport rule.
     pub fn viewport_constraints(&self) -> Option<&ViewportConstraints> {
         self.viewport_constraints.as_ref()
     }
 
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -110,17 +110,17 @@ use style::properties::parse_one_declara
 use style::rule_tree::StyleSource;
 use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
 use style::style_adjuster::StyleAdjuster;
 use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule};
 use style::stylesheets::{FontFeatureValuesRule, ImportRule, KeyframesRule, MallocSizeOfWithGuard};
-use style::stylesheets::{MediaRule, NamespaceRule, Origin, PageRule, SizeOfState, StyleRule};
+use style::stylesheets::{MediaRule, NamespaceRule, Origin, OriginSet, PageRule, SizeOfState, StyleRule};
 use style::stylesheets::{StylesheetContents, StylesheetInDocument, SupportsRule};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue};
 use style::stylesheets::supports_rule::parse_condition_or_declaration;
 use style::stylist::RuleInclusion;
 use style::thread_state;
 use style::timer::Timer;
 use style::traversal::{DomTraversal, TraversalDriver};
@@ -143,29 +143,32 @@ use super::stylesheet_loader::Stylesheet
  */
 
 // A dummy url data for where we don't pass url data in.
 // We need to get rid of this sooner than later.
 static mut DUMMY_URL_DATA: *mut URLExtraData = 0 as *mut URLExtraData;
 
 #[no_mangle]
 pub extern "C" fn Servo_Initialize(dummy_url_data: *mut URLExtraData) {
+    use style::gecko_bindings::sugar::origin_flags;
+
     // Initialize logging.
     let mut builder = LogBuilder::new();
     let default_level = if cfg!(debug_assertions) { "warn" } else { "error" };
     match env::var("RUST_LOG") {
       Ok(v) => builder.parse(&v).init().unwrap(),
       _ => builder.parse(default_level).init().unwrap(),
     };
 
     // Pretend that we're a Servo Layout thread, to make some assertions happy.
     thread_state::initialize(thread_state::LAYOUT);
 
     // Perform some debug-only runtime assertions.
     restyle_hints::assert_restyle_hints_match();
+    origin_flags::assert_flags_match();
     parser::assert_parsing_mode_match();
     traversal_flags::assert_traversal_flags_match();
 
     // Initialize the dummy url data
     unsafe { DUMMY_URL_DATA = dummy_url_data; }
 }
 
 #[no_mangle]
@@ -891,17 +894,17 @@ pub extern "C" fn Servo_StyleSet_AppendS
     data.stylesheets.append_stylesheet(&data.stylist, sheet, &guard);
     data.clear_stylist_origin(&origin);
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_MediumFeaturesChanged(
     raw_data: RawServoStyleSetBorrowed,
     viewport_units_used: *mut bool,
-) -> bool {
+) -> OriginFlags {
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
 
     // NOTE(emilio): We don't actually need to flush the stylist here and ensure
     // it's up to date.
     //
     // In case it isn't we would trigger a rebuild + restyle as needed too.
     //
@@ -911,22 +914,23 @@ pub extern "C" fn Servo_StyleSet_MediumF
     // FIXME(emilio, bug 1369984): do the computation conditionally, to do it
     // less often.
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
 
     unsafe {
         *viewport_units_used = data.stylist.device().used_viewport_size();
     }
     data.stylist.device_mut().reset_computed_values();
-    let rules_changed = data.stylist.media_features_change_changed_style(
-        data.stylesheets.iter(),
-        &guard,
-    );
-
-    rules_changed
+    let origins_in_which_rules_changed =
+        data.stylist.media_features_change_changed_style(
+            data.stylesheets.iter(),
+            &guard,
+        );
+
+    OriginFlags::from(origins_in_which_rules_changed)
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_PrependStyleSheet(
     raw_data: RawServoStyleSetBorrowed,
     sheet: *const ServoStyleSheet,
 ) {
     let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -989,17 +993,17 @@ pub extern "C" fn Servo_StyleSet_FlushSt
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged(
     raw_data: RawServoStyleSetBorrowed,
     author_style_disabled: bool,
     changed_origins: OriginFlags,
 ) {
     let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
-    for origin in changed_origins.iter() {
+    for origin in OriginSet::from(changed_origins).iter() {
         data.stylesheets.force_dirty_origin(&origin);
         data.clear_stylist_origin(&origin);
     }
     data.stylesheets.set_author_style_disabled(author_style_disabled);
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSheet_HasRules(