Don't expose any AtomicRefCell directly from style traits

This lets us experiment with how we store this data on the DOM side.

components/layout/query.rs

@@ -761,7 +761,7 @@ pub fn process_resolved_style_request<'dom>(
 
     // We call process_resolved_style_request after performing a whole-document
     // traversal, so in the common case, the element is styled.
-    if element.get_data().is_some() {
+    if element.has_data() {
         return process_resolved_style_request_internal(node, pseudo, property, layout_root);
     }
 

components/layout_thread/dom_wrapper.rs

@@ -30,7 +30,7 @@
 
 #![allow(unsafe_code)]
 
-use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
+use atomic_refcell::{AtomicRef, AtomicRefMut};
 use gfx_traits::ByteIndex;
 use html5ever::{LocalName, Namespace};
 use layout::data::StyleAndLayoutData;
@@ -551,8 +551,20 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         self.mutate_data().unwrap()
     }
 
-    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
+    /// Whether there is an ElementData container.
-        self.get_style_data().map(|data| &data.element_data)
+    fn has_data(&self) -> bool {
+        self.get_style_data().is_some()
+    }
+
+    /// Immutably borrows the ElementData.
+    fn borrow_data(&self) -> Option<AtomicRef<ElementData>> {
+        self.get_style_data().map(|data| data.element_data.borrow())
+    }
+
+    /// Mutably borrows the ElementData.
+    fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
+        self.get_style_data()
+            .map(|data| data.element_data.borrow_mut())
     }
 
     fn skip_item_display_fixup(&self) -> bool {
@@ -688,7 +700,7 @@ impl<'le> ServoLayoutElement<'le> {
     }
 
     fn get_style_data(&self) -> Option<&StyleData> {
-        self.get_style_and_layout_data().map(|opaque| {
+        self.get_style_and_layout_data().map(|opaque| unsafe {
             &opaque
                 .downcast_ref::<StyleAndLayoutData>()
                 .unwrap()
@@ -1027,7 +1039,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> {
 
     fn parent_style(&self) -> Arc<ComputedValues> {
         let parent = self.node.parent_node().unwrap().as_element().unwrap();
-        let parent_data = parent.get_data().unwrap().borrow();
+        let parent_data = parent.borrow_data().unwrap();
         parent_data.styles.primary().clone()
     }
 
@@ -1312,10 +1324,7 @@ impl<'le> ThreadSafeLayoutElement<'le> for ServoThreadSafeLayoutElement<'le> {
     }
 
     fn style_data(&self) -> AtomicRef<ElementData> {
-        self.element
+        self.element.borrow_data().expect("Unstyled layout node?")
-            .get_data()
-            .expect("Unstyled layout node?")
-            .borrow()
     }
 
     fn is_shadow_host(&self) -> bool {

components/layout_thread/lib.rs

@@ -1472,7 +1472,7 @@ impl LayoutThread {
 
             // If we haven't styled this node yet, we don't need to track a
             // restyle.
-            let style_data = match el.get_data() {
+            let mut style_data = match el.mutate_data() {
                 Some(d) => d,
                 None => {
                     unsafe { el.unset_snapshot_flags() };
@@ -1485,8 +1485,6 @@ impl LayoutThread {
                 map.insert(el.as_node().opaque(), s);
             }
 
-            let mut style_data = style_data.borrow_mut();
-
             // Stash the data on the element for processing by the style system.
             style_data.hint.insert(restyle.hint.into());
             style_data.damage = restyle.damage;

components/layout_thread_2020/dom_wrapper.rs

@@ -30,7 +30,7 @@
 
 #![allow(unsafe_code)]
 
-use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
+use atomic_refcell::{AtomicRef, AtomicRefMut};
 use gfx_traits::ByteIndex;
 use html5ever::{LocalName, Namespace};
 use layout::data::StyleAndLayoutData;
@@ -558,8 +558,20 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         self.mutate_data().unwrap()
     }
 
-    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
+    /// Whether there is an ElementData container.
-        self.get_style_data().map(|data| &data.element_data)
+    fn has_data(&self) -> bool {
+        self.get_style_data().is_some()
+    }
+
+    /// Immutably borrows the ElementData.
+    fn borrow_data(&self) -> Option<AtomicRef<ElementData>> {
+        self.get_style_data().map(|data| data.element_data.borrow())
+    }
+
+    /// Mutably borrows the ElementData.
+    fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
+        self.get_style_data()
+            .map(|data| data.element_data.borrow_mut())
     }
 
     fn skip_item_display_fixup(&self) -> bool {
@@ -695,7 +707,7 @@ impl<'le> ServoLayoutElement<'le> {
     }
 
     fn get_style_data(&self) -> Option<&StyleData> {
-        self.get_style_and_layout_data().map(|opaque| {
+        self.get_style_and_layout_data().map(|opaque| unsafe {
             &opaque
                 .downcast_ref::<StyleAndLayoutData>()
                 .unwrap()
@@ -1034,7 +1046,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> {
 
     fn parent_style(&self) -> Arc<ComputedValues> {
         let parent = self.node.parent_node().unwrap().as_element().unwrap();
-        let parent_data = parent.get_data().unwrap().borrow();
+        let parent_data = parent.borrow_data().unwrap();
         parent_data.styles.primary().clone()
     }
 
@@ -1319,10 +1331,7 @@ impl<'le> ThreadSafeLayoutElement<'le> for ServoThreadSafeLayoutElement<'le> {
     }
 
     fn style_data(&self) -> AtomicRef<ElementData> {
-        self.element
+        self.element.borrow_data().expect("Unstyled layout node?")
-            .get_data()
-            .expect("Unstyled layout node?")
-            .borrow()
     }
 
     fn is_shadow_host(&self) -> bool {

components/layout_thread_2020/lib.rs

@@ -1118,7 +1118,7 @@ impl LayoutThread {
 
             // If we haven't styled this node yet, we don't need to track a
             // restyle.
-            let style_data = match el.get_data() {
+            let mut style_data = match el.mutate_data() {
                 Some(d) => d,
                 None => {
                     unsafe { el.unset_snapshot_flags() };
@@ -1131,8 +1131,6 @@ impl LayoutThread {
                 map.insert(el.as_node().opaque(), s);
             }
 
-            let mut style_data = style_data.borrow_mut();
-
             // Stash the data on the element for processing by the style system.
             style_data.hint.insert(restyle.hint.into());
             style_data.damage = restyle.damage;

components/style/dom.rs

@@ -22,7 +22,7 @@ use crate::shared_lock::Locked;
 use crate::stylist::CascadeData;
 use crate::traversal_flags::TraversalFlags;
 use crate::{Atom, LocalName, Namespace, WeakAtom};
-use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
+use atomic_refcell::{AtomicRef, AtomicRefMut};
 use selectors::matching::{ElementSelectorFlags, QuirksMode, VisitedHandlingMode};
 use selectors::sink::Push;
 use selectors::Element as SelectorsElement;
@@ -689,18 +689,14 @@ pub trait TElement:
     /// Unsafe following the same reasoning as ensure_data.
     unsafe fn clear_data(&self);
 
-    /// Gets a reference to the ElementData container.
+    /// Whether there is an ElementData container.
-    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>>;
+    fn has_data(&self) -> bool;
 
     /// Immutably borrows the ElementData.
-    fn borrow_data(&self) -> Option<AtomicRef<ElementData>> {
+    fn borrow_data(&self) -> Option<AtomicRef<ElementData>>;
-        self.get_data().map(|x| x.borrow())
-    }
 
     /// Mutably borrows the ElementData.
-    fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
+    fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>>;
-        self.get_data().map(|x| x.borrow_mut())
-    }
 
     /// Whether we should skip any root- or item-based display property
     /// blockification on this element.  (This function exists so that Gecko

components/style/gecko/wrapper.rs

@@ -557,6 +557,11 @@ impl<'le> fmt::Debug for GeckoElement<'le> {
 }
 
 impl<'le> GeckoElement<'le> {
+    #[inline(always)]
+    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
+        unsafe { self.0.mServoData.get().as_ref() }
+    }
+
     #[inline(always)]
     fn attrs(&self) -> &[structs::AttrArray_InternalAttr] {
         unsafe {
@@ -1299,7 +1304,7 @@ impl<'le> TElement for GeckoElement<'le> {
     }
 
     unsafe fn set_handled_snapshot(&self) {
-        debug_assert!(self.get_data().is_some());
+        debug_assert!(self.has_data());
         self.set_flags(ELEMENT_HANDLED_SNAPSHOT as u32)
     }
 
@@ -1309,7 +1314,7 @@ impl<'le> TElement for GeckoElement<'le> {
     }
 
     unsafe fn set_dirty_descendants(&self) {
-        debug_assert!(self.get_data().is_some());
+        debug_assert!(self.has_data());
         debug!("Setting dirty descendants: {:?}", self);
         self.set_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32)
     }
@@ -1378,13 +1383,23 @@ impl<'le> TElement for GeckoElement<'le> {
         panic!("Atomic child count not implemented in Gecko");
     }
 
-    #[inline(always)]
+    /// Whether there is an ElementData container.
-    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
+    fn has_data(&self) -> bool {
-        unsafe { self.0.mServoData.get().as_ref() }
+        self.get_data().is_some()
+    }
+
+    /// Immutably borrows the ElementData.
+    fn borrow_data(&self) -> Option<AtomicRef<ElementData>> {
+        self.get_data().map(|x| x.borrow())
+    }
+
+    /// Mutably borrows the ElementData.
+    fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
+        self.get_data().map(|x| x.borrow_mut())
     }
 
     unsafe fn ensure_data(&self) -> AtomicRefMut<ElementData> {
-        if self.get_data().is_none() {
+        if !self.has_data() {
             debug!("Creating ElementData for {:?}", self);
             let ptr = Box::into_raw(Box::new(AtomicRefCell::new(ElementData::default())));
             self.0.mServoData.set(ptr);

components/style/invalidation/element/state_and_attributes.rs

@@ -90,7 +90,7 @@ pub fn invalidated_descendants<E>(element: E, child: E)
 where
     E: TElement,
 {
-    if child.get_data().is_none() {
+    if !child.has_data() {
         return;
     }
 

components/style/traversal.rs

@@ -845,7 +845,7 @@ where
                 //
                 // By consequence, any element without data has no descendants with
                 // data.
-                if kid.get_data().is_some() {
+                if kid.has_data() {
                     kid.clear_data();
                     parents.push(kid);
                 }