style: Remove Stylist::is_device_dirty.

More progress on unifying how Gecko and Servo track stylist dirtiness.

components/layout_thread/lib.rs

@@ -137,7 +137,7 @@ use style::selector_parser::SnapshotMap;
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW};
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards};
 use style::stylesheet_set::StylesheetSet;
-use style::stylesheets::{Origin, OriginSet, Stylesheet, StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
+use style::stylesheets::{Origin, Stylesheet, StylesheetContents, StylesheetInDocument, UserAgentStylesheets};
 use style::stylist::Stylist;
 use style::thread_state;
 use style::timer::Timer;
@@ -438,16 +438,6 @@ impl<'a, 'b: 'a> RwData<'a, 'b> {
             Some(x) => RWGuard::Held(x),
         }
     }
-
-    /// If no reflow has ever been triggered, this will keep the lock, locked
-    /// (and saved in `possibly_locked_rw_data`). If it has been, the lock will
-    /// be unlocked.
-    fn block(&mut self, rw_data: RWGuard<'b>) {
-        match rw_data {
-            RWGuard::Used(x) => drop(x),
-            RWGuard::Held(x) => *self.possibly_locked_rw_data = Some(x),
-        }
-    }
 }
 
 fn add_font_face_rules(stylesheet: &Stylesheet,
@@ -526,18 +516,6 @@ impl LayoutThread {
         let font_cache_receiver =
             ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_font_cache_receiver);
 
-        let stylist = Stylist::new(device, QuirksMode::NoQuirks);
-        let outstanding_web_fonts_counter = Arc::new(AtomicUsize::new(0));
-        let ua_stylesheets = &*UA_STYLESHEETS;
-        let guard = ua_stylesheets.shared_lock.read();
-        for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
-            add_font_face_rules(stylesheet,
-                                &guard,
-                                stylist.device(),
-                                &font_cache_thread,
-                                &ipc_font_cache_sender,
-                                &outstanding_web_fonts_counter);
-        }
 
         LayoutThread {
             id: id,
@@ -561,7 +539,7 @@ impl LayoutThread {
             generation: Cell::new(0),
             new_animations_sender: new_animations_sender,
             new_animations_receiver: new_animations_receiver,
-            outstanding_web_fonts: outstanding_web_fonts_counter,
+            outstanding_web_fonts: Arc::new(AtomicUsize::new(0)),
             root_flow: RefCell::new(None),
             document_shared_lock: None,
             running_animations: ServoArc::new(RwLock::new(FnvHashMap::default())),
@@ -570,7 +548,7 @@ impl LayoutThread {
             viewport_size: Size2D::new(Au(0), Au(0)),
             webrender_api: webrender_api_sender.create_api(),
             webrender_document,
-            stylist,
+            stylist: Stylist::new(device, QuirksMode::NoQuirks),
             stylesheets: StylesheetSet::new(),
             rw_data: Arc::new(Mutex::new(
                 LayoutThreadData {
@@ -718,12 +696,7 @@ impl LayoutThread {
         match request {
             Msg::AddStylesheet(stylesheet, before_stylesheet) => {
                 let guard = stylesheet.shared_lock.read();
-
+                self.handle_add_stylesheet(&stylesheet, &guard);
-                self.handle_add_stylesheet(
-                    &stylesheet,
-                    &guard,
-                    possibly_locked_rw_data,
-                );
 
                 match before_stylesheet {
                     Some(insertion_point) => {
@@ -916,16 +889,13 @@ impl LayoutThread {
         let _ = self.parallel_traversal.take();
     }
 
-    fn handle_add_stylesheet<'a, 'b>(
+    fn handle_add_stylesheet(
         &self,
-        stylesheet: &ServoArc<Stylesheet>,
+        stylesheet: &Stylesheet,
         guard: &SharedRwLockReadGuard,
-        possibly_locked_rw_data: &mut RwData<'a, 'b>,
     ) {
         // Find all font-face rules and notify the font cache of them.
         // GWTODO: Need to handle unloading web fonts.
-
-        let rw_data = possibly_locked_rw_data.lock();
         if stylesheet.is_effective_for_device(self.stylist.device(), &guard) {
             add_font_face_rules(&*stylesheet,
                                 &guard,
@@ -934,8 +904,6 @@ impl LayoutThread {
                                 &self.font_cache_sender,
                                 &self.outstanding_web_fonts);
         }
-
-        possibly_locked_rw_data.block(rw_data);
     }
 
     /// Advances the animation clock of the document.
@@ -1215,6 +1183,7 @@ impl LayoutThread {
         self.document_shared_lock = Some(document_shared_lock.clone());
         let author_guard = document_shared_lock.read();
         let device = Device::new(MediaType::screen(), initial_viewport, device_pixel_ratio);
+        let sheet_origins_affected_by_device_change =
             self.stylist.set_device(device, &author_guard, self.stylesheets.iter());
 
         self.viewport_size =
@@ -1269,12 +1238,35 @@ impl LayoutThread {
         };
 
         let needs_dirtying = {
-            let mut extra_data = Default::default();
+            debug!("Flushing stylist");
-            let (iter, mut origins_dirty) = self.stylesheets.flush(Some(element));
+
-            if data.stylesheets_changed {
+            let mut origins_dirty = sheet_origins_affected_by_device_change;
-                origins_dirty = OriginSet::all();
+
+            debug!("Device changes: {:?}", origins_dirty);
+
+            if self.first_reflow.get() {
+                debug!("First reflow, rebuilding user and UA rules");
+
+                origins_dirty |= Origin::User;
+                origins_dirty |= Origin::UserAgent;
+                for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
+                    self.handle_add_stylesheet(stylesheet, &ua_or_user_guard);
                 }
-            let origins_rebuilt = self.stylist.rebuild(
+            }
+
+            let (iter, invalidation_origins_dirty) = self.stylesheets.flush(Some(element));
+            debug!("invalidation: {:?}", invalidation_origins_dirty);
+
+            origins_dirty |= invalidation_origins_dirty;
+
+            if data.stylesheets_changed {
+                debug!("Doc sheets changed, flushing author sheets too");
+                origins_dirty |= Origin::Author;
+            }
+
+            if !origins_dirty.is_empty() {
+                let mut extra_data = Default::default();
+                self.stylist.rebuild(
                     iter,
                     &guards,
                     Some(ua_stylesheets),
@@ -1282,7 +1274,9 @@ impl LayoutThread {
                     &mut extra_data,
                     origins_dirty,
                 );
-            !origins_rebuilt.is_empty()
+            }
+
+            !origins_dirty.is_empty()
         };
 
         let needs_reflow = viewport_size_changed && !needs_dirtying;

components/style/stylist.rs

@@ -78,9 +78,6 @@ pub struct Stylist {
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "defined in selectors")]
     quirks_mode: QuirksMode,
 
-    /// If true, the device has changed, and the stylist needs to be updated.
-    is_device_dirty: bool,
-
     /// Selector maps for all of the style sheets in the stylist, after
     /// evalutaing media rules against the current device, split out per
     /// cascade level.
@@ -130,7 +127,6 @@ impl Stylist {
         Stylist {
             viewport_constraints: None,
             device: device,
-            is_device_dirty: true,
             quirks_mode: quirks_mode,
 
             cascade_data: Default::default(),
@@ -184,10 +180,6 @@ impl Stylist {
 
     /// Rebuild the stylist for the given document stylesheets, and optionally
     /// with a set of user agent stylesheets.
-    ///
-    /// This method resets all the style data each time the stylesheets change
-    /// (which is indicated by the `stylesheets_changed` parameter), or the
-    /// device is dirty, which means we need to re-evaluate media queries.
     pub fn rebuild<'a, I, S>(
         &mut self,
         doc_stylesheets: I,
@@ -195,19 +187,13 @@ impl Stylist {
         ua_stylesheets: Option<&UserAgentStylesheets>,
         author_style_disabled: bool,
         extra_data: &mut PerOrigin<ExtraStyleData>,
-        mut origins_to_rebuild: OriginSet,
+        origins_to_rebuild: OriginSet,
-    ) -> OriginSet
+    )
     where
         I: Iterator<Item = &'a S> + Clone,
         S: StylesheetInDocument + ToMediaListKey + 'static,
     {
-        if self.is_device_dirty {
+        debug_assert!(!origins_to_rebuild.is_empty());
-            origins_to_rebuild = OriginSet::all();
-        }
-
-        if origins_to_rebuild.is_empty() {
-            return origins_to_rebuild;
-        }
 
         self.num_rebuilds += 1;
 
@@ -252,19 +238,40 @@ impl Stylist {
 
         if let Some(ua_stylesheets) = ua_stylesheets {
             for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
+                let sheet_origin =
+                    stylesheet.contents(guards.ua_or_user).origin;
+
                 debug_assert!(matches!(
-                    stylesheet.contents(guards.ua_or_user).origin,
+                    sheet_origin,
-                    Origin::UserAgent | Origin::User));
+                    Origin::UserAgent | Origin::User
-                self.add_stylesheet(stylesheet, guards.ua_or_user, extra_data);
+                ));
+
+                if origins_to_rebuild.contains(sheet_origin.into()) {
+                    self.add_stylesheet(
+                        stylesheet,
+                        guards.ua_or_user,
+                        extra_data
+                    );
+                }
             }
 
             if self.quirks_mode != QuirksMode::NoQuirks {
                 let stylesheet = &ua_stylesheets.quirks_mode_stylesheet;
+                let sheet_origin =
+                    stylesheet.contents(guards.ua_or_user).origin;
+
                 debug_assert!(matches!(
-                    stylesheet.contents(guards.ua_or_user).origin,
+                    sheet_origin,
-                    Origin::UserAgent | Origin::User));
+                    Origin::UserAgent | Origin::User
-                self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet,
+                ));
-                                    guards.ua_or_user, extra_data);
+
+                if origins_to_rebuild.contains(sheet_origin.into()) {
+                    self.add_stylesheet(
+                        &ua_stylesheets.quirks_mode_stylesheet,
+                        guards.ua_or_user,
+                        extra_data
+                    );
+                }
             }
         }
 
@@ -280,9 +287,6 @@ impl Stylist {
         for stylesheet in sheets_to_add {
             self.add_stylesheet(stylesheet, guards.author, extra_data);
         }
-
-        self.is_device_dirty = false;
-        origins_to_rebuild
     }
 
     fn add_stylesheet<S>(
@@ -831,18 +835,14 @@ impl Stylist {
     /// Set a given device, which may change the styles that apply to the
     /// document.
     ///
+    /// Returns the sheet origins that were actually affected.
+    ///
     /// This means that we may need to rebuild style data even if the
     /// stylesheets haven't changed.
     ///
     /// Also, the device that arrives here may need to take the viewport rules
     /// into account.
     ///
-    /// TODO(emilio): Probably should be unified with `update`, right now I
-    /// don't think we take into account dynamic updates to viewport rules.
-    ///
-    /// Probably worth to make the stylist own a single `Device`, and have a
-    /// `update_device` function?
-    ///
     /// feature = "servo" because gecko only has one device, and manually tracks
     /// when the device is dirty.
     ///
@@ -854,7 +854,7 @@ impl Stylist {
         mut device: Device,
         guard: &SharedRwLockReadGuard,
         stylesheets: I,
-    )
+    ) -> OriginSet
     where
         I: Iterator<Item = &'a S> + Clone,
         S: StylesheetInDocument + ToMediaListKey + 'static,
@@ -875,11 +875,10 @@ impl Stylist {
         }
 
         self.device = device;
-        let features_changed = self.media_features_change_changed_style(
+        self.media_features_change_changed_style(
             stylesheets,
-            guard
+            guard,
-        );
+        )
-        self.is_device_dirty |= !features_changed.is_empty();
     }
 
     /// Returns whether, given a media feature change, any previously-applicable
@@ -1067,7 +1066,6 @@ impl Stylist {
               V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> + Debug,
               F: FnMut(&E, ElementSelectorFlags),
     {
-        debug_assert!(!self.is_device_dirty);
         // Gecko definitely has pseudo-elements with style attributes, like
         // ::-moz-color-swatch.
         debug_assert!(cfg!(feature = "gecko") ||
@@ -1217,13 +1215,6 @@ impl Stylist {
             .any(|(d, _)| d.mapped_ids.might_contain_hash(id.get_hash()))
     }
 
-    /// Return whether the device is dirty, that is, whether the screen size or
-    /// media type have changed (for now).
-    #[inline]
-    pub fn is_device_dirty(&self) -> bool {
-        self.is_device_dirty
-    }
-
     /// Returns the registered `@keyframes` animation for the specified name.
     #[inline]
     pub fn get_animation(&self, name: &Atom) -> Option<&KeyframesAnimation> {