Auto merge of #19841 - bholley:single_tocss, r=emilio

stylo: Avoid separate monomorphizations of CSS serialization for utf-8 and utf-16

https://bugzilla.mozilla.org/show_bug.cgi?id=1431268

<!-- 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/19841)
<!-- Reviewable:end -->

components/script/dom/cssstyledeclaration.rs

@@ -21,7 +21,7 @@ use style::properties::{DeclarationSource, Importance, PropertyDeclarationBlock,
 use style::properties::{parse_one_declaration_into, parse_style_attribute, SourcePropertyDeclaration};
 use style::selector_parser::PseudoElement;
 use style::shared_lock::Locked;
-use style_traits::{ParsingMode, ToCss};
+use style_traits::ParsingMode;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
 #[dom_struct]
@@ -85,7 +85,8 @@ impl CSSStyleOwner {
                     // [1]: https://github.com/whatwg/html/issues/2306
                     if let Some(pdb) = attr {
                         let guard = shared_lock.read();
-                        let serialization = pdb.read_with(&guard).to_css_string();
+                        let mut serialization = String::new();
+                        pdb.read_with(&guard).to_css(&mut serialization).unwrap();
                         el.set_attribute(&local_name!("style"),
                                          AttrValue::Declaration(serialization,
                                                                 pdb));
@@ -415,7 +416,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         self.owner.with_block(|pdb| {
             pdb.declarations().get(index as usize).map(|declaration| {
                 let important = pdb.declarations_importance().get(index);
-                let mut css = declaration.to_css_string();
+                let mut css = String::new();
+                declaration.to_css(&mut css).unwrap();
                 if important {
                     css += " !important";
                 }
@@ -427,7 +429,9 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext
     fn CssText(&self) -> DOMString {
         self.owner.with_block(|pdb| {
-            DOMString::from(pdb.to_css_string())
+            let mut serialization = String::new();
+            pdb.to_css(&mut serialization).unwrap();
+            DOMString::from(serialization)
         })
     }
 

components/style/counter_style/mod.rs

@@ -17,8 +17,9 @@ use selectors::parser::SelectorParseErrorKind;
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
 #[allow(unused_imports)] use std::ascii::AsciiExt;
 use std::borrow::Cow;
-use std::fmt;
+use std::fmt::{self, Write};
 use std::ops::Range;
+use str::CssStringWriter;
 use style_traits::{Comma, OneOrMoreSeparated, ParseError, StyleParseErrorKind, ToCss};
 use values::CustomIdent;
 
@@ -228,8 +229,7 @@ macro_rules! counter_style_descriptors {
         }
 
         impl ToCssWithGuard for CounterStyleRuleData {
-            fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-            where W: fmt::Write {
+            fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
                 dest.write_str("@counter-style ")?;
                 self.name.to_css(dest)?;
                 dest.write_str(" {\n")?;

components/style/font_face.rs

@@ -20,7 +20,8 @@ use parser::{ParserContext, ParserErrorContext, Parse};
 use properties::longhands::font_language_override;
 use selectors::parser::SelectorParseErrorKind;
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::{Comma, OneOrMoreSeparated, ParseError, StyleParseErrorKind, ToCss};
 use values::computed::font::FamilyName;
 use values::specified::url::SpecifiedUrl;
@@ -272,8 +273,7 @@ macro_rules! font_face_descriptors_common {
 
         impl ToCssWithGuard for FontFaceRuleData {
             // Serialization of FontFaceRule is not specced.
-            fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-            where W: fmt::Write {
+            fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
                 dest.write_str("@font-face {\n")?;
                 $(
                     if let Some(ref value) = self.$ident {

components/style/gecko/rules.rs

@@ -17,7 +17,9 @@ use gecko_bindings::sugar::refptr::{RefPtr, UniqueRefPtr};
 use nsstring::nsString;
 use properties::longhands::font_language_override;
 use shared_lock::{ToCssWithGuard, SharedRwLockReadGuard};
-use std::{fmt, str};
+use std::fmt::{self, Write};
+use std::str;
+use str::CssStringWriter;
 use values::computed::font::FamilyName;
 use values::generics::FontSettings;
 
@@ -200,8 +202,7 @@ impl From<FontFaceRuleData> for FontFaceRule {
 }
 
 impl ToCssWithGuard for FontFaceRule {
-    fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         let mut css_text = nsString::new();
         unsafe {
             bindings::Gecko_CSSFontFaceRule_GetCssText(self.get(), &mut *css_text);
@@ -237,8 +238,7 @@ impl From<counter_style::CounterStyleRuleData> for CounterStyleRule {
 }
 
 impl ToCssWithGuard for CounterStyleRule {
-    fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         let mut css_text = nsString::new();
         unsafe {
             bindings::Gecko_CSSCounterStyle_GetCssText(self.get(), &mut *css_text);

components/style/properties/declaration_block.rs

@@ -19,6 +19,7 @@ use smallvec::SmallVec;
 use std::fmt;
 use std::iter::{DoubleEndedIterator, Zip};
 use std::slice::Iter;
+use str::{CssString, CssStringBorrow, CssStringWriter};
 use style_traits::{ToCss, ParseError, ParsingMode, StyleParseErrorKind};
 use stylesheets::{CssRuleType, Origin, UrlExtraData};
 use super::*;
@@ -306,9 +307,7 @@ impl PropertyDeclarationBlock {
     /// Find the value of the given property in this block and serialize it
     ///
     /// <https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue>
-    pub fn property_value_to_css<W>(&self, property: &PropertyId, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+    pub fn property_value_to_css(&self, property: &PropertyId, dest: &mut CssStringWriter) -> fmt::Result {
         // Step 1.1: done when parsing a string to PropertyId
 
         // Step 1.2
@@ -608,15 +607,13 @@ impl PropertyDeclarationBlock {
     }
 
     /// Take a declaration block known to contain a single property and serialize it.
-    pub fn single_value_to_css<W>(
+    pub fn single_value_to_css(
         &self,
         property: &PropertyId,
-        dest: &mut W,
+        dest: &mut CssStringWriter,
         computed_values: Option<&ComputedValues>,
         custom_properties_block: Option<&PropertyDeclarationBlock>,
     ) -> fmt::Result
-    where
-        W: fmt::Write,
     {
         match property.as_shorthand() {
             Err(_longhand_or_custom) => {
@@ -664,7 +661,7 @@ impl PropertyDeclarationBlock {
                 let iter = self.declarations.iter();
                 match shorthand.get_shorthand_appendable_value(iter) {
                     Some(AppendableValue::Css { css, .. }) => {
-                        dest.write_str(css)
+                        css.append_to(dest)
                     },
                     Some(AppendableValue::DeclarationsForShorthand(_, decls)) => {
                         shorthand.longhands_to_css(decls, dest)
@@ -738,11 +735,13 @@ impl PropertyDeclarationBlock {
     }
 }
 
-impl ToCss for PropertyDeclarationBlock {
-    // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+impl PropertyDeclarationBlock {
+    /// Like the method on ToCss, but without the type parameter to avoid
+    /// accidentally monomorphizing this large function multiple times for
+    /// different writers.
+    ///
+    /// https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
+    pub fn to_css(&self, dest: &mut CssStringWriter) -> fmt::Result {
         let mut is_first_serialization = true; // trailing serializations should have a prepended space
 
         // Step 1 -> dest = result list
@@ -835,7 +834,7 @@ impl ToCss for PropertyDeclarationBlock {
 
                     // We avoid re-serializing if we're already an
                     // AppendableValue::Css.
-                    let mut v = String::new();
+                    let mut v = CssString::new();
                     let value = match (appendable_value, found_system) {
                         (AppendableValue::Css { css, with_variables }, _) => {
                             debug_assert!(!css.is_empty());
@@ -848,7 +847,7 @@ impl ToCss for PropertyDeclarationBlock {
                         (_, Some(sys)) => {
                             sys.to_css(&mut v)?;
                             AppendableValue::Css {
-                                css: &v,
+                                css: CssStringBorrow::from(&v),
                                 with_variables: false,
                             }
                         }
@@ -861,7 +860,7 @@ impl ToCss for PropertyDeclarationBlock {
                             }
 
                             AppendableValue::Css {
-                                css: &v,
+                                css: CssStringBorrow::from(&v),
                                 with_variables: false,
                             }
                         }
@@ -871,14 +870,14 @@ impl ToCss for PropertyDeclarationBlock {
                     // We need to check the shorthand whether it's an alias property or not.
                     // If it's an alias property, it should be serialized like its longhand.
                     if shorthand.flags().contains(PropertyFlags::SHORTHAND_ALIAS_PROPERTY) {
-                        append_serialization::<_, Cloned<slice::Iter< _>>, _>(
+                        append_serialization::<Cloned<slice::Iter< _>>, _>(
                              dest,
                              &property,
                              value,
                              importance,
                              &mut is_first_serialization)?;
                     } else {
-                        append_serialization::<_, Cloned<slice::Iter< _>>, _>(
+                        append_serialization::<Cloned<slice::Iter< _>>, _>(
                              dest,
                              &shorthand,
                              value,
@@ -913,7 +912,7 @@ impl ToCss for PropertyDeclarationBlock {
             // "error: unable to infer enough type information about `_`;
             //  type annotations or generic parameter binding required [E0282]"
             // Use the same type as earlier call to reuse generated code.
-            append_serialization::<_, Cloned<slice::Iter<_>>, _>(
+            append_serialization::<Cloned<slice::Iter<_>>, _>(
                 dest,
                 &property,
                 AppendableValue::Declaration(declaration),
@@ -945,7 +944,7 @@ pub enum AppendableValue<'a, I>
     /// or when storing a serialized shorthand value before appending directly.
     Css {
         /// The raw CSS string.
-        css: &'a str,
+        css: CssStringBorrow<'a>,
         /// Whether the original serialization contained variables or not.
         with_variables: bool,
     }
@@ -966,15 +965,16 @@ fn handle_first_serialization<W>(dest: &mut W,
 }
 
 /// Append a given kind of appendable value to a serialization.
-pub fn append_declaration_value<'a, W, I>(dest: &mut W,
-                                          appendable_value: AppendableValue<'a, I>)
-                                          -> fmt::Result
-    where W: fmt::Write,
-          I: Iterator<Item=&'a PropertyDeclaration>,
+pub fn append_declaration_value<'a, I>(
+    dest: &mut CssStringWriter,
+    appendable_value: AppendableValue<'a, I>,
+) -> fmt::Result
+where
+    I: Iterator<Item=&'a PropertyDeclaration>,
 {
     match appendable_value {
         AppendableValue::Css { css, .. } => {
-            dest.write_str(css)
+            css.append_to(dest)
         },
         AppendableValue::Declaration(decl) => {
             decl.to_css(dest)
@@ -986,15 +986,16 @@ pub fn append_declaration_value<'a, W, I>(dest: &mut W,
 }
 
 /// Append a given property and value pair to a serialization.
-pub fn append_serialization<'a, W, I, N>(dest: &mut W,
-                                         property_name: &N,
-                                         appendable_value: AppendableValue<'a, I>,
-                                         importance: Importance,
-                                         is_first_serialization: &mut bool)
-                                         -> fmt::Result
-    where W: fmt::Write,
-          I: Iterator<Item=&'a PropertyDeclaration>,
-          N: ToCss,
+pub fn append_serialization<'a, I, N>(
+    dest: &mut CssStringWriter,
+    property_name: &N,
+    appendable_value: AppendableValue<'a, I>,
+    importance: Importance,
+    is_first_serialization: &mut bool
+) -> fmt::Result
+where
+    I: Iterator<Item=&'a PropertyDeclaration>,
+    N: ToCss,
 {
     handle_first_serialization(dest, is_first_serialization)?;
 

components/style/properties/properties.mako.rs

@@ -16,8 +16,10 @@ use custom_properties::CustomPropertiesBuilder;
 use servo_arc::{Arc, UniqueArc};
 use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
-use std::{fmt, mem, ops};
 use std::cell::RefCell;
+use std::fmt::{self, Write};
+use std::mem;
+use std::ops;
 
 #[cfg(feature = "servo")] use cssparser::RGBA;
 use cssparser::{CowRcStr, Parser, TokenSerializationType, serialize_identifier};
@@ -46,6 +48,7 @@ use values::computed;
 use values::computed::NonNegativeLength;
 use rule_tree::{CascadeLevel, StrongRuleNode};
 use self::computed_value_flags::*;
+use str::{CssString, CssStringBorrow, CssStringWriter};
 use style_adjuster::StyleAdjuster;
 
 pub use self::declaration_block::*;
@@ -898,7 +901,7 @@ impl ShorthandId {
         if let Some(css) = first_declaration.with_variables_from_shorthand(self) {
             if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
                return Some(AppendableValue::Css {
-                   css: css,
+                   css: CssStringBorrow::from(css),
                    with_variables: true,
                });
             }
@@ -909,7 +912,7 @@ impl ShorthandId {
         if let Some(keyword) = first_declaration.get_css_wide_keyword() {
             if declarations2.all(|d| d.get_css_wide_keyword() == Some(keyword)) {
                 return Some(AppendableValue::Css {
-                    css: keyword.to_str(),
+                    css: CssStringBorrow::from(keyword.to_str()),
                     with_variables: false,
                 });
             }
@@ -1463,14 +1466,21 @@ impl fmt::Debug for PropertyDeclaration {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.id().to_css(f)?;
         f.write_str(": ")?;
-        self.to_css(f)
+
+        // Because PropertyDeclaration::to_css requires CssStringWriter, we can't write
+        // it directly to f, and need to allocate an intermediate string. This is
+        // fine for debug-only code.
+        let mut s = CssString::new();
+        self.to_css(&mut s)?;
+        write!(f, "{}", s)
     }
 }
 
-impl ToCss for PropertyDeclaration {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+impl PropertyDeclaration {
+    /// Like the method on ToCss, but without the type parameter to avoid
+    /// accidentally monomorphizing this large function multiple times for
+    /// different writers.
+    pub fn to_css(&self, dest: &mut CssStringWriter) -> fmt::Result {
         match *self {
             % for property in data.longhands:
                 PropertyDeclaration::${property.camel_case}(ref value) =>

components/style/shared_lock.rs

@@ -13,6 +13,7 @@ use std::cell::UnsafeCell;
 use std::fmt;
 #[cfg(feature = "gecko")]
 use std::ptr;
+use str::{CssString, CssStringWriter};
 use stylesheets::Origin;
 
 /// A shared read/write lock that can protect multiple objects.
@@ -219,18 +220,18 @@ mod compile_time_assert {
     impl<'a> Marker2 for SharedRwLockWriteGuard<'a> {}  // Assert SharedRwLockWriteGuard: !Copy
 }
 
-/// Like ToCss, but with a lock guard given by the caller.
+/// Like ToCss, but with a lock guard given by the caller, and with the writer specified
+/// concretely rather than with a parameter.
 pub trait ToCssWithGuard {
     /// Serialize `self` in CSS syntax, writing to `dest`, using the given lock guard.
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write;
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result;
 
     /// Serialize `self` in CSS syntax using the given lock guard and return a string.
     ///
     /// (This is a convenience wrapper for `to_css` and probably should not be overridden.)
     #[inline]
-    fn to_css_string(&self, guard: &SharedRwLockReadGuard) -> String {
-        let mut s = String::new();
+    fn to_css_string(&self, guard: &SharedRwLockReadGuard) -> CssString {
+        let mut s = CssString::new();
         self.to_css(guard, &mut s).unwrap();
         s
     }

components/style/str.rs

@@ -10,6 +10,7 @@ use num_traits::ToPrimitive;
 #[allow(unused_imports)] use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::convert::AsRef;
+use std::fmt::{self, Write};
 use std::iter::{Filter, Peekable};
 use std::str::Split;
 
@@ -163,3 +164,101 @@ pub fn string_as_ascii_lowercase<'a>(input: &'a str) -> Cow<'a, str> {
         Cow::Borrowed(input)
     }
 }
+
+/// To avoid accidentally instantiating multiple monomorphizations of large
+/// serialization routines, we define explicit concrete types and require
+/// them in those routines. This primarily avoids accidental mixing of UTF8
+/// with UTF16 serializations in Gecko.
+#[cfg(feature = "gecko")]
+pub type CssStringWriter = ::nsstring::nsAString;
+
+/// String type that coerces to CssStringWriter, used when serialization code
+/// needs to allocate a temporary string.
+#[cfg(feature = "gecko")]
+pub type CssString = ::nsstring::nsString;
+
+/// Certain serialization code needs to interact with borrowed strings, which
+/// are sometimes native UTF8 Rust strings, and other times serialized UTF16
+/// strings. This enum multiplexes the two cases.
+#[cfg(feature = "gecko")]
+pub enum CssStringBorrow<'a> {
+    /// A borrow of a UTF16 CssString.
+    UTF16(&'a ::nsstring::nsString),
+    /// A borrow of a regular Rust UTF8 string.
+    UTF8(&'a str),
+}
+
+#[cfg(feature = "gecko")]
+impl<'a> CssStringBorrow<'a> {
+    /// Writes the borrowed string to the provided writer.
+    pub fn append_to(&self, dest: &mut CssStringWriter) -> fmt::Result {
+        match *self {
+            CssStringBorrow::UTF16(s) => {
+                dest.append(s);
+                Ok(())
+            },
+            CssStringBorrow::UTF8(s) => dest.write_str(s),
+        }
+    }
+
+    /// Returns true of the borrowed string is empty.
+    pub fn is_empty(&self) -> bool {
+        match *self {
+            CssStringBorrow::UTF16(s) => s.is_empty(),
+            CssStringBorrow::UTF8(s) => s.is_empty(),
+        }
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl<'a> From<&'a str> for CssStringBorrow<'a> {
+    fn from(s: &'a str) -> Self {
+        CssStringBorrow::UTF8(s)
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl<'a> From<&'a ::nsstring::nsString> for CssStringBorrow<'a> {
+    fn from(s: &'a ::nsstring::nsString) -> Self {
+        CssStringBorrow::UTF16(s)
+    }
+}
+
+/// String. The comments for the Gecko types explain the need for this abstraction.
+#[cfg(feature = "servo")]
+pub type CssStringWriter = String;
+
+/// String. The comments for the Gecko types explain the need for this abstraction.
+#[cfg(feature = "servo")]
+pub type CssString = String;
+
+/// Borrowed string. The comments for the Gecko types explain the need for this abstraction.
+#[cfg(feature = "servo")]
+pub struct CssStringBorrow<'a>(&'a str);
+
+#[cfg(feature = "servo")]
+impl<'a> CssStringBorrow<'a> {
+    /// Appends the borrowed string to the given string.
+    pub fn append_to(&self, dest: &mut CssStringWriter) -> fmt::Result {
+        dest.write_str(self.0)
+    }
+
+    /// Returns true if the borrowed string is empty.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+}
+
+#[cfg(feature = "servo")]
+impl<'a> From<&'a str> for CssStringBorrow<'a> {
+    fn from(s: &'a str) -> Self {
+        CssStringBorrow(s)
+    }
+}
+
+#[cfg(feature = "servo")]
+impl<'a> From<&'a String> for CssStringBorrow<'a> {
+    fn from(s: &'a String) -> Self {
+        CssStringBorrow(&*s)
+    }
+}

components/style/stylesheets/document_rule.rs

@@ -13,7 +13,8 @@ use media_queries::Device;
 use parser::{Parse, ParserContext};
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::{ToCss, ParseError, StyleParseErrorKind};
 use stylesheets::CssRules;
 use values::specified::url::SpecifiedUrl;
@@ -40,8 +41,7 @@ impl DocumentRule {
 }
 
 impl ToCssWithGuard for DocumentRule {
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@-moz-document ")?;
         self.condition.to_css(dest)?;
         dest.write_str(" {")?;

components/style/stylesheets/font_feature_values_rule.rs

@@ -16,7 +16,8 @@ use gecko_bindings::bindings::Gecko_AppendFeatureValueHashEntry;
 use gecko_bindings::structs::{self, gfxFontFeatureValueSet, nsTArray};
 use parser::{ParserContext, ParserErrorContext, Parse};
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::{ParseError, StyleParseErrorKind, ToCss};
 use stylesheets::CssRuleType;
 use values::computed::font::FamilyName;
@@ -347,9 +348,7 @@ macro_rules! font_feature_values_blocks {
         }
 
         impl ToCssWithGuard for FontFeatureValuesRule {
-            fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-                where W: fmt::Write
-            {
+            fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
                 dest.write_str("@font-feature-values ")?;
                 self.font_family_to_css(dest)?;
                 dest.write_str(" {\n")?;

components/style/stylesheets/import_rule.rs

@@ -9,7 +9,8 @@
 use cssparser::SourceLocation;
 use media_queries::MediaList;
 use shared_lock::{DeepCloneWithLock, DeepCloneParams, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::ToCss;
 use stylesheets::{StylesheetContents, StylesheetInDocument};
 use values::specified::url::SpecifiedUrl;
@@ -107,9 +108,7 @@ impl DeepCloneWithLock for ImportRule {
 }
 
 impl ToCssWithGuard for ImportRule {
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@import ")?;
         self.url.to_css(dest)?;
 

components/style/stylesheets/keyframes_rule.rs

@@ -14,7 +14,8 @@ use properties::LonghandIdSet;
 use properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::{ParsingMode, ToCss, ParseError, StyleParseErrorKind};
 use stylesheets::{CssRuleType, StylesheetContents};
 use stylesheets::rule_parser::VendorPrefix;
@@ -37,9 +38,7 @@ pub struct KeyframesRule {
 
 impl ToCssWithGuard for KeyframesRule {
     // Serialization of KeyframesRule is not specced.
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@keyframes ")?;
         self.name.to_css(dest)?;
         dest.write_str(" {")?;
@@ -194,8 +193,7 @@ pub struct Keyframe {
 }
 
 impl ToCssWithGuard for Keyframe {
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         self.selector.to_css(dest)?;
         dest.write_str(" { ")?;
         self.block.read_with(guard).to_css(dest)?;

components/style/stylesheets/media_rule.rs

@@ -12,7 +12,8 @@ use malloc_size_of::{MallocSizeOfOps, MallocUnconditionalShallowSizeOf};
 use media_queries::MediaList;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use style_traits::ToCss;
 use stylesheets::CssRules;
 
@@ -42,8 +43,7 @@ impl MediaRule {
 impl ToCssWithGuard for MediaRule {
     // Serialization of MediaRule is not specced.
     // https://drafts.csswg.org/cssom/#serialize-a-css-rule CSSMediaRule
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@media ")?;
         self.media_queries.read_with(guard).to_css(dest)?;
         self.rules.read_with(guard).to_css_block(guard, dest)

components/style/stylesheets/mod.rs

@@ -31,6 +31,7 @@ use parser::{ParserContext, ParserErrorContext};
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
+use str::CssStringWriter;
 use style_traits::ParsingMode;
 
 pub use self::counter_style_rule::CounterStyleRule;
@@ -347,8 +348,7 @@ impl DeepCloneWithLock for CssRule {
 
 impl ToCssWithGuard for CssRule {
     // https://drafts.csswg.org/cssom/#serialize-a-css-rule
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         match *self {
             CssRule::Namespace(ref lock) => lock.read_with(guard).to_css(guard, dest),
             CssRule::Import(ref lock) => lock.read_with(guard).to_css(guard, dest),

components/style/stylesheets/namespace_rule.rs

@@ -7,7 +7,8 @@
 use {Namespace, Prefix};
 use cssparser::SourceLocation;
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 
 /// A `@namespace` rule.
 #[derive(Clone, Debug, PartialEq)]
@@ -23,8 +24,7 @@ pub struct NamespaceRule {
 
 impl ToCssWithGuard for NamespaceRule {
     // https://drafts.csswg.org/cssom/#serialize-a-css-rule CSSNamespaceRule
-    fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@namespace ")?;
         if let Some(ref prefix) = self.prefix {
             dest.write_str(&*prefix.to_string())?;

components/style/stylesheets/page_rule.rs

@@ -12,8 +12,8 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps, MallocUnconditionalShallowSi
 use properties::PropertyDeclarationBlock;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
-use style_traits::ToCss;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 
 /// A [`@page`][page] rule.
 ///
@@ -44,9 +44,7 @@ impl PageRule {
 impl ToCssWithGuard for PageRule {
     /// Serialization of PageRule is not specced, adapted from steps for
     /// StyleRule.
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@page { ")?;
         let declaration_block = self.block.read_with(guard);
         declaration_block.to_css(dest)?;

components/style/stylesheets/rule_list.rs

@@ -9,7 +9,8 @@ use malloc_size_of::{MallocShallowSizeOf, MallocSizeOfOps};
 use servo_arc::{Arc, RawOffsetArc};
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked};
 use shared_lock::{SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 use stylesheets::{CssRule, RulesMutateError};
 use stylesheets::loader::StylesheetLoader;
 use stylesheets::rule_parser::State;
@@ -95,9 +96,7 @@ impl CssRules {
     ///
     /// This should be speced into CSSOM spec at some point. See
     /// <https://github.com/w3c/csswg-drafts/issues/1985>
-    pub fn to_css_block<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W)
-        -> fmt::Result where W: fmt::Write
-    {
+    pub fn to_css_block(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str(" {")?;
         for rule in self.0.iter() {
             dest.write_str("\n  ")?;

components/style/stylesheets/style_rule.rs

@@ -12,8 +12,8 @@ use selector_parser::SelectorImpl;
 use selectors::SelectorList;
 use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
-use std::fmt;
-use style_traits::ToCss;
+use std::fmt::{self, Write};
+use str::CssStringWriter;
 
 /// A style rule, with selectors and declarations.
 #[derive(Debug)]
@@ -67,9 +67,7 @@ impl StyleRule {
 
 impl ToCssWithGuard for StyleRule {
     /// https://drafts.csswg.org/cssom/#serialize-a-css-rule CSSStyleRule
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-        where W: fmt::Write,
-    {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         use cssparser::ToCss;
 
         // Step 1

components/style/stylesheets/supports_rule.rs

@@ -15,8 +15,9 @@ use servo_arc::Arc;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
 #[allow(unused_imports)] use std::ascii::AsciiExt;
 use std::ffi::{CStr, CString};
-use std::fmt;
+use std::fmt::{self, Write};
 use std::str;
+use str::CssStringWriter;
 use style_traits::{ToCss, ParseError};
 use stylesheets::{CssRuleType, CssRules};
 
@@ -46,8 +47,7 @@ impl SupportsRule {
 }
 
 impl ToCssWithGuard for SupportsRule {
-    fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@supports ")?;
         self.condition.to_css(dest)?;
         self.rules.read_with(guard).to_css_block(guard, dest)

components/style/stylesheets/viewport_rule.rs

@@ -23,9 +23,10 @@ use shared_lock::{SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard};
 #[allow(unused_imports)] use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::cell::RefCell;
-use std::fmt;
+use std::fmt::{self, Write};
 use std::iter::Enumerate;
 use std::str::Chars;
+use str::CssStringWriter;
 use style_traits::{PinchZoomFactor, ToCss, ParseError, StyleParseErrorKind};
 use style_traits::viewport::{Orientation, UserZoom, ViewportConstraints, Zoom};
 use stylesheets::{StylesheetInDocument, Origin};
@@ -520,8 +521,7 @@ impl ViewportRule {
 
 impl ToCssWithGuard for ViewportRule {
     // Serialization of ViewportRule is not specced.
-    fn to_css<W>(&self, _guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
-    where W: fmt::Write {
+    fn to_css(&self, _guard: &SharedRwLockReadGuard, dest: &mut CssStringWriter) -> fmt::Result {
         dest.write_str("@viewport { ")?;
         let mut iter = self.declarations.iter();
         iter.next().unwrap().to_css(dest)?;

ports/geckolib/glue.rs

@@ -705,14 +705,11 @@ pub extern "C" fn Servo_AnimationValue_Serialize(value: RawServoAnimationValueBo
                                                  buffer: *mut nsAString)
 {
     let uncomputed_value = AnimationValue::as_arc(&value).uncompute();
-    let mut string = String::new();
+    let buffer = unsafe { buffer.as_mut().unwrap() };
     let rv = PropertyDeclarationBlock::with_one(uncomputed_value, Importance::Normal)
-        .single_value_to_css(&get_property_id_from_nscsspropertyid!(property, ()), &mut string,
+        .single_value_to_css(&get_property_id_from_nscsspropertyid!(property, ()), buffer,
                              None, None /* No extra custom properties */);
     debug_assert!(rv.is_ok());
-
-    let buffer = unsafe { buffer.as_mut().unwrap() };
-    buffer.assign_utf8(&string);
 }
 
 #[no_mangle]
@@ -2677,16 +2674,11 @@ pub extern "C" fn Servo_DeclarationBlock_SerializeOneValue(
     let guard = global_style_data.shared_lock.read();
     let decls = Locked::<PropertyDeclarationBlock>::as_arc(&declarations).read_with(&guard);
 
-    let mut string = String::new();
-
     let custom_properties = Locked::<PropertyDeclarationBlock>::arc_from_borrowed(&custom_properties);
     let custom_properties = custom_properties.map(|block| block.read_with(&guard));
-    let rv = decls.single_value_to_css(
-        &property_id, &mut string, computed_values, custom_properties);
-    debug_assert!(rv.is_ok());
-
     let buffer = unsafe { buffer.as_mut().unwrap() };
-    buffer.assign_utf8(&string);
+    let rv = decls.single_value_to_css(&property_id, buffer, computed_values, custom_properties);
+    debug_assert!(rv.is_ok());
 }
 
 #[no_mangle]

tests/unit/style/properties/serialization.rs

@@ -5,6 +5,7 @@
 use properties::{parse, parse_input};
 use style::computed_values::display::T as Display;
 use style::properties::{PropertyDeclaration, Importance};
+use style::properties::declaration_block::PropertyDeclarationBlock;
 use style::properties::parse_property_declaration_list;
 use style::values::{CustomIdent, RGBA};
 use style::values::generics::flex::FlexBasis;
@@ -15,6 +16,18 @@ use style::values::specified::url::SpecifiedUrl;
 use style_traits::ToCss;
 use stylesheets::block_from;
 
+trait ToCssString {
+    fn to_css_string(&self) -> String;
+}
+
+impl ToCssString for PropertyDeclarationBlock {
+    fn to_css_string(&self) -> String {
+        let mut css = String::new();
+        self.to_css(&mut css).unwrap();
+        css
+    }
+}
+
 #[test]
 fn property_declaration_block_should_serialize_correctly() {
     use style::properties::longhands::overflow_x::SpecifiedValue as OverflowValue;