Auto merge of #18070 - emilio:orig-medium, r=heycam

stylo: only clear relevant origins when medium features change

Bug: 1389871
Reviewed-by: heycam

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

components/style/gecko/generated/bindings.rs

@@ -1990,7 +1990,7 @@ extern "C" {
 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:

components/style/gecko_bindings/sugar/mod.rs

@@ -13,7 +13,7 @@ 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;

components/style/gecko_bindings/sugar/origin_flags.rs

@@ -8,43 +8,24 @@ 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())
     }
 }

components/style/stylesheets/mod.rs

@@ -14,7 +14,7 @@ mod loader;
 mod media_rule;
 mod memory;
 mod namespace_rule;
-mod origin;
+pub mod origin;
 mod page_rule;
 mod rule_list;
 mod rule_parser;
@@ -44,7 +44,7 @@ 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};

components/style/stylesheets/origin.rs

@@ -2,24 +2,103 @@
  * 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.
@@ -118,14 +197,14 @@ 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))
     }
 }
 
@@ -145,13 +224,13 @@ 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))
     }
 }

components/style/stylist.rs

@@ -40,7 +40,7 @@ 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};
@@ -977,16 +977,16 @@ impl Stylist {
             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,
@@ -995,11 +995,18 @@ impl Stylist {
 
         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);
 
@@ -1011,7 +1018,8 @@ impl Stylist {
             if effective_now != effective_then {
                 debug!(" > Stylesheet changed -> {}, {}",
                        effective_then, effective_now);
-                return true
+                origins |= origin;
+                continue;
             }
 
             if !effective_now {
@@ -1051,7 +1059,8 @@ impl Stylist {
                         if effective_now != effective_then {
                             debug!(" > @import rule changed {} -> {}",
                                    effective_then, effective_now);
-                            return true;
+                            origins |= origin;
+                            continue 'stylesheets_loop;
                         }
 
                         if !effective_now {
@@ -1070,7 +1079,8 @@ impl Stylist {
                         if effective_now != effective_then {
                             debug!(" > @media rule changed {} -> {}",
                                    effective_then, effective_now);
-                            return true;
+                            origins |= origin;
+                            continue 'stylesheets_loop;
                         }
 
                         if !effective_now {
@@ -1081,7 +1091,7 @@ impl Stylist {
             }
         }
 
-        return false;
+        return origins
     }
 
     /// Returns the viewport constraints that apply to this document because of

ports/geckolib/glue.rs

@@ -115,7 +115,7 @@ 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};
@@ -148,6 +148,8 @@ 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" };
@@ -161,6 +163,7 @@ pub extern "C" fn Servo_Initialize(dummy_url_data: *mut URLExtraData) {
 
     // 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();
 
@@ -896,7 +899,7 @@ pub extern "C" fn Servo_StyleSet_AppendStyleSheet(
 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();
 
@@ -916,12 +919,13 @@ pub extern "C" fn Servo_StyleSet_MediumFeaturesChanged(
         *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,
-    );
+    let origins_in_which_rules_changed =
+        data.stylist.media_features_change_changed_style(
+            data.stylesheets.iter(),
+            &guard,
+        );
 
-    rules_changed
+    OriginFlags::from(origins_in_which_rules_changed)
 }
 
 #[no_mangle]
@@ -994,7 +998,7 @@ pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged(
     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);
     }