Auto merge of #26105 - servo:layout-2020-less-opaque, r=emilio

Make DOM own the style and layout data, in an UnsafeCell

2 files changed, 40 insertions, 53 deletions

diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs
index 9237e9d47e7..cbd0df1f9a4 100644
--- a/components/layout_thread/dom_wrapper.rs
+++ b/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;
@@ -70,7 +70,6 @@ use std::borrow::Cow;
 use std::fmt;
 use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
-use std::ptr::NonNull;
 use std::sync::atomic::Ordering;
 use std::sync::Arc as StdArc;
 use style::applicable_declarations::ApplicableDeclarationBlock;
@@ -92,12 +91,6 @@ use style::str::is_whitespace;
 use style::stylist::CascadeData;
 use style::CaseSensitivityExt;
 
-pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) {
-    let ptr = data.ptr.as_ptr() as *mut StyleData;
-    let non_opaque: *mut StyleAndLayoutData = ptr as *mut _;
-    let _ = Box::from_raw(non_opaque);
-}
-
 #[derive(Clone, Copy)]
 pub struct ServoLayoutNode<'dom> {
     /// The wrapped node.
@@ -237,7 +230,7 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
     }
 
     fn opaque(&self) -> OpaqueNode {
-        unsafe { self.get_jsmanaged().opaque() }
+        self.get_jsmanaged().opaque()
     }
 
     fn debug_id(self) -> usize {
@@ -276,10 +269,7 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> {
 
     unsafe fn initialize_data(&self) {
         if self.get_raw_data().is_none() {
-            let ptr: *mut StyleAndLayoutData = Box::into_raw(Box::new(StyleAndLayoutData::new()));
-            let opaque = OpaqueStyleAndLayoutData {
-                ptr: NonNull::new_unchecked(ptr as *mut StyleData),
-            };
+            let opaque = OpaqueStyleAndLayoutData::new(StyleAndLayoutData::new());
             self.init_style_and_layout_data(opaque);
         };
     }
@@ -297,34 +287,33 @@ impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> {
     }
 }
 
-impl<'ln> GetLayoutData<'ln> for ServoLayoutNode<'ln> {
-    fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> {
-        unsafe { self.get_jsmanaged().get_style_and_layout_data() }
+impl<'dom> GetLayoutData<'dom> for ServoLayoutNode<'dom> {
+    fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> {
+        self.get_jsmanaged().get_style_and_layout_data()
     }
 }
 
-impl<'le> GetLayoutData<'le> for ServoLayoutElement<'le> {
-    fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> {
+impl<'dom> GetLayoutData<'dom> for ServoLayoutElement<'dom> {
+    fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> {
         self.as_node().get_style_and_layout_data()
     }
 }
 
-impl<'ln> GetLayoutData<'ln> for ServoThreadSafeLayoutNode<'ln> {
-    fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> {
+impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutNode<'dom> {
+    fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> {
         self.node.get_style_and_layout_data()
     }
 }
 
-impl<'le> GetLayoutData<'le> for ServoThreadSafeLayoutElement<'le> {
-    fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> {
+impl<'dom> GetLayoutData<'dom> for ServoThreadSafeLayoutElement<'dom> {
+    fn get_style_and_layout_data(self) -> Option<&'dom OpaqueStyleAndLayoutData> {
         self.element.as_node().get_style_and_layout_data()
     }
 }
 
-impl<'ln> ServoLayoutNode<'ln> {
-    /// Returns the interior of this node as a `LayoutDom`. This is highly unsafe for layout to
-    /// call and as such is marked `unsafe`.
-    pub unsafe fn get_jsmanaged(&self) -> LayoutDom<'ln, Node> {
+impl<'dom> ServoLayoutNode<'dom> {
+    /// Returns the interior of this node as a `LayoutDom`.
+    pub fn get_jsmanaged(self) -> LayoutDom<'dom, Node> {
         self.node
     }
 }
@@ -548,7 +537,7 @@ impl<'le> TElement for ServoLayoutElement<'le> {
 
     unsafe fn clear_data(&self) {
         if self.get_raw_data().is_some() {
-            drop_style_and_layout_data(self.as_node().take_style_and_layout_data());
+            drop(self.as_node().take_style_and_layout_data());
         }
     }
 
@@ -557,11 +546,20 @@ impl<'le> TElement for ServoLayoutElement<'le> {
         self.mutate_data().unwrap()
     }
 
-    fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
-        unsafe {
-            self.get_style_and_layout_data()
-                .map(|d| &(*(d.ptr.as_ptr() as *mut StyleData)).element_data)
-        }
+    /// Whether there is an ElementData container.
+    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 {
@@ -697,10 +695,12 @@ impl<'le> ServoLayoutElement<'le> {
     }
 
     fn get_style_data(&self) -> Option<&StyleData> {
-        unsafe {
-            self.get_style_and_layout_data()
-                .map(|d| &*(d.ptr.as_ptr() as *mut StyleData))
-        }
+        self.get_style_and_layout_data().map(|opaque| {
+            &opaque
+                .downcast_ref::<StyleAndLayoutData>()
+                .unwrap()
+                .style_data
+        })
     }
 
     pub unsafe fn unset_snapshot_flags(&self) {
@@ -1034,7 +1034,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()
     }
 
@@ -1060,7 +1060,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> {
             })
     }
 
-    fn get_style_and_layout_data(&self) -> Option<OpaqueStyleAndLayoutData> {
+    fn get_style_and_layout_data(self) -> Option<&'ln OpaqueStyleAndLayoutData> {
         self.node.get_style_and_layout_data()
     }
 
@@ -1319,10 +1319,7 @@ impl<'le> ThreadSafeLayoutElement<'le> for ServoThreadSafeLayoutElement<'le> {
     }
 
     fn style_data(&self) -> AtomicRef<ElementData> {
-        self.element
-            .get_data()
-            .expect("Unstyled layout node?")
-            .borrow()
+        self.element.borrow_data().expect("Unstyled layout node?")
     }
 
     fn is_shadow_host(&self) -> bool {
diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 54f1ec8d656..4c458065986 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -23,7 +23,6 @@ extern crate profile_traits;
 
 mod dom_wrapper;
 
-use crate::dom_wrapper::drop_style_and_layout_data;
 use crate::dom_wrapper::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode};
 use app_units::Au;
 use crossbeam_channel::{unbounded, Receiver, Sender};
@@ -692,7 +691,6 @@ impl LayoutThread {
             Msg::GetRPC(..) => LayoutHangAnnotation::GetRPC,
             Msg::TickAnimations(..) => LayoutHangAnnotation::TickAnimations,
             Msg::AdvanceClockMs(..) => LayoutHangAnnotation::AdvanceClockMs,
-            Msg::ReapStyleAndLayoutData(..) => LayoutHangAnnotation::ReapStyleAndLayoutData,
             Msg::CollectReports(..) => LayoutHangAnnotation::CollectReports,
             Msg::PrepareToExit(..) => LayoutHangAnnotation::PrepareToExit,
             Msg::ExitNow => LayoutHangAnnotation::ExitNow,
@@ -833,9 +831,6 @@ impl LayoutThread {
                     webrender_api::ScrollClamping::ToContentBounds,
                 );
             },
-            Msg::ReapStyleAndLayoutData(dead_data) => unsafe {
-                drop_style_and_layout_data(dead_data)
-            },
             Msg::CollectReports(reports_chan) => {
                 self.collect_reports(reports_chan, possibly_locked_rw_data);
             },
@@ -967,9 +962,6 @@ impl LayoutThread {
         response_chan.send(()).unwrap();
         loop {
             match self.port.recv().unwrap() {
-                Msg::ReapStyleAndLayoutData(dead_data) => unsafe {
-                    drop_style_and_layout_data(dead_data)
-                },
                 Msg::ExitNow => {
                     debug!("layout thread is exiting...");
                     self.exit_now();
@@ -1472,7 +1464,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 +1477,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;