Bug 1507305 - Add a mechanism to serialize shorthands for getComputedStyle(). r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Nov 2018 08:25:13 +0000
changeset 502983 5201cfe20675cb2ab6dbf4ddff079c176a807fad
parent 502982 6c54e79a66b51815649c5c8488050f6e12206bcb
child 502984 76aa81358d762cbc618c1913061b57eccc145b1d
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1507305
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1507305 - Add a mechanism to serialize shorthands for getComputedStyle(). r=heycam This implements the mechanism reusing the animation machinery for now, so it asserts in a few cases that this wouldn't handle correctly. For shorthands that have colors and other bits we'd need a more sophisticated mechanism with a bit more code (that resolves colors and such), but it'd look something like this regardless, and we should have this in any case. Differential Revision: https://phabricator.services.mozilla.com/D11944
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -304,17 +304,19 @@ impl PropertyDeclarationBlock {
                 return None;
             }
         }
 
         self.declaration_importance_iter()
             .find(|(declaration, _)| declaration.id() == property)
     }
 
-    fn shorthand_to_css(
+    /// Tries to serialize a given shorthand from the declarations in this
+    /// block.
+    pub fn shorthand_to_css(
         &self,
         shorthand: ShorthandId,
         dest: &mut CssStringWriter,
     ) -> fmt::Result {
         // Step 1.2.1 of
         // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue
         let mut list = SmallVec::<[&_; 10]>::new();
         let mut important_count = 0;
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1349,16 +1349,23 @@ impl ShorthandId {
 
     /// Converts from a ShorthandId to an adequate nsCSSPropertyID.
     #[cfg(feature = "gecko")]
     #[inline]
     pub fn to_nscsspropertyid(self) -> nsCSSPropertyID {
         NonCustomPropertyId::from(self).to_nscsspropertyid()
     }
 
+    /// Converts from a nsCSSPropertyID to a ShorthandId.
+    #[cfg(feature = "gecko")]
+    #[inline]
+    pub fn from_nscsspropertyid(prop: nsCSSPropertyID) -> Result<Self, ()> {
+        PropertyId::from_nscsspropertyid(prop)?.as_shorthand().map_err(|_| ())
+    }
+
     /// Get the longhand ids that form this shorthand.
     pub fn longhands(&self) -> NonCustomPropertyIterator<LonghandId> {
         % for property in data.shorthands:
             static ${property.ident.upper()}: &'static [LonghandId] = &[
                 % for sub in property.sub_properties:
                     LonghandId::${sub.camel_case},
                 % endfor
             ];
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -5393,31 +5393,58 @@ pub extern "C" fn Servo_StyleSet_HasDocu
     let state = DocumentState::from_bits_truncate(state);
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
 
     data.stylist.has_document_state_dependency(state)
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_GetPropertyValue(
-    computed_values: ComputedStyleBorrowed,
+    style: ComputedStyleBorrowed,
     prop: nsCSSPropertyID,
     value: *mut nsAString,
 ) {
     use style::properties::PropertyFlags;
 
-    let longhand = LonghandId::from_nscsspropertyid(prop).expect("Not a longhand?");
-    debug_assert!(
-        !longhand.flags().contains(PropertyFlags::GETCS_NEEDS_LAYOUT_FLUSH),
-        "We're not supposed to serialize layout-dependent properties"
-    );
-    computed_values.get_longhand_property_value(
-        longhand,
-        &mut CssWriter::new(&mut *value),
-    ).unwrap();
+    if let Ok(longhand) = LonghandId::from_nscsspropertyid(prop) {
+        debug_assert!(
+            !longhand.flags().contains(PropertyFlags::GETCS_NEEDS_LAYOUT_FLUSH),
+            "We're not supposed to serialize layout-dependent properties"
+        );
+        style.get_longhand_property_value(
+            longhand,
+            &mut CssWriter::new(&mut *value),
+        ).unwrap();
+        return;
+    }
+
+    let shorthand = ShorthandId::from_nscsspropertyid(prop)
+        .expect("Not a shorthand nor a longhand?");
+    let mut block = PropertyDeclarationBlock::new();
+    // NOTE(emilio): We reuse the animation value machinery to avoid blowing up
+    // code size, but may need to come up with something different if ever care
+    // about supporting the cases that assert below. Fortunately we don't right
+    // now.
+    for longhand in shorthand.longhands() {
+        debug_assert!(
+            !longhand.is_logical(),
+            "This won't quite do the right thing if we want to serialize \
+             logical shorthands"
+        );
+        debug_assert!(
+            !longhand.flags().contains(PropertyFlags::GETCS_NEEDS_LAYOUT_FLUSH),
+            "Layout-dependent properties shouldn't get here"
+        );
+        let animated = AnimationValue::from_computed_values(longhand, style)
+            .expect("Somebody tried to serialize a shorthand with \
+                     non-animatable properties, would need more code \
+                     to do this");
+        block.push(animated.uncompute(), Importance::Normal);
+    }
+    block.shorthand_to_css(shorthand, &mut *value).unwrap();
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn Servo_GetCustomPropertyValue(
     computed_values: ComputedStyleBorrowed,
     name: *const nsAString,
     value: *mut nsAString,
 ) -> bool {