Bug 1466963: Provide a before-mutation closure to C++. r?xidorn,smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 06 Jun 2018 00:04:39 +0200
changeset 804472 847d34fcc650760cf04882656f473ad13a024056
parent 804471 749d1d7017634b0ebb9ccf49cb9fc0727b79b684
push id112373
push userbmo:emilio@crisal.io
push dateTue, 05 Jun 2018 22:12:13 +0000
reviewersxidorn, smaug
bugs1466963
milestone62.0a1
Bug 1466963: Provide a before-mutation closure to C++. r?xidorn,smaug MozReview-Commit-ID: H2jwIeZoiBZ
layout/style/DeclarationBlock.h
layout/style/ServoBindingList.h
layout/style/ServoBindingTypes.h
layout/style/ServoBindings.toml
layout/style/nsDOMCSSDeclaration.cpp
layout/style/test/gtest/StyloParsingBench.cpp
servo/ports/geckolib/glue.rs
--- a/layout/style/DeclarationBlock.h
+++ b/layout/style/DeclarationBlock.h
@@ -205,28 +205,30 @@ public:
 
   bool GetPropertyIsImportant(const nsAString& aProperty) const
   {
     NS_ConvertUTF16toUTF8 property(aProperty);
     return Servo_DeclarationBlock_GetPropertyIsImportant(mRaw, &property);
   }
 
   // Returns whether the property was removed.
-  bool RemoveProperty(const nsAString& aProperty)
+  bool RemoveProperty(const nsAString& aProperty,
+                      DeclarationBlockMutationClosure aClosure = { })
   {
     AssertMutable();
     NS_ConvertUTF16toUTF8 property(aProperty);
-    return Servo_DeclarationBlock_RemoveProperty(mRaw, &property);
+    return Servo_DeclarationBlock_RemoveProperty(mRaw, &property, aClosure);
   }
 
   // Returns whether the property was removed.
-  bool RemovePropertyByID(nsCSSPropertyID aProperty)
+  bool RemovePropertyByID(nsCSSPropertyID aProperty,
+                          DeclarationBlockMutationClosure aClosure = { })
   {
     AssertMutable();
-    return Servo_DeclarationBlock_RemovePropertyById(mRaw, aProperty);
+    return Servo_DeclarationBlock_RemovePropertyById(mRaw, aProperty, aClosure);
   }
 
 private:
   ~DeclarationBlock() = default;
   union {
     // We only ever have one of these since we have an
     // nsHTMLCSSStyleSheet only for style attributes, and style
     // attributes never have an owning rule.
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -505,16 +505,22 @@ SERVO_BINDING_FUNC(Servo_AnimationValue_
                    RawServoAnimationValueBorrowed value)
 SERVO_BINDING_FUNC(Servo_AnimationValue_Compute,
                    RawServoAnimationValueStrong,
                    RawGeckoElementBorrowed element,
                    RawServoDeclarationBlockBorrowed declarations,
                    ComputedStyleBorrowed style,
                    RawServoStyleSetBorrowed raw_data)
 
+// There's no reason we couldn't expose more stuff here, but GetCssText is
+// pretty much all we'd ever want.
+SERVO_BINDING_FUNC(Servo_UnlockedDeclarationBlock_GetCssText, void,
+                   const RawServoUnlockedDeclarationBlock* declarations,
+                   nsAString* result)
+
 // Style attribute
 SERVO_BINDING_FUNC(Servo_ParseStyleAttribute, RawServoDeclarationBlockStrong,
                    const nsACString* data,
                    RawGeckoURLExtraData* extra_data,
                    nsCompatibility quirks_mode,
                    mozilla::css::Loader* loader)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_CreateEmpty,
                    RawServoDeclarationBlockStrong)
@@ -547,34 +553,38 @@ SERVO_BINDING_FUNC(Servo_DeclarationBloc
                    const nsACString* property)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetProperty, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    const nsACString* property,
                    const nsACString* value, bool is_important,
                    RawGeckoURLExtraData* data,
                    mozilla::ParsingMode parsing_mode,
                    nsCompatibility quirks_mode,
-                   mozilla::css::Loader* loader)
+                   mozilla::css::Loader* loader,
+                   DeclarationBlockMutationClosure)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetPropertyToAnimationValue, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    RawServoAnimationValueBorrowed animation_value)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_SetPropertyById, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    nsCSSPropertyID property,
                    const nsACString* value, bool is_important,
                    RawGeckoURLExtraData* data,
                    mozilla::ParsingMode parsing_mode,
                    nsCompatibility quirks_mode,
-                   mozilla::css::Loader* loader)
+                   mozilla::css::Loader* loader,
+                   DeclarationBlockMutationClosure)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_RemoveProperty, bool,
                    RawServoDeclarationBlockBorrowed declarations,
-                   const nsACString* property)
+                   const nsACString* property,
+                   DeclarationBlockMutationClosure)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_RemovePropertyById, bool,
                    RawServoDeclarationBlockBorrowed declarations,
-                   nsCSSPropertyID property)
+                   nsCSSPropertyID property,
+                   DeclarationBlockMutationClosure)
 SERVO_BINDING_FUNC(Servo_DeclarationBlock_HasCSSWideKeyword, bool,
                    RawServoDeclarationBlockBorrowed declarations,
                    nsCSSPropertyID property)
 // Compose animation value for a given property.
 // |base_values| is nsRefPtrHashtable<nsUint32HashKey, RawServoAnimationValue>.
 // We use RawServoAnimationValueTableBorrowed to avoid exposing
 // nsRefPtrHashtable in FFI.
 SERVO_BINDING_FUNC(Servo_AnimationCompose, void,
--- a/layout/style/ServoBindingTypes.h
+++ b/layout/style/ServoBindingTypes.h
@@ -78,16 +78,32 @@ typedef nsStyleAutoArray<mozilla::StyleA
 typedef nsTArray<nsFontFaceRuleContainer> RawGeckoFontFaceRuleList;
 typedef mozilla::AnimationPropertySegment RawGeckoAnimationPropertySegment;
 typedef mozilla::ComputedTiming RawGeckoComputedTiming;
 typedef nsTArray<const RawServoStyleRule*> RawGeckoServoStyleRuleList;
 typedef nsTArray<nsCSSPropertyID> RawGeckoCSSPropertyIDList;
 typedef mozilla::gfx::Float RawGeckoGfxMatrix4x4[16];
 typedef mozilla::dom::StyleChildrenIterator RawGeckoStyleChildrenIterator;
 
+// A struct which to be used as an opaque pointer to a DeclarationBlock that has
+// been read.
+//
+// This is effectively a *const PropertyDeclarationBlock in Rust.
+struct RawServoUnlockedDeclarationBlock;
+
+// A callback that can be sent via FFI which will be invoked _right before_
+// being mutated, and at most once.
+struct DeclarationBlockMutationClosure
+{
+  // The callback function, first argument is the unlocked servo declaration
+  // block, second is `data`.
+  void (*function)(const RawServoUnlockedDeclarationBlock*, void*) = nullptr;
+  void* data = nullptr;
+};
+
 // We have these helper types so that we can directly generate
 // things like &T or Borrowed<T> on the Rust side in the function, providing
 // additional safety benefits.
 //
 // FFI has a problem with templated types, so we just use raw pointers here.
 //
 // The "Borrowed" types generate &T or Borrowed<T> in the nullable case.
 //
@@ -131,17 +147,17 @@ struct MOZ_MUST_USE_TYPE ComputedStyleSt
   DECL_BORROWED_MUT_REF_TYPE_FOR(type_)
 
 #define DECL_NULLABLE_OWNED_REF_TYPE_FOR(type_)    \
   typedef type_* type_##OwnedOrNull;               \
   DECL_NULLABLE_BORROWED_REF_TYPE_FOR(type_)       \
   DECL_NULLABLE_BORROWED_MUT_REF_TYPE_FOR(type_)
 
 // This is a reference to a reference of RawServoDeclarationBlock, which
-// corresponds to Option<&Arc<RawServoDeclarationBlock>> in Servo side.
+// corresponds to Option<&Arc<Locked<RawServoDeclarationBlock>>> in Servo side.
 DECL_NULLABLE_BORROWED_REF_TYPE_FOR(RawServoDeclarationBlockStrong)
 DECL_OWNED_REF_TYPE_FOR(RawServoAuthorStyles)
 DECL_NULLABLE_BORROWED_REF_TYPE_FOR(RawServoAuthorStyles)
 DECL_OWNED_REF_TYPE_FOR(RawServoStyleSet)
 DECL_NULLABLE_BORROWED_REF_TYPE_FOR(RawServoStyleSet)
 DECL_NULLABLE_OWNED_REF_TYPE_FOR(StyleChildrenIterator)
 DECL_OWNED_REF_TYPE_FOR(StyleChildrenIterator)
 DECL_OWNED_REF_TYPE_FOR(ServoElementSnapshot)
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -205,16 +205,17 @@ whitelist-vars = [
     "kNameSpaceID_.*",
     "kGenericFont_.*",
     "kPresContext_.*",
     "nsContentUtils_.*",
     "GECKO_IS_NIGHTLY",
 ]
 whitelist-types = [
     "RawGecko.*",
+    "DeclarationBlockMutationClosure",
     "mozilla::AnimationPropertySegment",
     "mozilla::AnonymousCounterStyle",
     "mozilla::AtomArray",
     "mozilla::ComputedTiming",
     "mozilla::ComputedTimingFunction",
     "mozilla::ComputedTimingFunction::BeforeFlag",
     "mozilla::SeenPtrs",
     "mozilla::ServoElementSnapshot.*",
@@ -460,16 +461,18 @@ structs-types = [
     "mozilla::AtomArray",
     "mozilla::FontStretch",
     "mozilla::FontSlantStyle",
     "mozilla::FontWeight",
     "mozilla::MallocSizeOf",
     "mozilla::OriginFlags",
     "mozilla::UniquePtr",
     "ServoRawOffsetArc",
+    "DeclarationBlockMutationClosure",
+    "RawServoUnlockedDeclarationBlock",
     "nsIContent",
     "nsINode",
     "nsIDocument",
     "nsIDocument_DocumentTheme",
     "nsSimpleContentList",
     "MediumFeaturesChangedResult",
     "RawGeckoAnimationPropertySegment",
     "RawGeckoComputedTiming",
--- a/layout/style/nsDOMCSSDeclaration.cpp
+++ b/layout/style/nsDOMCSSDeclaration.cpp
@@ -285,17 +285,17 @@ nsDOMCSSDeclaration::ParsePropertyValue(
                                         nsIPrincipal* aSubjectPrincipal)
 {
   return ModifyDeclaration(
     aSubjectPrincipal,
     [&](DeclarationBlock* decl, ParsingEnvironment& env) {
       NS_ConvertUTF16toUTF8 value(aPropValue);
       return Servo_DeclarationBlock_SetPropertyById(
         decl->Raw(), aPropID, &value, aIsImportant, env.mUrlExtraData,
-        ParsingMode::Default, env.mCompatMode, env.mLoader);
+        ParsingMode::Default, env.mCompatMode, env.mLoader, /* aClosure = */ { });
     });
 }
 
 nsresult
 nsDOMCSSDeclaration::ParseCustomPropertyValue(const nsAString& aPropertyName,
                                               const nsAString& aPropValue,
                                               bool aIsImportant,
                                               nsIPrincipal* aSubjectPrincipal)
@@ -303,17 +303,17 @@ nsDOMCSSDeclaration::ParseCustomProperty
   MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aPropertyName));
   return ModifyDeclaration(
     aSubjectPrincipal,
     [&](DeclarationBlock* decl, ParsingEnvironment& env) {
       NS_ConvertUTF16toUTF8 property(aPropertyName);
       NS_ConvertUTF16toUTF8 value(aPropValue);
       return Servo_DeclarationBlock_SetProperty(
         decl->Raw(), &property, &value, aIsImportant, env.mUrlExtraData,
-        ParsingMode::Default, env.mCompatMode, env.mLoader);
+        ParsingMode::Default, env.mCompatMode, env.mLoader, /* aClosure = */ { });
     });
 }
 
 nsresult
 nsDOMCSSDeclaration::RemovePropertyInternal(nsCSSPropertyID aPropID)
 {
   DeclarationBlock* olddecl = GetCSSDeclaration(eOperation_RemoveProperty);
   if (!olddecl) {
--- a/layout/style/test/gtest/StyloParsingBench.cpp
+++ b/layout/style/test/gtest/StyloParsingBench.cpp
@@ -57,17 +57,18 @@ static void ServoSetPropertyByIdBench(co
     Servo_DeclarationBlock_SetPropertyById(
       block,
       eCSSProperty_width,
       &css,
       /* is_important = */ false,
       data,
       ParsingMode::Default,
       eCompatibility_FullStandards,
-      nullptr
+      nullptr,
+      { }
     );
   }
 }
 
 static void ServoGetPropertyValueById() {
   RefPtr<RawServoDeclarationBlock> block = Servo_DeclarationBlock_CreateEmpty().Consume();
   RefPtr<URLExtraData> data = new URLExtraData(
     NullPrincipalURI::Create(), nullptr, NullPrincipal::CreateWithoutOriginAttributes());
@@ -76,17 +77,18 @@ static void ServoGetPropertyValueById() 
   Servo_DeclarationBlock_SetPropertyById(
     block,
     eCSSProperty_width,
     &css,
     /* is_important = */ false,
     data,
     ParsingMode::Default,
     eCompatibility_FullStandards,
-    nullptr
+    nullptr,
+    { }
   );
 
   for (int i = 0; i < GETPROPERTY_REPETITIONS; i++) {
     DOMString value_;
     nsAString& value = value_;
     Servo_DeclarationBlock_GetPropertyValueById(
       block,
       eCSSProperty_width,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -84,17 +84,17 @@ use style::gecko_bindings::bindings::Raw
 use style::gecko_bindings::bindings::RawServoStyleRuleBorrowed;
 use style::gecko_bindings::bindings::RawServoStyleSet;
 use style::gecko_bindings::bindings::nsCSSValueBorrowedMut;
 use style::gecko_bindings::bindings::nsTArrayBorrowed_uintptr_t;
 use style::gecko_bindings::bindings::nsTimingFunctionBorrowed;
 use style::gecko_bindings::bindings::nsTimingFunctionBorrowedMut;
 use style::gecko_bindings::structs;
 use style::gecko_bindings::structs::{CallerType, CSSPseudoElementType, CompositeOperation};
-use style::gecko_bindings::structs::{Loader, LoaderReusableStyleSheets};
+use style::gecko_bindings::structs::{DeclarationBlockMutationClosure, Loader, LoaderReusableStyleSheets};
 use style::gecko_bindings::structs::{RawServoStyleRule, ComputedStyleStrong, RustString};
 use style::gecko_bindings::structs::{SheetParsingMode, nsAtom, nsCSSPropertyID};
 use style::gecko_bindings::structs::{StyleSheet as DomStyleSheet, SheetLoadData, SheetLoadDataHolder};
 use style::gecko_bindings::structs::{nsCSSFontDesc, nsCSSCounterDesc};
 use style::gecko_bindings::structs::{nsRestyleHint, nsChangeHint, PropertyValuePair};
 use style::gecko_bindings::structs::AtomArray;
 use style::gecko_bindings::structs::IterationCompositeOperation;
 use style::gecko_bindings::structs::MallocSizeOf as GeckoMallocSizeOf;
@@ -161,16 +161,31 @@ use style::values::distance::ComputeSqua
 use style::values::generics::rect::Rect;
 use style::values::specified;
 use style::values::specified::gecko::{IntersectionObserverRootMargin, PixelOrPercentage};
 use style::values::specified::source_size_list::SourceSizeList;
 use style_traits::{CssType, CssWriter, ParsingMode, StyleParseErrorKind, ToCss};
 use super::error_reporter::ErrorReporter;
 use super::stylesheet_loader::{AsyncStylesheetParser, StylesheetLoader};
 
+trait ClosureHelper {
+    fn invoke(&self, decls: &PropertyDeclarationBlock);
+}
+
+impl ClosureHelper for DeclarationBlockMutationClosure {
+    #[inline]
+    fn invoke(&self, decls: &PropertyDeclarationBlock) {
+        if let Some(function) = self.function.as_ref() {
+            unsafe {
+                function(decls as *const _ as *const _, self.data);
+            }
+        }
+    }
+}
+
 /*
  * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in
  * the C header in Gecko. In order to catch accidental mismatches, we run rust-bindgen against
  * those signatures as well, giving us a second declaration of all the Servo_* functions in this
  * crate. If there's a mismatch, LLVM will assert and abort, which is a rather awful thing to
  * depend on but good enough for our purposes.
  */
 
@@ -3389,24 +3404,36 @@ pub extern "C" fn Servo_DeclarationBlock
 ) -> bool {
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
     *Locked::<PropertyDeclarationBlock>::as_arc(&a).read_with(&guard).declarations() ==
     *Locked::<PropertyDeclarationBlock>::as_arc(&b).read_with(&guard).declarations()
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_DeclarationBlock_GetCssText(declarations: RawServoDeclarationBlockBorrowed,
-                                                    result: *mut nsAString) {
+pub unsafe extern "C" fn Servo_DeclarationBlock_GetCssText(
+    declarations: RawServoDeclarationBlockBorrowed,
+    result: *mut nsAString,
+) {
     read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
-        decls.to_css(unsafe { result.as_mut().unwrap() }).unwrap();
+        decls.to_css(&mut *result).unwrap()
     })
 }
 
 #[no_mangle]
+pub unsafe extern "C" fn Servo_UnlockedDeclarationBlock_GetCssText(
+    declarations: *const structs::RawServoUnlockedDeclarationBlock,
+    result: *mut nsAString,
+) {
+    let decls = &*(declarations as *const PropertyDeclarationBlock);
+    decls.to_css(&mut *result).unwrap()
+}
+
+
+#[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue(
     declarations: RawServoDeclarationBlockBorrowed,
     property_id: nsCSSPropertyID, buffer: *mut nsAString,
     computed_values: ComputedStyleBorrowedOrNull,
     custom_properties: RawServoDeclarationBlockBorrowedOrNull,
 ) {
     let property_id = get_property_id_from_nscsspropertyid!(property_id, ());
 
@@ -3524,56 +3551,68 @@ pub unsafe extern "C" fn Servo_Declarati
 fn set_property(
     declarations: RawServoDeclarationBlockBorrowed,
     property_id: PropertyId,
     value: *const nsACString,
     is_important: bool,
     data: *mut URLExtraData,
     parsing_mode: structs::ParsingMode,
     quirks_mode: QuirksMode,
-    loader: *mut Loader
+    loader: *mut Loader,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
     let mut source_declarations = SourcePropertyDeclaration::new();
     let reporter = ErrorReporter::new(ptr::null_mut(), loader, data);
-    match parse_property_into(&mut source_declarations, property_id, value, data,
-                              parsing_mode, quirks_mode, &reporter) {
-        Ok(()) => {
-            let importance = if is_important { Importance::Important } else { Importance::Normal };
-            write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
-                decls.extend(
-                    source_declarations.drain(),
-                    importance,
-                    DeclarationSource::CssOm
-                )
-            })
-        },
-        Err(_) => false,
+    let result = parse_property_into(
+        &mut source_declarations,
+        property_id,
+        value,
+        data,
+        parsing_mode,
+        quirks_mode,
+        &reporter,
+    );
+
+    if result.is_err() {
+        return false;
     }
+
+    let importance = if is_important { Importance::Important } else { Importance::Normal };
+    write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
+        before_change_closure.invoke(&*decls);
+        decls.extend(
+            source_declarations.drain(),
+            importance,
+            DeclarationSource::CssOm
+        )
+    })
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_DeclarationBlock_SetProperty(
     declarations: RawServoDeclarationBlockBorrowed,
     property: *const nsACString,
     value: *const nsACString,
     is_important: bool,
     data: *mut URLExtraData,
     parsing_mode: structs::ParsingMode,
     quirks_mode: nsCompatibility,
     loader: *mut Loader,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
     set_property(
         declarations,
         get_property_id_from_property!(property, false),
         value,
         is_important,
         data,
         parsing_mode,
         quirks_mode.into(),
         loader,
+        before_change_closure,
     )
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyToAnimationValue(
     declarations: RawServoDeclarationBlockBorrowed,
     animation_value: RawServoAnimationValueBorrowed,
 ) -> bool {
@@ -3591,52 +3630,67 @@ pub unsafe extern "C" fn Servo_Declarati
     declarations: RawServoDeclarationBlockBorrowed,
     property: nsCSSPropertyID,
     value: *const nsACString,
     is_important: bool,
     data: *mut URLExtraData,
     parsing_mode: structs::ParsingMode,
     quirks_mode: nsCompatibility,
     loader: *mut Loader,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
     set_property(
         declarations,
         get_property_id_from_nscsspropertyid!(property, false),
         value,
         is_important,
         data,
         parsing_mode,
         quirks_mode.into(),
         loader,
+        before_change_closure,
     )
 }
 
 fn remove_property(
     declarations: RawServoDeclarationBlockBorrowed,
-    property_id: PropertyId
+    property_id: PropertyId,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
     write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
-        decls.remove_property(&property_id, |_| {})
+        decls.remove_property(&property_id, |decls| {
+            before_change_closure.invoke(decls)
+        })
     })
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_DeclarationBlock_RemoveProperty(
     declarations: RawServoDeclarationBlockBorrowed,
     property: *const nsACString,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
-    remove_property(declarations, get_property_id_from_property!(property, false))
+    remove_property(
+        declarations,
+        get_property_id_from_property!(property, false),
+        before_change_closure,
+    )
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_RemovePropertyById(
     declarations: RawServoDeclarationBlockBorrowed,
-    property: nsCSSPropertyID
+    property: nsCSSPropertyID,
+    before_change_closure: DeclarationBlockMutationClosure,
 ) -> bool {
-    remove_property(declarations, get_property_id_from_nscsspropertyid!(property, false))
+    remove_property(
+        declarations,
+        get_property_id_from_nscsspropertyid!(property, false),
+        before_change_closure,
+    )
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_MediaList_Create() -> RawServoMediaListStrong {
     let global_style_data = &*GLOBAL_STYLE_DATA;
     Arc::new(global_style_data.shared_lock.wrap(MediaList::empty())).into_strong()
 }