diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 1a6bd945caf..097cf74a2ac 100644 --- a/components/layout_thread/lib.rs +++ b/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; @@ -434,20 +434,10 @@ impl<'a, 'b: 'a> RwData<'a, 'b> { /// use `block`. fn lock(&mut self) -> RWGuard<'b> { match self.possibly_locked_rw_data.take() { - None => RWGuard::Used(self.rw_data.lock().unwrap()), + None => RWGuard::Used(self.rw_data.lock().unwrap()), 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, - possibly_locked_rw_data, - ); + self.handle_add_stylesheet(&stylesheet, &guard); 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, 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,7 +1183,8 @@ 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); - self.stylist.set_device(device, &author_guard, self.stylesheets.iter()); + let sheet_origins_affected_by_device_change = + self.stylist.set_device(device, &author_guard, self.stylesheets.iter()); self.viewport_size = self.stylist.viewport_constraints().map_or(current_screen_size, |constraints| { @@ -1269,20 +1238,45 @@ impl LayoutThread { }; let needs_dirtying = { - let mut extra_data = Default::default(); - let (iter, mut origins_dirty) = self.stylesheets.flush(Some(element)); - if data.stylesheets_changed { - origins_dirty = OriginSet::all(); + debug!("Flushing stylist"); + + let mut origins_dirty = sheet_origins_affected_by_device_change; + + 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( - iter, - &guards, - Some(ua_stylesheets), - /* author_style_disabled = */ false, - &mut extra_data, - origins_dirty, - ); - !origins_rebuilt.is_empty() + + 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), + /* author_style_disabled = */ false, + &mut extra_data, + origins_dirty, + ); + } + + !origins_dirty.is_empty() }; let needs_reflow = viewport_size_changed && !needs_dirtying; diff --git a/components/style/animation.rs b/components/style/animation.rs index c1a49fca7a1..6736d39269f 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -437,21 +437,23 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender old_style, Arc::make_mut(new_style)); for property_animation in property_animations { - // Per [1], don't trigger a new transition if the end state for that transition is - // the same as that of a transition that's already running on the same node. - // - // [1]: https://drafts.csswg.org/css-transitions/#starting - if possibly_expired_animations.iter().any(|animation| { - animation.has_the_same_end_value_as(&property_animation) - }) { - continue - } - // Set the property to the initial value. + // // NB: get_mut is guaranteed to succeed since we called make_mut() // above. property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0); + // Per [1], don't trigger a new transition if the end state for that + // transition is the same as that of a transition that's already + // running on the same node. + // + // [1]: https://drafts.csswg.org/css-transitions/#starting + if possibly_expired_animations.iter().any(|animation| { + animation.has_the_same_end_value_as(&property_animation) + }) { + continue + } + // Kick off the animation. let box_style = new_style.get_box(); let now = timer.seconds(); diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 93d538d4c9c..e12dfe31a83 100644 --- a/components/style/stylist.rs +++ b/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, - mut origins_to_rebuild: OriginSet, - ) -> OriginSet + origins_to_rebuild: OriginSet, + ) where I: Iterator + Clone, S: StylesheetInDocument + ToMediaListKey + 'static, { - if self.is_device_dirty { - origins_to_rebuild = OriginSet::all(); - } - - if origins_to_rebuild.is_empty() { - return origins_to_rebuild; - } + debug_assert!(!origins_to_rebuild.is_empty()); 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, - Origin::UserAgent | Origin::User)); - self.add_stylesheet(stylesheet, guards.ua_or_user, extra_data); + sheet_origin, + Origin::UserAgent | Origin::User + )); + + 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, - Origin::UserAgent | Origin::User)); - self.add_stylesheet(&ua_stylesheets.quirks_mode_stylesheet, - guards.ua_or_user, extra_data); + sheet_origin, + Origin::UserAgent | Origin::User + )); + + 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( @@ -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 + 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 - ); - self.is_device_dirty |= !features_changed.is_empty(); + guard, + ) } /// Returns whether, given a media feature change, any previously-applicable @@ -1067,7 +1066,6 @@ impl Stylist { V: Push + VecLike + 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> { diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index f39a8b44904..7623a1a6a46 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -24766,7 +24766,7 @@ "support" ], "css/offset_properties_inline.html": [ - "e879ede3d52eed5ebdd90e988380ba4fa365817a", + "ed9cf0062ff6b08dc0cc578a0ae6acce146bec10", "testharness" ], "css/ol_japanese_iroha_a.html": [ diff --git a/tests/wpt/mozilla/tests/css/offset_properties_inline.html b/tests/wpt/mozilla/tests/css/offset_properties_inline.html index cb88fa3a246..9b93cde7fc0 100644 --- a/tests/wpt/mozilla/tests/css/offset_properties_inline.html +++ b/tests/wpt/mozilla/tests/css/offset_properties_inline.html @@ -30,6 +30,7 @@ -->