From 23b92d54d48274f4e80521c427a959cfd4cb646c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 8 Feb 2019 19:57:00 +0100 Subject: [PATCH] Remove stylesheets ownership from DocumentOrShadowRoot --- components/script/dom/bindings/trace.rs | 12 ++ components/script/dom/document.rs | 45 +++-- components/script/dom/documentorshadowroot.rs | 62 ++----- components/script/dom/htmlstyleelement.rs | 2 +- components/script/dom/shadowroot.rs | 31 +++- components/style/author_styles.rs | 2 + components/style/stylesheet_set.rs | 168 ++++++++++++------ components/style/stylist.rs | 2 +- 8 files changed, 200 insertions(+), 124 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 06ee56e173a..2d568208b69 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -119,6 +119,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize}; use std::sync::{Arc, Mutex}; use std::time::{Instant, SystemTime}; use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto}; +use style::author_styles::AuthorStyles; use style::context::QuirksMode; use style::dom::OpaqueNode; use style::element_state::*; @@ -130,6 +131,7 @@ use style::stylesheet_set::{AuthorStylesheetSet, DocumentStylesheetSet}; use style::stylesheets::keyframes_rule::Keyframe; use style::stylesheets::{CssRules, FontFaceRule, KeyframesRule, MediaRule, Stylesheet}; use style::stylesheets::{ImportRule, NamespaceRule, StyleRule, SupportsRule, ViewportRule}; +use style::stylist::CascadeData; use style::values::specified::Length; use tendril::fmt::UTF8; use tendril::stream::LossyDecoder; @@ -499,6 +501,7 @@ unsafe_no_jsmanaged_fields!(Rotation3D, Transform2D, Transform3D) unsafe_no_jsmanaged_fields!(Point2D, Vector2D, Rect); unsafe_no_jsmanaged_fields!(Rect, RigidTransform3D); unsafe_no_jsmanaged_fields!(StyleSheetListOwner); +unsafe_no_jsmanaged_fields!(CascadeData); unsafe impl<'a> JSTraceable for &'a str { #[inline] @@ -728,6 +731,15 @@ where } } +unsafe impl JSTraceable for AuthorStyles +where + S: JSTraceable + ::style::stylesheets::StylesheetInDocument + PartialEq + 'static, +{ + unsafe fn trace(&self, tracer: *mut JSTracer) { + self.stylesheets.trace(tracer) + } +} + unsafe impl JSTraceable for LossyDecoder where Sink: JSTraceable + TendrilSink, diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index ad4099bdbf9..977bff505dc 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -156,7 +156,7 @@ use style::selector_parser::{RestyleDamage, Snapshot}; use style::shared_lock::SharedRwLock as StyleSharedRwLock; use style::str::{split_html_space_chars, str_join}; use style::stylesheet_set::DocumentStylesheetSet; -use style::stylesheets::Stylesheet; +use style::stylesheets::{Origin, OriginSet, Stylesheet}; use url::percent_encoding::percent_decode; use url::Host; @@ -574,7 +574,7 @@ impl Document { // FIXME: This should check the dirty bit on the document, // not the document element. Needs some layout changes to make // that workable. - self.document_or_shadow_root.stylesheets_have_changed() || + self.stylesheets.borrow().has_changed() || self.GetDocumentElement().map_or(false, |root| { root.upcast::().has_dirty_descendants() || !self.pending_restyles.borrow().is_empty() || @@ -1536,7 +1536,7 @@ impl Document { } pub fn invalidate_stylesheets(&self) { - self.document_or_shadow_root.invalidate_stylesheets(); + self.stylesheets.borrow_mut().force_dirty(OriginSet::all()); // Mark the document element dirty so a reflow will be performed. // @@ -2821,9 +2821,9 @@ impl Document { // and normal stylesheets additions / removals, because in the last case // the layout thread already has that information and we could avoid // dirtying the whole thing. - let have_changed = self.document_or_shadow_root.stylesheets_have_changed(); - self.document_or_shadow_root - .flush_stylesheets_without_invalidation(); + let mut stylesheets = self.stylesheets.borrow_mut(); + let have_changed = stylesheets.has_changed(); + stylesheets.flush_without_invalidation(); have_changed } @@ -4550,24 +4550,47 @@ impl PendingScript { impl StyleSheetListOwner for Dom { fn stylesheet_count(&self) -> usize { - self.document_or_shadow_root.stylesheet_count() + self.stylesheets.borrow().len() } fn stylesheet_at(&self, index: usize) -> Option> { - self.document_or_shadow_root.stylesheet_at(index) + let stylesheets = self.stylesheets.borrow(); + + stylesheets + .get(Origin::Author, index) + .and_then(|s| s.owner.upcast::().get_cssom_stylesheet()) } /// Add a stylesheet owned by `owner` to the list of document sheets, in the /// correct tree position. #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily. fn add_stylesheet(&self, owner: &Element, sheet: Arc) { - self.document_or_shadow_root - .add_stylesheet(owner, sheet, self.style_shared_lock()); + let stylesheets = &mut *self.stylesheets.borrow_mut(); + let insertion_point = stylesheets + .iter() + .map(|(sheet, _origin)| sheet) + .find(|sheet_in_doc| { + owner + .upcast::() + .is_before(sheet_in_doc.owner.upcast()) + }) + .cloned(); + self.document_or_shadow_root.add_stylesheet( + owner, + stylesheets, + sheet, + insertion_point, + self.style_shared_lock(), + ); } /// Remove a stylesheet owned by `owner` from the list of document sheets. #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily. fn remove_stylesheet(&self, owner: &Element, s: &Arc) { - self.document_or_shadow_root.remove_stylesheet(owner, s) + self.document_or_shadow_root.remove_stylesheet( + owner, + s, + &mut *self.stylesheets.borrow_mut(), + ) } } diff --git a/components/script/dom/documentorshadowroot.rs b/components/script/dom/documentorshadowroot.rs index b1ea9ee1549..577df4bfd51 100644 --- a/components/script/dom/documentorshadowroot.rs +++ b/components/script/dom/documentorshadowroot.rs @@ -2,16 +2,14 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeBinding::NodeMethods; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::num::Finite; use crate::dom::bindings::root::{Dom, DomRoot}; -use crate::dom::cssstylesheet::CSSStyleSheet; use crate::dom::element::Element; use crate::dom::htmlelement::HTMLElement; use crate::dom::htmlmetaelement::HTMLMetaElement; -use crate::dom::node::{self, Node}; +use crate::dom::node; use crate::dom::window::Window; use euclid::Point2D; use js::jsapi::JS_GetRuntime; @@ -22,15 +20,15 @@ use std::fmt; use style::context::QuirksMode; use style::media_queries::MediaList; use style::shared_lock::{SharedRwLock as StyleSharedRwLock, SharedRwLockReadGuard}; -use style::stylesheet_set::DocumentStylesheetSet; -use style::stylesheets::{CssRule, Origin, OriginSet, Stylesheet}; +use style::stylesheet_set::StylesheetSet; +use style::stylesheets::{CssRule, Origin, Stylesheet}; #[derive(Clone, JSTraceable, MallocSizeOf)] #[must_root] pub struct StyleSheetInDocument { #[ignore_malloc_size_of = "Arc"] - sheet: Arc, - owner: Dom, + pub sheet: Arc, + pub owner: Dom, } impl fmt::Debug for StyleSheetInDocument { @@ -72,16 +70,12 @@ impl ::style::stylesheets::StylesheetInDocument for StyleSheetInDocument { #[derive(JSTraceable, MallocSizeOf)] pub struct DocumentOrShadowRoot { window: Dom, - /// List of stylesheets associated with nodes in this document or shadow root. - /// |None| if the list needs to be refreshed. - stylesheets: DomRefCell>, } impl DocumentOrShadowRoot { pub fn new(window: &Window) -> Self { Self { window: Dom::from_ref(window), - stylesheets: DomRefCell::new(DocumentStylesheetSet::new()), } } @@ -207,33 +201,14 @@ impl DocumentOrShadowRoot { } } - pub fn stylesheet_count(&self) -> usize { - self.stylesheets.borrow().len() - } - - pub fn stylesheet_at(&self, index: usize) -> Option> { - let stylesheets = self.stylesheets.borrow(); - - stylesheets - .get(Origin::Author, index) - .and_then(|s| s.owner.upcast::().get_cssom_stylesheet()) - } - - pub fn stylesheets_have_changed(&self) -> bool { - self.stylesheets.borrow().has_changed() - } - - pub fn invalidate_stylesheets(&self) { - self.stylesheets.borrow_mut().force_dirty(OriginSet::all()); - } - - pub fn flush_stylesheets_without_invalidation(&self) { - self.stylesheets.borrow_mut().flush_without_invalidation(); - } - /// Remove a stylesheet owned by `owner` from the list of document sheets. #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily. - pub fn remove_stylesheet(&self, owner: &Element, s: &Arc) { + pub fn remove_stylesheet( + &self, + owner: &Element, + s: &Arc, + stylesheets: &mut StylesheetSet, + ) { self.window .layout_chan() .send(Msg::RemoveStylesheet(s.clone())) @@ -242,7 +217,7 @@ impl DocumentOrShadowRoot { let guard = s.shared_lock.read(); // FIXME(emilio): Would be nice to remove the clone, etc. - self.stylesheets.borrow_mut().remove_stylesheet( + stylesheets.remove_stylesheet( None, StyleSheetInDocument { sheet: s.clone(), @@ -258,7 +233,9 @@ impl DocumentOrShadowRoot { pub fn add_stylesheet( &self, owner: &Element, + stylesheets: &mut StylesheetSet, sheet: Arc, + insertion_point: Option, style_shared_lock: &StyleSharedRwLock, ) { // FIXME(emilio): It'd be nice to unify more code between the elements @@ -269,17 +246,6 @@ impl DocumentOrShadowRoot { "Wat" ); - let mut stylesheets = self.stylesheets.borrow_mut(); - let insertion_point = stylesheets - .iter() - .map(|(sheet, _origin)| sheet) - .find(|sheet_in_doc| { - owner - .upcast::() - .is_before(sheet_in_doc.owner.upcast()) - }) - .cloned(); - self.window .layout_chan() .send(Msg::AddStylesheet( diff --git a/components/script/dom/htmlstyleelement.rs b/components/script/dom/htmlstyleelement.rs index f8465874a38..763afcf1bdb 100644 --- a/components/script/dom/htmlstyleelement.rs +++ b/components/script/dom/htmlstyleelement.rs @@ -82,7 +82,7 @@ impl HTMLStyleElement { pub fn parse_own_css(&self) { let node = self.upcast::(); let element = self.upcast::(); - assert!(node.is_in_doc()); + assert!(node.is_connected()); let window = window_from_node(node); let doc = document_from_node(self); diff --git a/components/script/dom/shadowroot.rs b/components/script/dom/shadowroot.rs index 3b1e9d34b87..7790423fb0a 100644 --- a/components/script/dom/shadowroot.rs +++ b/components/script/dom/shadowroot.rs @@ -19,7 +19,7 @@ use crate::dom::stylesheetlist::{StyleSheetList, StyleSheetListOwner}; use crate::dom::window::Window; use dom_struct::dom_struct; use servo_arc::Arc; -use style::stylesheet_set::AuthorStylesheetSet; +use style::author_styles::AuthorStyles; use style::stylesheets::Stylesheet; // https://dom.spec.whatwg.org/#interface-shadowroot @@ -31,7 +31,7 @@ pub struct ShadowRoot { host: Dom, /// List of stylesheets associated with nodes in this shadow tree. /// |None| if the list needs to be refreshed. - stylesheets: DomRefCell>, + author_styles: DomRefCell>, stylesheet_list: MutNullableDom, window: Dom, } @@ -48,7 +48,7 @@ impl ShadowRoot { document_or_shadow_root: DocumentOrShadowRoot::new(document.window()), document: Dom::from_ref(document), host: Dom::from_ref(host), - stylesheets: DomRefCell::new(AuthorStylesheetSet::new()), + author_styles: DomRefCell::new(AuthorStyles::new()), stylesheet_list: MutNullableDom::new(None), window: Dom::from_ref(document.window()), } @@ -131,20 +131,35 @@ impl LayoutShadowRootHelpers for LayoutDom { impl StyleSheetListOwner for Dom { fn stylesheet_count(&self) -> usize { - self.document_or_shadow_root.stylesheet_count() + self.author_styles.borrow().stylesheets.len() } fn stylesheet_at(&self, index: usize) -> Option> { - self.document_or_shadow_root.stylesheet_at(index) + let stylesheets = &self.author_styles.borrow().stylesheets; + + stylesheets + .get(index) + .and_then(|s| s.owner.upcast::().get_cssom_stylesheet()) } /// Add a stylesheet owned by `owner` to the list of shadow root sheets, in the /// correct tree position. #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily. fn add_stylesheet(&self, owner: &Element, sheet: Arc) { + let stylesheets = &mut self.author_styles.borrow_mut().stylesheets; + let insertion_point = stylesheets + .iter() + .find(|sheet_in_shadow| { + owner + .upcast::() + .is_before(sheet_in_shadow.owner.upcast()) + }) + .cloned(); self.document_or_shadow_root.add_stylesheet( owner, + stylesheets, sheet, + insertion_point, self.document.style_shared_lock(), ); } @@ -152,6 +167,10 @@ impl StyleSheetListOwner for Dom { /// Remove a stylesheet owned by `owner` from the list of shadow root sheets. #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily. fn remove_stylesheet(&self, owner: &Element, s: &Arc) { - self.document_or_shadow_root.remove_stylesheet(owner, s) + self.document_or_shadow_root.remove_stylesheet( + owner, + s, + &mut self.author_styles.borrow_mut().stylesheets, + ) } } diff --git a/components/style/author_styles.rs b/components/style/author_styles.rs index 37eb68ff4f7..46f95e1cac9 100644 --- a/components/style/author_styles.rs +++ b/components/style/author_styles.rs @@ -18,6 +18,7 @@ use crate::stylist::CascadeData; /// A set of author stylesheets and their computed representation, such as the /// ones used for ShadowRoot and XBL. +#[derive(MallocSizeOf)] pub struct AuthorStyles where S: StylesheetInDocument + PartialEq + 'static, @@ -29,6 +30,7 @@ where pub data: CascadeData, /// The quirks mode of the last stylesheet flush, used because XBL sucks and /// we should really fix it, see bug 1406875. + #[ignore_malloc_size_of = "XXX"] pub quirks_mode: QuirksMode, } diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 409978c3a9e..9a53339f5f9 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -373,69 +373,113 @@ where invalidations: StylesheetInvalidationSet, } +/// Functionality common to DocumentStylesheetSet and AuthorStylesheetSet. +pub trait StylesheetSet +where + S: StylesheetInDocument + PartialEq + 'static, +{ + /// Appends a new stylesheet to the current set. + /// + /// No device implies not computing invalidations. + fn append_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard, + ); + + /// Insert a given stylesheet before another stylesheet in the document. + fn insert_stylesheet_before( + &mut self, + device: Option<&Device>, + sheet: S, + before_sheet: S, + guard: &SharedRwLockReadGuard, + ); + + /// Remove a given stylesheet from the set. + fn remove_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard, + ); +} + /// This macro defines methods common to DocumentStylesheetSet and /// AuthorStylesheetSet. /// /// We could simplify the setup moving invalidations to SheetCollection, but /// that would imply not sharing invalidations across origins of the same /// documents, which is slightly annoying. -macro_rules! sheet_set_methods { - ($set_name:expr) => { - fn collect_invalidations_for( - &mut self, - device: Option<&Device>, - sheet: &S, - guard: &SharedRwLockReadGuard, - ) { - if let Some(device) = device { - self.invalidations.collect_invalidations_for(device, sheet, guard); +macro_rules! stylesheetset_impl { + ($set_name:expr, $set_type:ty) => { + impl $set_type + where + S: StylesheetInDocument + PartialEq + 'static, + { + fn collect_invalidations_for( + &mut self, + device: Option<&Device>, + sheet: &S, + guard: &SharedRwLockReadGuard, + ) { + if let Some(device) = device { + self.invalidations + .collect_invalidations_for(device, sheet, guard); + } } } - /// Appends a new stylesheet to the current set. - /// - /// No device implies not computing invalidations. - pub fn append_stylesheet( - &mut self, - device: Option<&Device>, - sheet: S, - guard: &SharedRwLockReadGuard, - ) { - debug!(concat!($set_name, "::append_stylesheet")); - self.collect_invalidations_for(device, &sheet, guard); - let collection = self.collection_for(&sheet, guard); - collection.append(sheet); + impl StylesheetSet for $set_type + where + S: StylesheetInDocument + PartialEq + 'static, + { + /// Appends a new stylesheet to the current set. + /// + /// No device implies not computing invalidations. + fn append_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard, + ) { + debug!(concat!($set_name, "::append_stylesheet")); + self.collect_invalidations_for(device, &sheet, guard); + let collection = self.collection_for(&sheet, guard); + collection.append(sheet); + } + + /// Insert a given stylesheet before another stylesheet in the document. + fn insert_stylesheet_before( + &mut self, + device: Option<&Device>, + sheet: S, + before_sheet: S, + guard: &SharedRwLockReadGuard, + ) { + debug!(concat!($set_name, "::insert_stylesheet_before")); + self.collect_invalidations_for(device, &sheet, guard); + + let collection = self.collection_for(&sheet, guard); + collection.insert_before(sheet, &before_sheet); + } + + /// Remove a given stylesheet from the set. + fn remove_stylesheet( + &mut self, + device: Option<&Device>, + sheet: S, + guard: &SharedRwLockReadGuard, + ) { + debug!(concat!($set_name, "::remove_stylesheet")); + self.collect_invalidations_for(device, &sheet, guard); + + let collection = self.collection_for(&sheet, guard); + collection.remove(&sheet) + } } - - /// Insert a given stylesheet before another stylesheet in the document. - pub fn insert_stylesheet_before( - &mut self, - device: Option<&Device>, - sheet: S, - before_sheet: S, - guard: &SharedRwLockReadGuard, - ) { - debug!(concat!($set_name, "::insert_stylesheet_before")); - self.collect_invalidations_for(device, &sheet, guard); - - let collection = self.collection_for(&sheet, guard); - collection.insert_before(sheet, &before_sheet); - } - - /// Remove a given stylesheet from the set. - pub fn remove_stylesheet( - &mut self, - device: Option<&Device>, - sheet: S, - guard: &SharedRwLockReadGuard, - ) { - debug!(concat!($set_name, "::remove_stylesheet")); - self.collect_invalidations_for(device, &sheet, guard); - - let collection = self.collection_for(&sheet, guard); - collection.remove(&sheet) - } - } + }; } impl DocumentStylesheetSet @@ -459,8 +503,6 @@ where self.collections.borrow_mut_for_origin(&origin) } - sheet_set_methods!("DocumentStylesheetSet"); - /// Returns the number of stylesheets in the set. pub fn len(&self) -> usize { self.collections @@ -539,6 +581,8 @@ where } } +stylesheetset_impl!("DocumentStylesheetSet", DocumentStylesheetSet); + /// The set of stylesheets effective for a given XBL binding or Shadow Root. #[derive(MallocSizeOf)] pub struct AuthorStylesheetSet @@ -585,6 +629,16 @@ where self.collection.len() == 0 } + /// Returns the `index`th stylesheet in the collection of author styles if present. + pub fn get(&self, index: usize) -> Option<&S> { + self.collection.get(index) + } + + /// Returns the number of author stylesheets. + pub fn len(&self) -> usize { + self.collection.len() + } + fn collection_for( &mut self, _sheet: &S, @@ -593,8 +647,6 @@ where &mut self.collection } - sheet_set_methods!("AuthorStylesheetSet"); - /// Iterate over the list of stylesheets. pub fn iter(&self) -> StylesheetCollectionIterator { self.collection.iter() @@ -626,3 +678,5 @@ where } } } + +stylesheetset_impl!("AuthorStylesheetSet", AuthorStylesheetSet); diff --git a/components/style/stylist.rs b/components/style/stylist.rs index ece14e9896f..045d6b01cd1 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -23,7 +23,7 @@ use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, S use crate::selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap}; use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind}; -use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher}; +use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher, StylesheetSet}; use crate::stylesheets::keyframes_rule::KeyframesAnimation; use crate::stylesheets::viewport_rule::{self, MaybeNew, ViewportRule}; use crate::stylesheets::StyleRule;