Remove the extra level of GeckoDeclarationBlock.

components/style/gecko/conversions.rs

@@ -9,10 +9,11 @@
 #![allow(unsafe_code)]
 
 use app_units::Au;
-use gecko_bindings::bindings::{RawServoStyleSheet, ServoComputedValues};
+use gecko_bindings::bindings::{RawServoStyleSheet, ServoComputedValues, RawServoDeclarationBlock};
 use gecko_bindings::structs::nsStyleCoord_CalcValue;
 use gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI};
-use properties::ComputedValues;
+use parking_lot::RwLock;
+use properties::{ComputedValues, PropertyDeclarationBlock};
 use stylesheets::Stylesheet;
 use values::computed::{CalcLengthOrPercentage, LengthOrPercentage, LengthOrPercentageOrAuto};
 
@@ -25,6 +26,11 @@ unsafe impl HasFFI for ComputedValues {
 }
 unsafe impl HasArcFFI for ComputedValues {}
 
+unsafe impl HasFFI for RwLock<PropertyDeclarationBlock> {
+    type FFIType = RawServoDeclarationBlock;
+}
+unsafe impl HasArcFFI for RwLock<PropertyDeclarationBlock> {}
+
 impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {
         let has_percentage = other.percentage.is_some();

components/style/gecko/wrapper.rs

@@ -30,7 +30,7 @@ use gecko_bindings::structs::{NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO, NODE_IS_DIRT
 use gecko_bindings::structs::{RawGeckoDocument, RawGeckoElement, RawGeckoNode};
 use gecko_bindings::structs::{nsChangeHint, nsIAtom, nsIContent, nsStyleContext};
 use gecko_bindings::structs::OpaqueStyleData;
-use gecko_bindings::sugar::ownership::{FFIArcHelpers, HasArcFFI, HasFFI};
+use gecko_bindings::sugar::ownership::FFIArcHelpers;
 use libc::uintptr_t;
 use parking_lot::RwLock;
 use parser::ParserContextExtraData;
@@ -45,7 +45,6 @@ use std::fmt;
 use std::ops::BitOr;
 use std::ptr;
 use std::sync::Arc;
-use std::sync::atomic::{AtomicBool, AtomicPtr};
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use url::Url;
 
@@ -57,35 +56,6 @@ impl NonOpaqueStyleData {
     }
 }
 
-
-pub struct GeckoDeclarationBlock {
-    pub declarations: Option<Arc<RwLock<PropertyDeclarationBlock>>>,
-    // XXX The following two fields are made atomic to work around the
-    // ownership system so that they can be changed inside a shared
-    // instance. It wouldn't provide safety as Rust usually promises,
-    // but it is fine as far as we only access them in a single thread.
-    // If we need to access them in different threads, we would need
-    // to redesign how it works with MiscContainer in Gecko side.
-    pub cache: AtomicPtr<bindings::nsHTMLCSSStyleSheet>,
-    pub immutable: AtomicBool,
-}
-
-impl PartialEq for GeckoDeclarationBlock {
-    fn eq(&self, other: &GeckoDeclarationBlock) -> bool {
-        match (&self.declarations, &other.declarations) {
-            (&None, &None) => true,
-            (&Some(ref s), &Some(ref other)) => *s.read() == *other.read(),
-            _ => false,
-        }
-    }
-}
-
-unsafe impl HasFFI for GeckoDeclarationBlock {
-    type FFIType = bindings::RawServoDeclarationBlock;
-}
-unsafe impl HasArcFFI for GeckoDeclarationBlock {}
-
-
 // We can eliminate OpaqueStyleData when the bindings move into the style crate.
 fn to_opaque_style_data(d: *mut NonOpaqueStyleData) -> *mut OpaqueStyleData {
     d as *mut OpaqueStyleData
@@ -486,13 +456,13 @@ impl<'le> fmt::Debug for GeckoElement<'le> {
 }
 
 impl<'le> GeckoElement<'le> {
-    pub fn parse_style_attribute(value: &str) -> Option<PropertyDeclarationBlock> {
+    pub fn parse_style_attribute(value: &str) -> PropertyDeclarationBlock {
         // FIXME(bholley): Real base URL and error reporter.
         let base_url = &*DUMMY_BASE_URL;
         // FIXME(heycam): Needs real ParserContextExtraData so that URLs parse
         // properly.
         let extra_data = ParserContextExtraData::default();
-        Some(parse_style_attribute(value, &base_url, Box::new(StdoutErrorReporter), extra_data))
+        parse_style_attribute(value, &base_url, Box::new(StdoutErrorReporter), extra_data)
     }
 }
 
@@ -512,12 +482,7 @@ impl<'le> TElement for GeckoElement<'le> {
 
     fn style_attribute(&self) -> Option<&Arc<RwLock<PropertyDeclarationBlock>>> {
         let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.0) };
-        if declarations.is_none() {
-            None
-        } else {
-            let declarations = GeckoDeclarationBlock::arc_from_borrowed(&declarations).unwrap();
-            declarations.declarations.as_ref().map(|r| r as *const Arc<_>).map(|ptr| unsafe { &*ptr })
-        }
+        declarations.map(|s| s.as_arc_opt()).unwrap_or(None)
     }
 
     fn get_state(&self) -> ElementState {

components/style/gecko_bindings/bindings.rs

@@ -178,14 +178,6 @@ extern "C" {
     pub fn Gecko_ClearPODTArray(aArray: *mut ::std::os::raw::c_void,
                                 aElementSize: usize, aElementAlign: usize);
 }
-#[repr(C)]
-#[derive(Debug, Copy)]
-pub struct nsHTMLCSSStyleSheet {
-    pub _address: u8,
-}
-impl Clone for nsHTMLCSSStyleSheet {
-    fn clone(&self) -> Self { *self }
-}
 extern "C" {
     pub fn Gecko_ChildrenCount(node: RawGeckoNodeBorrowed) -> u32;
 }
@@ -372,7 +364,7 @@ extern "C" {
 }
 extern "C" {
     pub fn Gecko_GetServoDeclarationBlock(element: RawGeckoElementBorrowed)
-     -> RawServoDeclarationBlockBorrowedOrNull;
+     -> RawServoDeclarationBlockStrongBorrowedOrNull;
 }
 extern "C" {
     pub fn Gecko_Atomize(aString: *const ::std::os::raw::c_char, aLength: u32)
@@ -877,8 +869,7 @@ extern "C" {
      -> ServoComputedValuesStrong;
 }
 extern "C" {
-    pub fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32,
-                                     cache: *mut nsHTMLCSSStyleSheet)
+    pub fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32)
      -> RawServoDeclarationBlockStrong;
 }
 extern "C" {
@@ -894,19 +885,6 @@ extern "C" {
                                          b: RawServoDeclarationBlockBorrowed)
      -> bool;
 }
-extern "C" {
-    pub fn Servo_DeclarationBlock_GetCache(declarations:
-                                               RawServoDeclarationBlockBorrowed)
-     -> *mut nsHTMLCSSStyleSheet;
-}
-extern "C" {
-    pub fn Servo_DeclarationBlock_SetImmutable(declarations:
-                                                   RawServoDeclarationBlockBorrowed);
-}
-extern "C" {
-    pub fn Servo_DeclarationBlock_ClearCachePointer(declarations:
-                                                        RawServoDeclarationBlockBorrowed);
-}
 extern "C" {
     pub fn Servo_DeclarationBlock_SerializeOneValue(declarations:
                                                         RawServoDeclarationBlockBorrowed,

ports/geckolib/glue.rs

@@ -8,11 +8,9 @@ use env_logger;
 use euclid::Size2D;
 use parking_lot::RwLock;
 use std::mem::transmute;
-use std::ptr;
 use std::slice;
 use std::str::from_utf8_unchecked;
 use std::sync::{Arc, Mutex};
-use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
 use style::arc_ptr_eq;
 use style::context::{LocalStyleContextCreationInfo, ReflowGoal, SharedStyleContext};
 use style::dom::{NodeInfo, TElement, TNode};
@@ -21,16 +19,16 @@ use style::gecko::data::{NUM_THREADS, PerDocumentStyleData};
 use style::gecko::selector_impl::{GeckoSelectorImpl, PseudoElement};
 use style::gecko::snapshot::GeckoElementSnapshot;
 use style::gecko::traversal::RecalcStyleOnly;
-use style::gecko::wrapper::{DUMMY_BASE_URL, GeckoDeclarationBlock};
 use style::gecko::wrapper::{GeckoElement, GeckoNode};
+use style::gecko::wrapper::DUMMY_BASE_URL;
 use style::gecko_bindings::bindings::{RawGeckoElementBorrowed, RawGeckoNodeBorrowed};
 use style::gecko_bindings::bindings::{RawServoDeclarationBlockBorrowed, RawServoDeclarationBlockStrong};
 use style::gecko_bindings::bindings::{RawServoStyleSetBorrowed, RawServoStyleSetOwned};
 use style::gecko_bindings::bindings::{RawServoStyleSheetBorrowed, ServoComputedValuesBorrowed};
 use style::gecko_bindings::bindings::{RawServoStyleSheetStrong, ServoComputedValuesStrong};
 use style::gecko_bindings::bindings::{ThreadSafePrincipalHolder, ThreadSafeURIHolder};
-use style::gecko_bindings::bindings::{nsHTMLCSSStyleSheet, ServoComputedValuesBorrowedOrNull};
 use style::gecko_bindings::bindings::Gecko_Utf8SliceToString;
+use style::gecko_bindings::bindings::ServoComputedValuesBorrowedOrNull;
 use style::gecko_bindings::structs::{SheetParsingMode, nsIAtom};
 use style::gecko_bindings::structs::ServoElementSnapshot;
 use style::gecko_bindings::structs::nsRestyleHint;
@@ -131,28 +129,24 @@ pub extern "C" fn Servo_RestyleWithAddedDeclaration(declarations: RawServoDeclar
                                                     previous_style: ServoComputedValuesBorrowed)
   -> ServoComputedValuesStrong
 {
-    match GeckoDeclarationBlock::as_arc(&declarations).declarations {
-        Some(ref declarations) => {
-            let declaration_block = ApplicableDeclarationBlock {
-                mixed_declarations: declarations.clone(),
-                importance: Importance::Normal,
-                source_order: 0,
-                specificity: ::std::u32::MAX,
-            };
-            let previous_style = ComputedValues::as_arc(&previous_style);
+    let declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations);
+    let declaration_block = ApplicableDeclarationBlock {
+        mixed_declarations: declarations.clone(),
+        importance: Importance::Normal,
+        source_order: 0,
+        specificity: ::std::u32::MAX,
+    };
+    let previous_style = ComputedValues::as_arc(&previous_style);
 
-            // FIXME (bug 1303229): Use the actual viewport size here
-            let (computed, _) = cascade(Size2D::new(Au(0), Au(0)),
-                                        &[declaration_block],
-                                        false,
-                                        Some(previous_style),
-                                        None,
-                                        None,
-                                        Box::new(StdoutErrorReporter));
-            Arc::new(computed).into_strong()
-        },
-        None => ServoComputedValuesStrong::null(),
-    }
+    // FIXME (bug 1303229): Use the actual viewport size here
+    let (computed, _) = cascade(Size2D::new(Au(0), Au(0)),
+                                &[declaration_block],
+                                false,
+                                Some(previous_style),
+                                None,
+                                None,
+                                Box::new(StdoutErrorReporter));
+    Arc::new(computed).into_strong()
 }
 
 #[no_mangle]
@@ -409,60 +403,34 @@ pub extern "C" fn Servo_ParseProperty(property_bytes: *const u8,
 
     let results = results.into_iter().map(|r| (r, Importance::Normal)).collect();
 
-    Arc::new(GeckoDeclarationBlock {
-        declarations: Some(Arc::new(RwLock::new(PropertyDeclarationBlock {
-            declarations: results,
-            important_count: 0,
-        }))),
-        cache: AtomicPtr::new(ptr::null_mut()),
-        immutable: AtomicBool::new(false),
-    }).into_strong()
+    Arc::new(RwLock::new(PropertyDeclarationBlock {
+        declarations: results,
+        important_count: 0,
+    })).into_strong()
 }
+
 #[no_mangle]
-pub extern "C" fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32,
-                                            cache: *mut nsHTMLCSSStyleSheet)
+pub extern "C" fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32)
                                             -> RawServoDeclarationBlockStrong {
     let value = unsafe { from_utf8_unchecked(slice::from_raw_parts(bytes, length as usize)) };
-    Arc::new(GeckoDeclarationBlock {
-        declarations: GeckoElement::parse_style_attribute(value).map(|block| {
-            Arc::new(RwLock::new(block))
-        }),
-        cache: AtomicPtr::new(cache),
-        immutable: AtomicBool::new(false),
-    }).into_strong()
+    Arc::new(RwLock::new(GeckoElement::parse_style_attribute(value))).into_strong()
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_AddRef(declarations: RawServoDeclarationBlockBorrowed) {
-    unsafe { GeckoDeclarationBlock::addref(declarations) };
+    unsafe { RwLock::<PropertyDeclarationBlock>::addref(declarations) };
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_Release(declarations: RawServoDeclarationBlockBorrowed) {
-    unsafe { GeckoDeclarationBlock::release(declarations) };
+    unsafe { RwLock::<PropertyDeclarationBlock>::release(declarations) };
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_Equals(a: RawServoDeclarationBlockBorrowed,
                                                 b: RawServoDeclarationBlockBorrowed)
                                                 -> bool {
-    GeckoDeclarationBlock::as_arc(&a) == GeckoDeclarationBlock::as_arc(&b)
-}
-
-#[no_mangle]
-pub extern "C" fn Servo_DeclarationBlock_GetCache(declarations: RawServoDeclarationBlockBorrowed)
-                                                 -> *mut nsHTMLCSSStyleSheet {
-    GeckoDeclarationBlock::as_arc(&declarations).cache.load(Ordering::Relaxed)
-}
-
-#[no_mangle]
-pub extern "C" fn Servo_DeclarationBlock_SetImmutable(declarations: RawServoDeclarationBlockBorrowed) {
-    GeckoDeclarationBlock::as_arc(&declarations).immutable.store(true, Ordering::Relaxed)
-}
-
-#[no_mangle]
-pub extern "C" fn Servo_DeclarationBlock_ClearCachePointer(declarations: RawServoDeclarationBlockBorrowed) {
-    GeckoDeclarationBlock::as_arc(&declarations).cache.store(ptr::null_mut(), Ordering::Relaxed)
+    *RwLock::<PropertyDeclarationBlock>::as_arc(&a).read() == *RwLock::<PropertyDeclarationBlock>::as_arc(&b).read()
 }
 
 #[no_mangle]
@@ -472,29 +440,28 @@ pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue(
 {
     let mut string = String::new();
 
-    if let Some(ref declarations) = GeckoDeclarationBlock::as_arc(&declarations).declarations {
-        declarations.read().to_css(&mut string).unwrap();
-        // FIXME: We are expecting |declarations| to be a declaration block with either a single
-        // longhand property-declaration or a series of longhand property-declarations that make
-        // up a single shorthand property. As a result, it should be possible to serialize
-        // |declarations| as a single declaration. However, we only want to return the *value* from
-        // that single declaration. For now, we just manually strip the property name, colon,
-        // leading spacing, and trailing space. In future we should find a more robust way to do
-        // this.
-        //
-        // See https://github.com/servo/servo/issues/13423
-        debug_assert!(string.find(':').is_some());
-        let position = string.find(':').unwrap();
-        // Get the value after the first colon and any following whitespace.
-        let value = &string[(position + 1)..].trim_left();
-        debug_assert!(value.ends_with(';'));
-        let length = value.len() - 1; // Strip last semicolon.
+    let declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations);
+    declarations.read().to_css(&mut string).unwrap();
+    // FIXME: We are expecting |declarations| to be a declaration block with either a single
+    // longhand property-declaration or a series of longhand property-declarations that make
+    // up a single shorthand property. As a result, it should be possible to serialize
+    // |declarations| as a single declaration. However, we only want to return the *value* from
+    // that single declaration. For now, we just manually strip the property name, colon,
+    // leading spacing, and trailing space. In future we should find a more robust way to do
+    // this.
+    //
+    // See https://github.com/servo/servo/issues/13423
+    debug_assert!(string.find(':').is_some());
+    let position = string.find(':').unwrap();
+    // Get the value after the first colon and any following whitespace.
+    let value = &string[(position + 1)..].trim_left();
+    debug_assert!(value.ends_with(';'));
+    let length = value.len() - 1; // Strip last semicolon.
 
-        // FIXME: Once we have nsString bindings for Servo (bug 1294742), we should be able to drop
-        // this and fill in |buffer| directly.
-        unsafe {
-            Gecko_Utf8SliceToString(buffer, value.as_ptr(), length);
-        }
+    // FIXME: Once we have nsString bindings for Servo (bug 1294742), we should be able to drop
+    // this and fill in |buffer| directly.
+    unsafe {
+        Gecko_Utf8SliceToString(buffer, value.as_ptr(), length);
     }
 }