Remove stylesheets ownership from DocumentOrShadowRoot

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<f64>, Transform2D<f32>, Transform3D<f64>)
 unsafe_no_jsmanaged_fields!(Point2D<f32>, Vector2D<f32>, Rect<Au>);
 unsafe_no_jsmanaged_fields!(Rect<f32>, RigidTransform3D<f64>);
 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<S> JSTraceable for AuthorStyles<S>
+where
+    S: JSTraceable + ::style::stylesheets::StylesheetInDocument + PartialEq + 'static,
+{
+    unsafe fn trace(&self, tracer: *mut JSTracer) {
+        self.stylesheets.trace(tracer)
+    }
+}
+
 unsafe impl<Sink> JSTraceable for LossyDecoder<Sink>
 where
     Sink: JSTraceable + TendrilSink<UTF8>,

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::<Node>().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();
+        let mut stylesheets = self.stylesheets.borrow_mut();
-        self.document_or_shadow_root
+        let have_changed = stylesheets.has_changed();
-            .flush_stylesheets_without_invalidation();
+        stylesheets.flush_without_invalidation();
         have_changed
     }
 
@@ -4550,24 +4550,47 @@ impl PendingScript {
 
 impl StyleSheetListOwner for Dom<Document> {
     fn stylesheet_count(&self) -> usize {
-        self.document_or_shadow_root.stylesheet_count()
+        self.stylesheets.borrow().len()
     }
 
     fn stylesheet_at(&self, index: usize) -> Option<DomRoot<CSSStyleSheet>> {
-        self.document_or_shadow_root.stylesheet_at(index)
+        let stylesheets = self.stylesheets.borrow();
+
+        stylesheets
+            .get(Origin::Author, index)
+            .and_then(|s| s.owner.upcast::<Node>().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<Stylesheet>) {
-        self.document_or_shadow_root
+        let stylesheets = &mut *self.stylesheets.borrow_mut();
-            .add_stylesheet(owner, sheet, self.style_shared_lock());
+        let insertion_point = stylesheets
+            .iter()
+            .map(|(sheet, _origin)| sheet)
+            .find(|sheet_in_doc| {
+                owner
+                    .upcast::<Node>()
+                    .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<Stylesheet>) {
-        self.document_or_shadow_root.remove_stylesheet(owner, s)
+        self.document_or_shadow_root.remove_stylesheet(
+            owner,
+            s,
+            &mut *self.stylesheets.borrow_mut(),
+        )
     }
 }

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::stylesheet_set::StylesheetSet;
-use style::stylesheets::{CssRule, Origin, OriginSet, Stylesheet};
+use style::stylesheets::{CssRule, Origin, Stylesheet};
 
 #[derive(Clone, JSTraceable, MallocSizeOf)]
 #[must_root]
 pub struct StyleSheetInDocument {
     #[ignore_malloc_size_of = "Arc"]
-    sheet: Arc<Stylesheet>,
+    pub sheet: Arc<Stylesheet>,
-    owner: Dom<Element>,
+    pub owner: Dom<Element>,
 }
 
 impl fmt::Debug for StyleSheetInDocument {
@@ -72,16 +70,12 @@ impl ::style::stylesheets::StylesheetInDocument for StyleSheetInDocument {
 #[derive(JSTraceable, MallocSizeOf)]
 pub struct DocumentOrShadowRoot {
     window: Dom<Window>,
-    /// List of stylesheets associated with nodes in this document or shadow root.
-    /// |None| if the list needs to be refreshed.
-    stylesheets: DomRefCell<DocumentStylesheetSet<StyleSheetInDocument>>,
 }
 
 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<DomRoot<CSSStyleSheet>> {
-        let stylesheets = self.stylesheets.borrow();
-
-        stylesheets
-            .get(Origin::Author, index)
-            .and_then(|s| s.owner.upcast::<Node>().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<Stylesheet>) {
+    pub fn remove_stylesheet(
+        &self,
+        owner: &Element,
+        s: &Arc<Stylesheet>,
+        stylesheets: &mut StylesheetSet<StyleSheetInDocument>,
+    ) {
         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<StyleSheetInDocument>,
         sheet: Arc<Stylesheet>,
+        insertion_point: Option<StyleSheetInDocument>,
         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::<Node>()
-                    .is_before(sheet_in_doc.owner.upcast())
-            })
-            .cloned();
-
         self.window
             .layout_chan()
             .send(Msg::AddStylesheet(

components/script/dom/htmlstyleelement.rs

@@ -82,7 +82,7 @@ impl HTMLStyleElement {
     pub fn parse_own_css(&self) {
         let node = self.upcast::<Node>();
         let element = self.upcast::<Element>();
-        assert!(node.is_in_doc());
+        assert!(node.is_connected());
 
         let window = window_from_node(node);
         let doc = document_from_node(self);

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<Element>,
     /// List of stylesheets associated with nodes in this shadow tree.
     /// |None| if the list needs to be refreshed.
-    stylesheets: DomRefCell<AuthorStylesheetSet<StyleSheetInDocument>>,
+    author_styles: DomRefCell<AuthorStyles<StyleSheetInDocument>>,
     stylesheet_list: MutNullableDom<StyleSheetList>,
     window: Dom<Window>,
 }
@@ -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<ShadowRoot> {
 
 impl StyleSheetListOwner for Dom<ShadowRoot> {
     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<DomRoot<CSSStyleSheet>> {
-        self.document_or_shadow_root.stylesheet_at(index)
+        let stylesheets = &self.author_styles.borrow().stylesheets;
+
+        stylesheets
+            .get(index)
+            .and_then(|s| s.owner.upcast::<Node>().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<Stylesheet>) {
+        let stylesheets = &mut self.author_styles.borrow_mut().stylesheets;
+        let insertion_point = stylesheets
+            .iter()
+            .find(|sheet_in_shadow| {
+                owner
+                    .upcast::<Node>()
+                    .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<ShadowRoot> {
     /// 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<Stylesheet>) {
-        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,
+        )
     }
 }

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<S>
 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,
 }
 

components/style/stylesheet_set.rs

@@ -373,69 +373,113 @@ where
     invalidations: StylesheetInvalidationSet,
 }
 
+/// Functionality common to DocumentStylesheetSet and AuthorStylesheetSet.
+pub trait StylesheetSet<S>
+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 {
+macro_rules! stylesheetset_impl {
-    ($set_name:expr) => {
+    ($set_name:expr, $set_type:ty) => {
-        fn collect_invalidations_for(
+        impl<S> $set_type
-            &mut self,
+        where
-            device: Option<&Device>,
+            S: StylesheetInDocument + PartialEq + 'static,
-            sheet: &S,
+        {
-            guard: &SharedRwLockReadGuard,
+            fn collect_invalidations_for(
-        ) {
+                &mut self,
-            if let Some(device) = device {
+                device: Option<&Device>,
-                self.invalidations.collect_invalidations_for(device, sheet, guard);
+                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.
+        impl<S> StylesheetSet<S> for $set_type
-        ///
+        where
-        /// No device implies not computing invalidations.
+            S: StylesheetInDocument + PartialEq + 'static,
-        pub fn append_stylesheet(
+        {
-            &mut self,
+            /// Appends a new stylesheet to the current set.
-            device: Option<&Device>,
+            ///
-            sheet: S,
+            /// No device implies not computing invalidations.
-            guard: &SharedRwLockReadGuard,
+            fn append_stylesheet(
-        ) {
+                &mut self,
-            debug!(concat!($set_name, "::append_stylesheet"));
+                device: Option<&Device>,
-            self.collect_invalidations_for(device, &sheet, guard);
+                sheet: S,
-            let collection = self.collection_for(&sheet, guard);
+                guard: &SharedRwLockReadGuard,
-            collection.append(sheet);
+            ) {
+                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<S> DocumentStylesheetSet<S>
@@ -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<S>);
+
 /// The set of stylesheets effective for a given XBL binding or Shadow Root.
 #[derive(MallocSizeOf)]
 pub struct AuthorStylesheetSet<S>
@@ -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<S> {
         self.collection.iter()
@@ -626,3 +678,5 @@ where
         }
     }
 }
+
+stylesheetset_impl!("AuthorStylesheetSet", AuthorStylesheetSet<S>);

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;