Auto merge of #13134 - servo:archery, r=emilio

Add lots of Arc’s in style, and prepare for using DOMRefCell

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

`DOMRefCell` usage is not there year because of thread-safety questions, but I have this much already that I’d like to land before it bitrots.

r? @emilio

---
<!-- 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 new 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/13134)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-08-31 16:30:56 -05:00 committed by GitHub
commit bbfe38e35f
29 changed files with 487 additions and 246 deletions

View file

@ -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]

View file

@ -1,139 +0,0 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
//! A shareable mutable container for the DOM.
use dom::bindings::trace::JSTraceable;
use js::jsapi::JSTracer;
use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut};
use style::thread_state;
use style::thread_state::SCRIPT;
/// 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, HeapSizeOf)]
pub struct DOMRefCell<T> {
value: RefCell<T>,
}
// Functionality specific to Servo's `DOMRefCell` type
// ===================================================
impl<T> DOMRefCell<T> {
/// Return a reference to the contents.
///
/// For use in the layout thread only.
#[allow(unsafe_code)]
pub unsafe fn borrow_for_layout(&self) -> &T {
debug_assert!(thread_state::get().is_layout());
&*self.value.as_ptr()
}
/// Borrow the contents for the purpose of GC tracing.
///
/// This succeeds even if the object is mutably borrowed,
/// so you have to be careful in trace code!
#[allow(unsafe_code)]
pub unsafe fn borrow_for_gc_trace(&self) -> &T {
// FIXME: IN_GC isn't reliable enough - doesn't catch minor GCs
// https://github.com/servo/servo/issues/6389
// debug_assert!(thread_state::get().contains(SCRIPT | IN_GC));
&*self.value.as_ptr()
}
/// Borrow the contents for the purpose of script deallocation.
///
#[allow(unsafe_code)]
pub unsafe fn borrow_for_script_deallocation(&self) -> &mut T {
debug_assert!(thread_state::get().contains(SCRIPT));
&mut *self.value.as_ptr()
}
/// Version of the above that we use during restyle while the script thread
/// is blocked.
pub fn borrow_mut_for_layout(&self) -> RefMut<T> {
debug_assert!(thread_state::get().is_layout());
self.value.borrow_mut()
}
}
impl<T: JSTraceable> JSTraceable for DOMRefCell<T> {
fn trace(&self, trc: *mut JSTracer) {
unsafe {
(*self).borrow_for_gc_trace().trace(trc)
}
}
}
// Functionality duplicated with `core::cell::RefCell`
// ===================================================
impl<T> DOMRefCell<T> {
/// Create a new `DOMRefCell` containing `value`.
pub fn new(value: T) -> DOMRefCell<T> {
DOMRefCell {
value: RefCell::new(value),
}
}
/// Immutably borrows the wrapped value.
///
/// The borrow lasts until the returned `Ref` exits scope. Multiple
/// immutable borrows can be taken out at the same time.
///
/// # Panics
///
/// Panics if this is called off the script thread.
///
/// Panics if the value is currently mutably borrowed.
pub fn borrow(&self) -> Ref<T> {
self.try_borrow().expect("DOMRefCell<T> already mutably borrowed")
}
/// Mutably borrows the wrapped value.
///
/// The borrow lasts until the returned `RefMut` exits scope. The value
/// cannot be borrowed while this borrow is active.
///
/// # Panics
///
/// Panics if this is called off the script thread.
///
/// Panics if the value is currently borrowed.
pub fn borrow_mut(&self) -> RefMut<T> {
self.try_borrow_mut().expect("DOMRefCell<T> already borrowed")
}
/// Attempts to immutably borrow the wrapped value.
///
/// The borrow lasts until the returned `Ref` exits scope. Multiple
/// immutable borrows can be taken out at the same time.
///
/// Returns `None` if the value is currently mutably borrowed.
///
/// # Panics
///
/// Panics if this is called off the script thread.
pub fn try_borrow(&self) -> Result<Ref<T>, BorrowError<T>> {
debug_assert!(thread_state::get().is_script());
self.value.try_borrow()
}
/// Mutably borrows the wrapped value.
///
/// The borrow lasts until the returned `RefMut` exits scope. The value
/// cannot be borrowed while this borrow is active.
///
/// Returns `None` if the value is currently borrowed.
///
/// # Panics
///
/// Panics if this is called off the script thread.
pub fn try_borrow_mut(&self) -> Result<RefMut<T>, BorrowMutError<T>> {
debug_assert!(thread_state::get().is_script());
self.value.try_borrow_mut()
}
}

View file

@ -128,8 +128,9 @@
//! 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 conversions;
pub mod error;
pub mod global;

View file

@ -88,6 +88,7 @@ use std::sync::mpsc::{Receiver, Sender};
use std::time::SystemTime;
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::{PseudoElement, ElementSnapshot};
@ -172,6 +173,13 @@ impl<T: JSTraceable> JSTraceable for UnsafeCell<T> {
}
}
impl<T: JSTraceable> JSTraceable for DOMRefCell<T> {
fn trace(&self, trc: *mut JSTracer) {
unsafe {
(*self).borrow_for_gc_trace().trace(trc)
}
}
}
impl JSTraceable for Heap<*mut JSObject> {
fn trace(&self, trc: *mut JSTracer) {

View file

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

View file

@ -14,12 +14,13 @@ use dom::element::Element;
use dom::node::{Node, NodeDamage, window_from_node};
use dom::window::Window;
use std::ascii::AsciiExt;
use std::cell::Ref;
use std::slice;
use std::sync::Arc;
use string_cache::Atom;
use style::parser::ParserContextExtraData;
use style::properties::{PropertyDeclaration, 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
@ -365,7 +366,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
*element.style_attribute().borrow_mut() = if decl_block.declarations.is_empty() {
None // Step 2
} else {
Some(decl_block)
Some(Arc::new(decl_block))
};
element.sync_property_with_attrs_style();
let node = element.upcast::<Node>();

View file

@ -111,7 +111,7 @@ use script_traits::{TouchEventType, TouchId};
use std::ascii::AsciiExt;
use std::borrow::ToOwned;
use std::boxed::FnBox;
use std::cell::{Cell, Ref, RefMut};
use std::cell::Cell;
use std::collections::HashMap;
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::default::Default;
@ -122,6 +122,7 @@ use std::sync::Arc;
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;

View file

@ -70,13 +70,12 @@ use html5ever::serialize::SerializeOpts;
use html5ever::serialize::TraversalScope;
use html5ever::serialize::TraversalScope::{ChildrenOnly, IncludeNode};
use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks};
use ref_filter_map::ref_filter_map;
use selectors::matching::{ElementFlags, matches};
use selectors::matching::{HAS_SLOW_SELECTOR, HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
use selectors::parser::{AttrSelector, NamespaceConstraint, parse_author_origin_selector_list_from_str};
use std::ascii::AsciiExt;
use std::borrow::Cow;
use std::cell::{Cell, Ref};
use std::cell::Cell;
use std::convert::TryFrom;
use std::default::Default;
use std::fmt;
@ -90,6 +89,7 @@ use style::parser::ParserContextExtraData;
use style::properties::longhands::{self, background_image, border_spacing, font_family, overflow_x, font_size};
use style::properties::{DeclaredValue, Importance};
use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, parse_style_attribute};
use style::refcell::Ref;
use style::selector_impl::{NonTSPseudoClass, ServoSelectorImpl};
use style::selector_matching::DeclarationBlock;
use style::sink::Push;
@ -109,7 +109,7 @@ pub struct Element {
prefix: Option<DOMString>,
attrs: DOMRefCell<Vec<JS<Attr>>>,
id_attribute: DOMRefCell<Option<Atom>>,
style_attribute: DOMRefCell<Option<PropertyDeclarationBlock>>,
style_attribute: DOMRefCell<Option<Arc<PropertyDeclarationBlock>>>,
attr_list: MutNullableHeap<JS<NamedNodeMap>>,
class_list: MutNullableHeap<JS<DOMTokenList>>,
state: Cell<ElementState>,
@ -297,7 +297,7 @@ pub trait LayoutElementHelpers {
#[allow(unsafe_code)]
unsafe fn html_element_in_html_document_for_layout(&self) -> bool;
fn id_attribute(&self) -> *const Option<Atom>;
fn style_attribute(&self) -> *const Option<PropertyDeclarationBlock>;
fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>>;
fn local_name(&self) -> &Atom;
fn namespace(&self) -> &Namespace;
fn get_checked_state_for_layout(&self) -> bool;
@ -329,7 +329,10 @@ impl LayoutElementHelpers for LayoutJS<Element> {
#[inline]
fn from_declaration(rule: PropertyDeclaration) -> DeclarationBlock {
DeclarationBlock::from_declarations(
Arc::new(vec![(rule, Importance::Normal)]),
Arc::new(PropertyDeclarationBlock {
declarations: vec![(rule, Importance::Normal)],
important_count: 0,
}),
Importance::Normal)
}
@ -615,7 +618,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
}
#[allow(unsafe_code)]
fn style_attribute(&self) -> *const Option<PropertyDeclarationBlock> {
fn style_attribute(&self) -> *const Option<Arc<PropertyDeclarationBlock>> {
unsafe {
(*self.unsafe_get()).style_attribute.borrow_for_layout()
}
@ -704,7 +707,7 @@ impl Element {
self.attrs.borrow()
}
pub fn style_attribute(&self) -> &DOMRefCell<Option<PropertyDeclarationBlock>> {
pub fn style_attribute(&self) -> &DOMRefCell<Option<Arc<PropertyDeclarationBlock>>> {
&self.style_attribute
}
@ -774,7 +777,8 @@ impl Element {
matching
});
if let Some(index) = index {
Arc::make_mut(&mut declarations.declarations).remove(index);
let declarations = Arc::make_mut(declarations);
declarations.declarations.remove(index);
if importance.unwrap().important() {
declarations.important_count -= 1;
}
@ -796,7 +800,8 @@ impl Element {
{
// Usually, the reference count will be 1 here. But transitions could make it greater
// than that.
let existing_declarations = Arc::make_mut(&mut declaration_block.declarations);
let declaration_block = Arc::make_mut(declaration_block);
let existing_declarations = &mut declaration_block.declarations;
'outer: for incoming_declaration in declarations {
for existing_declaration in &mut *existing_declarations {
@ -829,10 +834,10 @@ impl Element {
0
};
*inline_declarations = Some(PropertyDeclarationBlock {
declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()),
*inline_declarations = Some(Arc::new(PropertyDeclarationBlock {
declarations: declarations.into_iter().map(|d| (d, importance)).collect(),
important_count: important_count,
});
}));
}
update(self, declarations, importance);
@ -847,7 +852,8 @@ impl Element {
if let &mut Some(ref mut block) = &mut *inline_declarations {
// Usually, the reference counts of `from` and `to` will be 1 here. But transitions
// could make them greater than that.
let declarations = Arc::make_mut(&mut block.declarations);
let block = Arc::make_mut(block);
let declarations = &mut block.declarations;
for &mut (ref declaration, ref mut importance) in declarations {
if properties.iter().any(|p| declaration.name() == **p) {
match (*importance, new_importance) {
@ -871,7 +877,7 @@ impl Element {
pub fn get_inline_style_declaration(&self,
property: &Atom)
-> Option<Ref<(PropertyDeclaration, Importance)>> {
ref_filter_map(self.style_attribute.borrow(), |inline_declarations| {
Ref::filter_map(self.style_attribute.borrow(), |inline_declarations| {
inline_declarations.as_ref().and_then(|declarations| {
declarations.declarations
.iter()
@ -2102,8 +2108,11 @@ impl VirtualMethods for Element {
*self.style_attribute.borrow_mut() =
mutation.new_value(attr).map(|value| {
let win = window_from_node(self);
parse_style_attribute(&value, &doc.base_url(), win.css_error_reporter(),
ParserContextExtraData::default())
Arc::new(parse_style_attribute(
&value,
&doc.base_url(),
win.css_error_reporter(),
ParserContextExtraData::default()))
});
if node.is_in_doc() {
node.dirty(NodeDamage::NodeStyleDamaged);

View file

@ -79,7 +79,7 @@ impl HTMLMetaElement {
if !content.is_empty() {
if let Some(translated_rule) = ViewportRule::from_meta(&**content) {
*self.stylesheet.borrow_mut() = Some(Arc::new(Stylesheet {
rules: vec![CSSRule::Viewport(translated_rule)],
rules: vec![CSSRule::Viewport(Arc::new(translated_rule))],
origin: Origin::Author,
media: None,
// Viewport constraints are always recomputed on resize; they don't need to