Bug 1550554 - Use ArcSlice for quotes. r=heycam

This saves the intermediate allocation.

Differential Revision: https://phabricator.services.mozilla.com/D30546

layout/base/nsQuoteList.cpp

@@ -37,10 +37,29 @@ nsString nsQuoteNode::Text() {
   NS_ASSERTION(mType == StyleContentType::OpenQuote ||
                    mType == StyleContentType::CloseQuote,
                "should only be called when mText should be non-null");
-
   nsString result;
-  Servo_Quotes_GetQuote(mPseudoFrame->StyleList()->mQuotes.get(), Depth(),
-                        mType, &result);
+  int32_t depth = Depth();
+  MOZ_ASSERT(depth >= -1);
+
+  Span<const StyleQuotePair> quotes =
+    mPseudoFrame->StyleList()->mQuotes._0.AsSpan();
+
+  // Reuse the last pair when the depth is greater than the number of
+  // pairs of quotes.  (Also make 'quotes: none' and close-quote from
+  // a depth of 0 equivalent for the next test.)
+  if (depth >= static_cast<int32_t>(quotes.Length())) {
+      depth = static_cast<int32_t>(quotes.Length()) - 1;
+  }
+
+  if (depth == -1) {
+    // close-quote from a depth of 0 or 'quotes: none'
+    return result;
+  }
+
+  const StyleQuotePair& pair = quotes[depth];
+  const StyleOwnedStr& quote =
+    mType == StyleContentType::OpenQuote ? pair.opening : pair.closing;
+  result.Assign(NS_ConvertUTF8toUTF16(quote.AsString()));
   return result;
 }
 

layout/style/ServoBindings.toml

@@ -481,6 +481,8 @@ cbindgen-types = [
     { gecko = "StyleArcSlice", servo = "style_traits::arc_slice::ArcSlice" },
     { gecko = "StyleForgottenArcSlicePtr", servo = "style_traits::arc_slice::ForgottenArcSlicePtr" },
     { gecko = "StyleMozContextProperties", servo = "values::specified::svg::MozContextProperties" },
+    { gecko = "StyleQuotes", servo = "values::specified::list::Quotes" },
+    { gecko = "StyleOwnedStr", servo = "style_traits::owned_str::OwnedStr" },
 ]
 
 mapped-generic-types = [

layout/style/ServoStyleConstsInlines.h

@@ -111,6 +111,12 @@ inline StyleAtom::StyleAtom(const StyleAtom& aOther) : _0(aOther._0) {
   }
 }
 
+inline nsDependentCSubstring StyleOwnedStr::AsString() const {
+  Span<const uint8_t> s = _0.AsSpan();
+  return nsDependentCSubstring(reinterpret_cast<const char*>(s.Elements()),
+                               s.Length());
+}
+
 }  // namespace mozilla
 
 #endif

layout/style/nsStyleStruct.cpp

@@ -480,12 +480,12 @@ nsChangeHint nsStyleOutline::CalcDifference(
 //
 nsStyleList::nsStyleList(const Document& aDocument)
     : mListStylePosition(NS_STYLE_LIST_STYLE_POSITION_OUTSIDE),
+      mQuotes{StyleArcSlice<StyleQuotePair>(Servo_Quotes_GetInitialValue())},
       mMozListReversed(StyleMozListReversed::False) {
   MOZ_COUNT_CTOR(nsStyleList);
   MOZ_ASSERT(NS_IsMainThread());
 
   mCounterStyle = nsGkAtoms::disc;
-  mQuotes = Servo_Quotes_GetInitialValue().Consume();
 }
 
 nsStyleList::~nsStyleList() { MOZ_COUNT_DTOR(nsStyleList); }
@@ -514,8 +514,7 @@ nsChangeHint nsStyleList::CalcDifference(
     const nsStyleList& aNewData, const nsStyleDisplay& aOldDisplay) const {
   // If the quotes implementation is ever going to change we might not need
   // a framechange here and a reflow should be sufficient.  See bug 35768.
-  if (mQuotes != aNewData.mQuotes &&
-      !Servo_Quotes_Equal(mQuotes.get(), aNewData.mQuotes.get())) {
+  if (mQuotes != aNewData.mQuotes) {
     return nsChangeHint_ReconstructFrame;
   }
   nsChangeHint hint = nsChangeHint(0);

layout/style/nsStyleStruct.h

@@ -1103,7 +1103,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleList {
   nsStyleList& operator=(const nsStyleList& aOther) = delete;
 
  public:
-  RefPtr<RawServoQuotes> mQuotes;
+  mozilla::StyleQuotes mQuotes;
   nsRect mImageRegion;  // the rect to use within an image
   mozilla::StyleMozListReversed
       mMozListReversed;  // true in an <ol reversed> scope

servo/components/style/gecko/arc_types.rs

@@ -23,7 +23,6 @@ use crate::gecko_bindings::structs::RawServoMediaRule;
 use crate::gecko_bindings::structs::RawServoMozDocumentRule;
 use crate::gecko_bindings::structs::RawServoNamespaceRule;
 use crate::gecko_bindings::structs::RawServoPageRule;
-use crate::gecko_bindings::structs::RawServoQuotes;
 use crate::gecko_bindings::structs::RawServoStyleRule;
 use crate::gecko_bindings::structs::RawServoStyleSheetContents;
 use crate::gecko_bindings::structs::RawServoSupportsRule;
@@ -40,7 +39,6 @@ use crate::stylesheets::{NamespaceRule, PageRule};
 use crate::stylesheets::{StyleRule, StylesheetContents, SupportsRule};
 use servo_arc::{Arc, ArcBorrow};
 use std::{mem, ptr};
-use values::computed::QuotePair;
 
 macro_rules! impl_arc_ffi {
     ($servo_type:ty => $gecko_type:ty[$addref:ident, $release:ident]) => {
@@ -115,9 +113,6 @@ impl_arc_ffi!(Locked<CounterStyleRule> => RawServoCounterStyleRule
 impl_arc_ffi!(CssUrlData => RawServoCssUrlData
               [Servo_CssUrlData_AddRef, Servo_CssUrlData_Release]);
 
-impl_arc_ffi!(Box<[QuotePair]> => RawServoQuotes
-              [Servo_Quotes_AddRef, Servo_Quotes_Release]);
-
 // ComputedStyle is not an opaque type on any side of FFI.
 // This means that doing the HasArcFFI type trick is actually unsound,
 // since it gives us a way to construct an Arc<ComputedStyle> from

servo/components/style/lib.rs

@@ -190,6 +190,7 @@ pub use servo_atoms::Atom;
 
 pub use style_traits::arc_slice::ArcSlice;
 pub use style_traits::owned_slice::OwnedSlice;
+pub use style_traits::owned_str::OwnedStr;
 
 /// The CSS properties supported by the style system.
 /// Generated from the properties.mako.rs template by build.rs

servo/components/style/properties/gecko.mako.rs

@@ -3302,7 +3302,7 @@ fn static_assert() {
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="List"
-                  skip_longhands="list-style-image list-style-type quotes -moz-image-region">
+                  skip_longhands="list-style-image list-style-type -moz-image-region">
 
     pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) {
         match image {
@@ -3378,28 +3378,6 @@ fn static_assert() {
         }
     }
 
-    pub fn set_quotes(&mut self, other: longhands::quotes::computed_value::T) {
-        self.gecko.mQuotes.set_arc(other.0.clone());
-    }
-
-    pub fn copy_quotes_from(&mut self, other: &Self) {
-        self.set_quotes(other.clone_quotes());
-    }
-
-    pub fn reset_quotes(&mut self, other: &Self) {
-        self.copy_quotes_from(other)
-    }
-
-    pub fn clone_quotes(&self) -> longhands::quotes::computed_value::T {
-        use gecko_bindings::sugar::ownership::HasArcFFI;
-        use values::computed::QuotePair;
-
-        let quote_pairs = unsafe { &*self.gecko.mQuotes.mRawPtr };
-        longhands::quotes::computed_value::T(
-            Box::<[QuotePair]>::as_arc(&quote_pairs).clone_arc()
-        )
-    }
-
     #[allow(non_snake_case)]
     pub fn set__moz_image_region(&mut self, v: longhands::_moz_image_region::computed_value::T) {
         use crate::values::Either;

servo/components/style/values/computed/list.rs

@@ -9,21 +9,19 @@ pub use crate::values::specified::list::ListStyleType;
 pub use crate::values::specified::list::MozListReversed;
 pub use crate::values::specified::list::{QuotePair, Quotes};
 
-use servo_arc::Arc;
-
 lazy_static! {
-    static ref INITIAL_QUOTES: Arc<Box<[QuotePair]>> = Arc::new(
+    static ref INITIAL_QUOTES: crate::ArcSlice<QuotePair> = crate::ArcSlice::from_iter(
         vec![
             QuotePair {
-                opening: "\u{201c}".to_owned().into_boxed_str(),
-                closing: "\u{201d}".to_owned().into_boxed_str(),
+                opening: "\u{201c}".to_owned().into(),
+                closing: "\u{201d}".to_owned().into(),
             },
             QuotePair {
-                opening: "\u{2018}".to_owned().into_boxed_str(),
-                closing: "\u{2019}".to_owned().into_boxed_str(),
+                opening: "\u{2018}".to_owned().into(),
+                closing: "\u{2019}".to_owned().into(),
             },
         ]
-        .into_boxed_slice()
+        .into_iter()
     );
 }
 

servo/components/style/values/specified/list.rs

@@ -10,7 +10,6 @@ use crate::values::generics::CounterStyleOrNone;
 #[cfg(feature = "gecko")]
 use crate::values::CustomIdent;
 use cssparser::{Parser, Token};
-use servo_arc::Arc;
 use style_traits::{ParseError, StyleParseErrorKind};
 
 /// Specified and computed `list-style-type` property.
@@ -96,18 +95,20 @@ impl Parse for ListStyleType {
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(C)]
 pub struct QuotePair {
     /// The opening quote.
-    pub opening: Box<str>,
+    pub opening: crate::OwnedStr,
 
     /// The closing quote.
-    pub closing: Box<str>,
+    pub closing: crate::OwnedStr,
 }
 
 /// Specified and computed `quotes` property.
 #[derive(
     Clone,
     Debug,
+    Default,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
@@ -116,10 +117,11 @@ pub struct QuotePair {
     ToResolvedValue,
     ToShmem,
 )]
+#[repr(C)]
 pub struct Quotes(
     #[css(iterable, if_empty = "none")]
     #[ignore_malloc_size_of = "Arc"]
-    pub Arc<Box<[QuotePair]>>,
+    pub crate::ArcSlice<QuotePair>,
 );
 
 impl Parse for Quotes {
@@ -131,24 +133,26 @@ impl Parse for Quotes {
             .try(|input| input.expect_ident_matching("none"))
             .is_ok()
         {
-            return Ok(Quotes(Arc::new(Box::new([]))));
+            return Ok(Self::default());
         }
 
         let mut quotes = Vec::new();
         loop {
             let location = input.current_source_location();
             let opening = match input.next() {
-                Ok(&Token::QuotedString(ref value)) => value.as_ref().to_owned().into_boxed_str(),
+                Ok(&Token::QuotedString(ref value)) => {
+                    value.as_ref().to_owned().into()
+                },
                 Ok(t) => return Err(location.new_unexpected_token_error(t.clone())),
                 Err(_) => break,
             };
 
-            let closing = input.expect_string()?.as_ref().to_owned().into_boxed_str();
+            let closing = input.expect_string()?.as_ref().to_owned().into();
             quotes.push(QuotePair { opening, closing });
         }
 
         if !quotes.is_empty() {
-            Ok(Quotes(Arc::new(quotes.into_boxed_slice())))
+            Ok(Quotes(crate::ArcSlice::from_iter(quotes.into_iter())))
         } else {
             Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
         }

servo/components/style_traits/lib.rs

@@ -93,6 +93,7 @@ pub mod values;
 #[macro_use]
 pub mod viewport;
 pub mod owned_slice;
+pub mod owned_str;
 
 pub use crate::specified_value_info::{CssType, KeywordsCollectFn, SpecifiedValueInfo};
 pub use crate::values::{

servo/components/style_traits/owned_str.rs

@@ -0,0 +1,53 @@
+/* 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 https://mozilla.org/MPL/2.0/. */
+
+#![allow(unsafe_code)]
+
+//! A replacement for `Box<str>` that has a defined layout for FFI.
+
+use crate::owned_slice::OwnedSlice;
+use std::fmt;
+use std::ops::{Deref, DerefMut};
+
+/// A struct that basically replaces a Box<str>, but with a defined layout,
+/// suitable for FFI.
+#[repr(C)]
+#[derive(Default, Clone, PartialEq, Eq, MallocSizeOf, ToShmem)]
+pub struct OwnedStr(OwnedSlice<u8>);
+
+impl fmt::Debug for OwnedStr {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        self.deref().fmt(formatter)
+    }
+}
+
+impl Deref for OwnedStr {
+    type Target = str;
+
+    #[inline(always)]
+    fn deref(&self) -> &Self::Target {
+        unsafe { std::str::from_utf8_unchecked(&*self.0) }
+    }
+}
+
+impl DerefMut for OwnedStr {
+    #[inline(always)]
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        unsafe { std::str::from_utf8_unchecked_mut(&mut *self.0) }
+    }
+}
+
+impl From<Box<str>> for OwnedStr {
+    #[inline]
+    fn from(b: Box<str>) -> Self {
+        Self::from(b.into_string())
+    }
+}
+
+impl From<String> for OwnedStr {
+    #[inline]
+    fn from(s: String) -> Self {
+        OwnedStr(s.into_bytes().into())
+    }
+}

servo/components/style_traits/specified_value_info.rs

@@ -84,6 +84,7 @@ impl SpecifiedValueInfo for u16 {}
 impl SpecifiedValueInfo for u32 {}
 impl SpecifiedValueInfo for str {}
 impl SpecifiedValueInfo for String {}
+impl SpecifiedValueInfo for crate::owned_str::OwnedStr {}
 
 #[cfg(feature = "servo")]
 impl SpecifiedValueInfo for ::servo_atoms::Atom {}

servo/components/style_traits/values.rs

@@ -75,6 +75,16 @@ where
     }
 }
 
+impl ToCss for crate::owned_str::OwnedStr {
+    #[inline]
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        serialize_string(self, dest)
+    }
+}
+
 impl ToCss for str {
     #[inline]
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result

servo/ports/geckolib/cbindgen.toml

@@ -128,6 +128,7 @@ include = [
   "ForgottenArcSlicePtr",
   "HeaderWithLength",
   "MozContextProperties",
+  "Quotes",
 ]
 item_types = ["enums", "structs", "typedefs", "functions"]
 renaming_overrides_prefixing = true
@@ -400,3 +401,7 @@ renaming_overrides_prefixing = true
   inline StyleAtom(const StyleAtom& aOther);
   inline ~StyleAtom();
 """
+
+"OwnedStr" = """
+  inline nsDependentCSubstring AsString() const;
+"""

servo/ports/geckolib/glue.rs

@@ -51,7 +51,7 @@ use style::gecko_bindings::bindings::Gecko_NewNoneTransform;
 use style::gecko_bindings::structs;
 use style::gecko_bindings::structs::{Element as RawGeckoElement, nsINode as RawGeckoNode};
 use style::gecko_bindings::structs::{
-    RawServoQuotes, RawServoStyleSet, RawServoAuthorStyles,
+    RawServoStyleSet, RawServoAuthorStyles,
     RawServoCssUrlData, RawServoDeclarationBlock, RawServoMediaList,
     RawServoCounterStyleRule, RawServoAnimationValue, RawServoSupportsRule,
     RawServoKeyframesRule, ServoCssRules, RawServoStyleSheetContents,
@@ -93,7 +93,6 @@ use style::gecko_bindings::structs::ServoTraversalFlags;
 use style::gecko_bindings::structs::SheetLoadData;
 use style::gecko_bindings::structs::SheetLoadDataHolder;
 use style::gecko_bindings::structs::SheetParsingMode;
-use style::gecko_bindings::structs::StyleContentType as ContentType;
 use style::gecko_bindings::structs::StyleRuleInclusion;
 use style::gecko_bindings::structs::StyleSheet as DomStyleSheet;
 use style::gecko_bindings::structs::URLExtraData;
@@ -133,7 +132,7 @@ use style::traversal::DomTraversal;
 use style::traversal_flags::{self, TraversalFlags};
 use style::use_counters::UseCounters;
 use style::values::animated::{Animate, Procedure, ToAnimatedZero};
-use style::values::computed::{self, Context, QuotePair, ToComputedValue};
+use style::values::computed::{self, Context, ToComputedValue};
 use style::values::distance::ComputeSquaredDistance;
 use style::values::specified;
 use style::values::specified::gecko::IntersectionObserverRootMargin;
@@ -6303,52 +6302,8 @@ pub unsafe extern "C" fn Servo_IsCssPropertyRecordedInUseCounter(
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_Quotes_GetInitialValue() -> Strong<RawServoQuotes> {
-    computed::Quotes::get_initial_value()
-        .0
-        .clone()
-        .into_strong()
-}
-
-#[no_mangle]
-pub extern "C" fn Servo_Quotes_Equal(a: &RawServoQuotes, b: &RawServoQuotes) -> bool {
-    let a = Box::<[QuotePair]>::as_arc(&a);
-    let b = Box::<[QuotePair]>::as_arc(&b);
-
-    a == b
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn Servo_Quotes_GetQuote(
-    quotes: &RawServoQuotes,
-    mut depth: i32,
-    quote_type: ContentType,
-    result: *mut nsAString,
-) {
-    debug_assert!(depth >= -1);
-
-    let quotes = Box::<[QuotePair]>::as_arc(&quotes);
-
-    // Reuse the last pair when the depth is greater than the number of
-    // pairs of quotes.  (Also make 'quotes: none' and close-quote from
-    // a depth of 0 equivalent for the next test.)
-    if depth >= quotes.len() as i32 {
-        depth = quotes.len() as i32 - 1;
-    }
-
-    if depth == -1 {
-        // close-quote from a depth of 0 or 'quotes: none'
-        return;
-    }
-
-    let quote_pair = &quotes[depth as usize];
-    let quote = if quote_type == ContentType::OpenQuote {
-        &quote_pair.opening
-    } else {
-        debug_assert!(quote_type == ContentType::CloseQuote);
-        &quote_pair.closing
-    };
-    (*result).write_str(quote).unwrap();
+pub extern "C" fn Servo_Quotes_GetInitialValue() -> style_traits::arc_slice::ForgottenArcSlicePtr<specified::list::QuotePair> {
+    computed::Quotes::get_initial_value().0.forget()
 }
 
 #[no_mangle]