Auto merge of #13459 - servo:no-arc-heapsize, r=emilio

Use parking_lot::RwLock for PropertyDeclarationBlock

<!-- Please describe your changes on the following line: -->

As discussed in https://bugzilla.mozilla.org/show_bug.cgi?id=1305141

Closes #13176

---

Original PR title: Stop relying on `impl<T: HeapSizeOf> HeapSizeOf for Arc<T>`
servo/heapsize#37 (comment)

This builds on top of that.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because refactor

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13459)
<!-- Reviewable:end -->

components/layout/Cargo.toml

@@ -33,7 +33,7 @@ range = {path = "../range"}
 rustc-serialize = "0.3"
 script_layout_interface = {path = "../script_layout_interface"}
 script_traits = {path = "../script_traits"}
-selectors = {version = "0.13", features = ["heap_size"]}
+selectors = "0.13"
 serde_macros = "0.8"
 smallvec = "0.1"
 string_cache = {version = "0.2.26", features = ["heap_size"]}

components/plugins/lints/unrooted_must_root.rs

@@ -51,8 +51,6 @@ fn is_unrooted_ty(cx: &LateContext, ty: &ty::TyS, in_new_function: bool) -> bool
                     false
                 } else if match_def_path(cx, did.did, &["core", "cell", "Ref"])
                         || match_def_path(cx, did.did, &["core", "cell", "RefMut"])
-                        || match_def_path(cx, did.did, &["style", "refcell", "Ref"])
-                        || match_def_path(cx, did.did, &["style", "refcell", "RefMut"])
                         || match_def_path(cx, did.did, &["core", "slice", "Iter"])
                         || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "OccupiedEntry"])
                         || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "VacantEntry"]) {

components/script/Cargo.toml

@@ -51,6 +51,7 @@ net_traits = {path = "../net_traits"}
 num-traits = "0.1.32"
 offscreen_gl_context = "0.4"
 open = "1.1.1"
+parking_lot = "0.3"
 phf = "0.7.16"
 phf_macros = "0.7.16"
 plugins = {path = "../plugins"}
@@ -62,7 +63,7 @@ regex = "0.1.43"
 rustc-serialize = "0.3"
 script_layout_interface = {path = "../script_layout_interface"}
 script_traits = {path = "../script_traits"}
-selectors = {version = "0.13", features = ["heap_size"]}
+selectors = "0.13"
 serde = "0.8"
 smallvec = "0.1"
 string_cache = {version = "0.2.26", features = ["heap_size", "unstable"]}

components/script/body.rs

@@ -19,9 +19,9 @@ use js::jsapi::JS_ParseJSON;
 use js::jsapi::Value as JSValue;
 use js::jsval::UndefinedValue;
 use mime::{Mime, TopLevel, SubLevel};
+use std::cell::Ref;
 use std::rc::Rc;
 use std::str;
-use style::refcell::Ref;
 use url::form_urlencoded;
 
 pub enum BodyType {

components/script/dom/attr.rs

@@ -15,10 +15,10 @@ use dom::element::{AttributeMutation, Element};
 use dom::virtualmethods::vtable_for;
 use dom::window::Window;
 use std::borrow::ToOwned;
+use std::cell::Ref;
 use std::mem;
 use string_cache::{Atom, Namespace};
 use style::attr::{AttrIdentifier, AttrValue};
-use style::refcell::Ref;
 
 // https://dom.spec.whatwg.org/#interface-attr
 #[dom_struct]

components/style/domrefcell.rs → components/script/dom/bindings/cell.rs

@@ -4,15 +4,14 @@
 
 //! A shareable mutable container for the DOM.
 
-use refcell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut};
-use thread_state;
+use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut};
+use style::thread_state;
 
 /// A mutable field in the DOM.
 ///
 /// This extends the API of `core::cell::RefCell` to allow unsafe access in
 /// certain situations, with dynamic checking in debug builds.
-#[derive(Clone)]
-#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, PartialEq, Debug, HeapSizeOf)]
 pub struct DOMRefCell<T> {
     value: RefCell<T>,
 }

components/script/dom/bindings/mod.rs

@@ -128,9 +128,8 @@
 //! return `Err()` from the method with the appropriate [error value]
 //! (error/enum.Error.html).
 
-pub use style::domrefcell as cell;
-
 pub mod callback;
+pub mod cell;
 pub mod constant;
 pub mod conversions;
 pub mod error;

components/script/dom/bindings/trace.rs

@@ -35,6 +35,7 @@ use cssparser::RGBA;
 use devtools_traits::CSSError;
 use devtools_traits::WorkerId;
 use dom::abstractworker::SharedRt;
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::refcounted::{Trusted, TrustedPromise};
 use dom::bindings::reflector::{Reflectable, Reflector};
@@ -89,7 +90,6 @@ use std::sync::mpsc::{Receiver, Sender};
 use std::time::{SystemTime, Instant};
 use string_cache::{Atom, Namespace, QualName};
 use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto};
-use style::domrefcell::DOMRefCell;
 use style::element_state::*;
 use style::properties::PropertyDeclarationBlock;
 use style::selector_impl::{ElementSnapshot, PseudoElement};

components/script/dom/characterdata.rs

@@ -19,7 +19,7 @@ use dom::element::Element;
 use dom::node::{Node, NodeDamage};
 use dom::processinginstruction::ProcessingInstruction;
 use dom::text::Text;
-use style::refcell::Ref;
+use std::cell::Ref;
 use util::opts;
 
 // https://dom.spec.whatwg.org/#characterdata

components/script/dom/cssstyledeclaration.rs

@@ -13,14 +13,13 @@ use dom::bindings::str::DOMString;
 use dom::element::Element;
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
+use parking_lot::RwLock;
 use std::ascii::AsciiExt;
-use std::slice;
 use std::sync::Arc;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
-use style::properties::{PropertyDeclaration, Shorthand, Importance};
+use style::properties::{Shorthand, Importance};
 use style::properties::{is_supported_property, parse_one_declaration, parse_style_attribute};
-use style::refcell::Ref;
 use style::selector_impl::PseudoElement;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
@@ -93,7 +92,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     fn Length(&self) -> u32 {
         let elem = self.owner.upcast::<Element>();
         let len = match *elem.style_attribute().borrow() {
-            Some(ref declarations) => declarations.declarations.len(),
+            Some(ref declarations) => declarations.read().declarations.len(),
             None => 0,
         };
         len as u32
@@ -119,43 +118,42 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         // Step 2
         if let Some(shorthand) = Shorthand::from_name(&property) {
+            let style_attribute = owner.style_attribute().borrow();
+            let style_attribute = if let Some(ref style_attribute) = *style_attribute {
+                style_attribute.read()
+            } else {
+                // shorthand.longhands() is never empty, so with no style attribute
+                // step 2.2.2 would do this:
+                return DOMString::new()
+            };
+
             // Step 2.1
             let mut list = vec![];
 
             // Step 2.2
             for longhand in shorthand.longhands() {
                 // Step 2.2.1
-                let declaration = owner.get_inline_style_declaration(&Atom::from(*longhand));
+                let declaration = style_attribute.get(longhand);
 
                 // Step 2.2.2 & 2.2.3
                 match declaration {
-                    Some(declaration) => list.push(declaration),
+                    Some(&(ref declaration, _importance)) => list.push(declaration),
                     None => return DOMString::new(),
                 }
             }
 
             // Step 2.3
-            // Work around closures not being Clone
-            #[derive(Clone)]
-            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, (PropertyDeclaration, Importance)>>);
-            impl<'a, 'b> Iterator for Map<'a, 'b> {
-                type Item = &'a PropertyDeclaration;
-                fn next(&mut self) -> Option<Self::Item> {
-                    self.0.next().map(|r| &r.0)
-                }
-            }
-
             // TODO: important is hardcoded to false because method does not implement it yet
             let serialized_value = shorthand.serialize_shorthand_value_to_string(
-                Map(list.iter()), Importance::Normal);
+                list, Importance::Normal);
             return DOMString::from(serialized_value);
         }
 
         // Step 3 & 4
-        match owner.get_inline_style_declaration(&property) {
+        owner.get_inline_style_declaration(&property, |d| match d {
             Some(declaration) => DOMString::from(declaration.0.value()),
             None => DOMString::new(),
-        }
+        })
     }
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
@@ -172,13 +170,18 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                                     .all(|priority| priority == "important") {
                 return DOMString::from("important");
             }
-        // Step 3
         } else {
-            if let Some(decl) = self.owner.get_inline_style_declaration(&property) {
-                if decl.1.important() {
-                    return DOMString::from("important");
+            // Step 3
+            return self.owner.get_inline_style_declaration(&property, |d| {
+                if let Some(decl) = d {
+                    if decl.1.important() {
+                        return DOMString::from("important");
+                    }
                 }
-            }
+
+                // Step 4
+                DOMString::new()
+            })
         }
 
         // Step 4
@@ -328,13 +331,14 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let elem = self.owner.upcast::<Element>();
         let style_attribute = elem.style_attribute().borrow();
         style_attribute.as_ref().and_then(|declarations| {
-            declarations.declarations.get(index)
-        }).map(|&(ref declaration, importance)| {
-            let mut css = declaration.to_css_string();
-            if importance.important() {
-                css += " !important";
-            }
-            DOMString::from(css)
+            declarations.read().declarations.get(index).map(|entry| {
+                let (ref declaration, importance) = *entry;
+                let mut css = declaration.to_css_string();
+                if importance.important() {
+                    css += " !important";
+                }
+                DOMString::from(css)
+            })
         })
     }
 
@@ -344,7 +348,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let style_attribute = elem.style_attribute().borrow();
 
         if let Some(declarations) = style_attribute.as_ref() {
-            DOMString::from(declarations.to_css_string())
+            DOMString::from(declarations.read().to_css_string())
         } else {
             DOMString::new()
         }
@@ -366,7 +370,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         *element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() {
             None // Step 2
         } else {
-            Some(Arc::new(decl_block))
+            Some(Arc::new(RwLock::new(decl_block)))
         };
         element.sync_property_with_attrs_style();
         let node = element.upcast::<Node>();

components/script/dom/document.rs

@@ -113,7 +113,7 @@ use script_traits::UntrustedNodeAddress;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::boxed::FnBox;
-use std::cell::Cell;
+use std::cell::{Cell, Ref, RefMut};
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::default::Default;
@@ -125,7 +125,6 @@ use std::time::{Duration, Instant};
 use string_cache::{Atom, QualName};
 use style::attr::AttrValue;
 use style::context::ReflowGoal;
-use style::refcell::{Ref, RefMut};
 use style::selector_impl::ElementSnapshot;
 use style::str::{split_html_space_chars, str_join};
 use style::stylesheets::Stylesheet;
@@ -146,6 +145,14 @@ enum ParserBlockedByScript {
     Unblocked,
 }
 
+#[derive(JSTraceable, HeapSizeOf)]
+#[must_root]
+struct StylesheetInDocument {
+    node: JS<Node>,
+    #[ignore_heap_size_of = "Arc"]
+    stylesheet: Arc<Stylesheet>,
+}
+
 // https://dom.spec.whatwg.org/#document
 #[dom_struct]
 pub struct Document {
@@ -174,7 +181,7 @@ pub struct Document {
     anchors: MutNullableHeap<JS<HTMLCollection>>,
     applets: MutNullableHeap<JS<HTMLCollection>>,
     /// List of stylesheets associated with nodes in this document. |None| if the list needs to be refreshed.
-    stylesheets: DOMRefCell<Option<Vec<(JS<Node>, Arc<Stylesheet>)>>>,
+    stylesheets: DOMRefCell<Option<Vec<StylesheetInDocument>>>,
     /// Whether the list of stylesheets has changed since the last reflow was triggered.
     stylesheets_changed_since_reflow: Cell<bool>,
     ready_state: Cell<DocumentReadyState>,
@@ -1879,13 +1886,16 @@ impl Document {
                             node.get_stylesheet()
                         } else {
                             None
-                        }.map(|stylesheet| (JS::from_ref(&*node), stylesheet))
+                        }.map(|stylesheet| StylesheetInDocument {
+                            node: JS::from_ref(&*node),
+                            stylesheet: stylesheet
+                        })
                     })
                     .collect());
             };
         }
         self.stylesheets.borrow().as_ref().unwrap().iter()
-                        .map(|&(_, ref stylesheet)| stylesheet.clone())
+                        .map(|s| s.stylesheet.clone())
                         .collect()
     }
 

components/script/dom/dommatrixreadonly.rs

@@ -14,9 +14,8 @@ use dom::bindings::reflector::{reflect_dom_object, Reflectable, Reflector};
 use dom::dommatrix::DOMMatrix;
 use dom::dompoint::DOMPoint;
 use euclid::{Matrix4D, Point4D, Radians};
-use std::cell::Cell;
+use std::cell::{Cell, Ref};
 use std::f64;
-use style::refcell::Ref;
 
 #[dom_struct]
 pub struct DOMMatrixReadOnly {