Bug 1346693 - Part 2: stylo: Use namespace ids for content: attr(..); r?heycam draft
authorManish Goregaokar <manishearth@gmail.com>
Wed, 31 May 2017 18:03:33 -0700
changeset 587572 913251c40fb9fbf078f6874f14d573b8371310db
parent 587571 b8e551ae5c995af2024edd2648776ece3b73ede8
child 587573 89da042f7ae6effbaeeddf5b2e68880407c1966e
push id61754
push userbmo:manishearth@gmail.com
push dateThu, 01 Jun 2017 09:47:26 +0000
reviewersheycam
bugs1346693
milestone55.0a1
Bug 1346693 - Part 2: stylo: Use namespace ids for content: attr(..); r?heycam MozReview-Commit-ID: FZ9YEpHQCBh
layout/reftests/bugs/reftest.list
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
servo/components/style/gecko/generated/bindings.rs
servo/components/style/gecko/selector_parser.rs
servo/components/style/parser.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/counters.mako.rs
servo/components/style/servo/selector_parser.rs
servo/components/style/stylesheets.rs
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -976,17 +976,17 @@ fuzzy-if(Android,11,17) == 413361-1.html
 == 413840-ltr-offsets.html 413840-ltr-offsets-ref.html
 == 413840-rtl-offsets.html 413840-rtl-offsets-ref.html
 == 413840-pushed-line-bullet.html 413840-pushed-line-bullet-ref.html
 == 413840-bullet-first-line.html 413840-bullet-first-line-ref.html
 == 413982.html 413982-ref.html
 == 414123.xhtml 414123-ref.xhtml
 == 414638.html 414638-ref.html
 == 414851-1.html 414851-1-ref.html
-fails-if(styloVsGecko||stylo) == 416106-1.xhtml 416106-1-ref.xhtml
+== 416106-1.xhtml 416106-1-ref.xhtml
 == 416752-1.html 416752-1-ref.html
 == 417178-1.html 417178-1-ref.html
 == 417246-1.html 417246-1-ref.html
 == 417676.html 417676-ref.html
 asserts(1) asserts-if(styloVsGecko,2) == 418574-1.html 418574-1-ref.html # bug 478135
 asserts(1) asserts-if(styloVsGecko,2) == 418574-2.html 418574-2-ref.html # bug 478135
 == 418766-1a.html 418766-1-ref.html
 == 418766-1b.html 418766-1-ref.html
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -2234,16 +2234,33 @@ Gecko_CSSFontFaceRule_GetCssText(const n
 
 void
 Gecko_AddPropertyToSet(nsCSSPropertyIDSetBorrowedMut aPropertySet,
                        nsCSSPropertyID aProperty)
 {
   aPropertySet->AddProperty(aProperty);
 }
 
+int32_t
+Gecko_RegisterNamespace(nsIAtom* aNamespace)
+{
+  int32_t id;
+
+  MOZ_ASSERT(NS_IsMainThread());
+
+  nsAutoString str;
+  aNamespace->ToString(str);
+  nsresult rv = nsContentUtils::NameSpaceManager()->RegisterNameSpace(str, id);
+
+  if (NS_FAILED(rv)) {
+    return -1;
+  }
+  return id;
+}
+
 NS_IMPL_FFI_REFCOUNTING(nsCSSFontFaceRule, CSSFontFaceRule);
 
 nsCSSCounterStyleRule*
 Gecko_CSSCounterStyle_Create(nsIAtom* aName)
 {
   RefPtr<nsCSSCounterStyleRule> rule = new nsCSSCounterStyleRule(aName, 0, 0);
   return rule.forget().take();
 }
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -553,16 +553,20 @@ nscolor Gecko_GetLookAndFeelSystemColor(
 
 bool Gecko_MatchStringArgPseudo(RawGeckoElementBorrowed element,
                                 mozilla::CSSPseudoClassType type,
                                 const char16_t* ident,
                                 bool* set_slow_selector);
 
 void Gecko_AddPropertyToSet(nsCSSPropertyIDSetBorrowedMut, nsCSSPropertyID);
 
+// Register a namespace and get a namespace id.
+// Returns -1 on error (OOM)
+int32_t Gecko_RegisterNamespace(nsIAtom* ns);
+
 // Style-struct management.
 #define STYLE_STRUCT(name, checkdata_cb)                                       \
   void Gecko_Construct_Default_nsStyle##name(                                  \
     nsStyle##name* ptr,                                                        \
     RawGeckoPresContextBorrowed pres_context);                                 \
   void Gecko_CopyConstruct_nsStyle##name(nsStyle##name* ptr,                   \
                                          const nsStyle##name* other);          \
   void Gecko_Destroy_nsStyle##name(nsStyle##name* ptr);
--- a/servo/components/style/gecko/generated/bindings.rs
+++ b/servo/components/style/gecko/generated/bindings.rs
@@ -1395,16 +1395,19 @@ extern "C" {
                                       ident: *const u16,
                                       set_slow_selector: *mut bool) -> bool;
 }
 extern "C" {
     pub fn Gecko_AddPropertyToSet(arg1: nsCSSPropertyIDSetBorrowedMut,
                                   arg2: nsCSSPropertyID);
 }
 extern "C" {
+    pub fn Gecko_RegisterNamespace(ns: *mut nsIAtom) -> i32;
+}
+extern "C" {
     pub fn Gecko_Construct_Default_nsStyleFont(ptr: *mut nsStyleFont,
                                                pres_context:
                                                    RawGeckoPresContextBorrowed);
 }
 extern "C" {
     pub fn Gecko_CopyConstruct_nsStyleFont(ptr: *mut nsStyleFont,
                                            other: *const nsStyleFont);
 }
--- a/servo/components/style/gecko/selector_parser.rs
+++ b/servo/components/style/gecko/selector_parser.rs
@@ -268,21 +268,21 @@ impl<'a> ::selectors::Parser for Selecto
     }
 
     fn parse_pseudo_element(&self, name: Cow<str>) -> Result<PseudoElement, ()> {
         PseudoElement::from_slice(&name, self.in_user_agent_stylesheet())
             .ok_or(())
     }
 
     fn default_namespace(&self) -> Option<Namespace> {
-        self.namespaces.default.clone()
+        self.namespaces.default.clone().map(|(ns, _)| ns)
     }
 
     fn namespace_for_prefix(&self, prefix: &Atom) -> Option<Namespace> {
-        self.namespaces.prefixes.get(prefix).cloned()
+        self.namespaces.prefixes.get(prefix).cloned().map(|(ns, _)| ns)
     }
 }
 
 impl SelectorImpl {
     #[inline]
     /// Legacy alias for PseudoElement::cascade_type.
     pub fn pseudo_element_cascade_type(pseudo: &PseudoElement) -> PseudoElementCascadeType {
         pseudo.cascade_type()
--- a/servo/components/style/parser.rs
+++ b/servo/components/style/parser.rs
@@ -2,19 +2,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! The context within which CSS code is parsed.
 
 use context::QuirksMode;
 use cssparser::{Parser, SourcePosition, UnicodeRange};
 use error_reporting::ParseErrorReporter;
+use parking_lot::RwLock;
 use style_traits::OneOrMoreCommaSeparated;
 use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces};
-use parking_lot::RwLock;
 
 bitflags! {
     /// The mode to use when parsing values.
     pub flags ParsingMode: u8 {
         /// In CSS, lengths must have units, except for zero values, where the unit can be omitted.
         /// https://www.w3.org/TR/css3-values/#lengths
         const PARSING_MODE_DEFAULT = 0x00,
         /// In SVG, a coordinate or length value without a unit identifier (e.g., "25") is assumed
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -4398,20 +4398,20 @@ clip-path
                             unsafe {
                                 // NB: we share allocators, so doing this is fine.
                                 *self.gecko.mContents[i].mContent.mString.as_mut() =
                                     as_utf16_and_forget(&value);
                             }
                         }
                         ContentItem::Attr(ns, val) => {
                             self.gecko.mContents[i].mType = eStyleContentType_Attr;
-                            let s = if let Some(ns) = ns {
+                            let s = if let Some((_, ns)) = ns {
                                 format!("{}|{}", ns, val)
                             } else {
-                                val
+                                val.into()
                             };
                             unsafe {
                                 // NB: we share allocators, so doing this is fine.
                                 *self.gecko.mContents[i].mContent.mString.as_mut() =
                                     as_utf16_and_forget(&s);
                             }
                         }
                         ContentItem::OpenQuote
--- a/servo/components/style/properties/longhand/counters.mako.rs
+++ b/servo/components/style/properties/longhand/counters.mako.rs
@@ -9,16 +9,18 @@
 <%helpers:longhand name="content" boxed="True" animation_value_type="none"
                    spec="https://drafts.csswg.org/css-content/#propdef-content">
     use cssparser::Token;
     use values::computed::ComputedValueAsSpecified;
     #[cfg(feature = "gecko")]
     use values::generics::CounterStyleOrNone;
     #[cfg(feature = "gecko")]
     use values::specified::url::SpecifiedUrl;
+    #[cfg(feature = "gecko")]
+    use gecko_string_cache::namespace::Namespace;
 
     #[cfg(feature = "servo")]
     use super::list_style_type;
 
     pub use self::computed_value::T as SpecifiedValue;
     pub use self::computed_value::ContentItem;
 
     impl ComputedValueAsSpecified for SpecifiedValue {}
@@ -30,16 +32,18 @@
         use style_traits::ToCss;
         #[cfg(feature = "gecko")]
         use values::specified::url::SpecifiedUrl;
 
         #[cfg(feature = "servo")]
         type CounterStyleType = super::super::list_style_type::computed_value::T;
         #[cfg(feature = "gecko")]
         type CounterStyleType = ::values::generics::CounterStyleOrNone;
+        #[cfg(feature = "gecko")]
+        use gecko_string_cache::namespace::Namespace;
 
         #[derive(Debug, PartialEq, Eq, Clone)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
         pub enum ContentItem {
             /// Literal string content.
             String(String),
             /// `counter(name, style)`.
             Counter(String, CounterStyleType),
@@ -51,17 +55,17 @@
             CloseQuote,
             /// `no-open-quote`.
             NoOpenQuote,
             /// `no-close-quote`.
             NoCloseQuote,
 
             % if product == "gecko":
                 /// `attr([namespace? `|`]? ident)`
-                Attr(Option<String>, String),
+                Attr(Option<(Namespace, u32)>, String),
                 /// `url(url)`
                 Url(SpecifiedUrl),
             % endif
         }
 
         impl ToCss for ContentItem {
             fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
                 match *self {
@@ -88,17 +92,17 @@
                     ContentItem::CloseQuote => dest.write_str("close-quote"),
                     ContentItem::NoOpenQuote => dest.write_str("no-open-quote"),
                     ContentItem::NoCloseQuote => dest.write_str("no-close-quote"),
 
                     % if product == "gecko":
                         ContentItem::Attr(ref ns, ref attr) => {
                             dest.write_str("attr(")?;
                             if let Some(ref ns) = *ns {
-                                cssparser::Token::Ident((&**ns).into()).to_css(dest)?;
+                                cssparser::Token::Ident(ns.0.to_string().into()).to_css(dest)?;
                                 dest.write_str("|")?;
                             }
                             cssparser::Token::Ident((&**attr).into()).to_css(dest)?;
                             dest.write_str(")")
                         }
                         ContentItem::Url(ref url) => url.to_css(dest),
                     % endif
                 }
@@ -203,33 +207,48 @@
                         }),
                         % if product == "gecko":
                             "attr" => input.parse_nested_block(|input| {
                                 // Syntax is `[namespace? `|`]? ident`
                                 // no spaces allowed
                                 // FIXME (bug 1346693) we should be checking that
                                 // this is a valid namespace and encoding it as a namespace
                                 // number from the map
-                                let first = input.try(|i| i.expect_ident()).ok().map(|i| i.into_owned());
+                                let first = input.try(|i| i.expect_ident()).ok();
                                 if let Ok(token) = input.try(|i| i.next_including_whitespace()) {
                                     match token {
                                         Token::Delim('|') => {
                                             // must be followed by an ident
                                             let tok2 = input.next_including_whitespace()?;
                                             if let Token::Ident(second) = tok2 {
-                                                return Ok(ContentItem::Attr(first, second.into_owned()))
+                                                let first: Option<Namespace> = first.map(|i| i.into());
+                                                let first_with_id = match (first, context.namespaces) {
+                                                    (Some(prefix), Some(map)) => {
+                                                        let map = map.read();
+                                                        if let Some(ref entry) = map.prefixes.get(&prefix.0) {
+                                                            Some((prefix, entry.1))
+                                                        } else {
+                                                            return Err(())
+                                                        }
+                                                    }
+                                                    // if we don't have a namespace map (e.g. in CSSOM)
+                                                    // we can't parse namespaces
+                                                    (Some(_), None) => return Err(()),
+                                                    _ => None
+                                                };
+                                                return Ok(ContentItem::Attr(first_with_id, second.into_owned()))
                                             } else {
                                                 return Err(())
                                             }
                                         }
                                         _ => return Err(())
                                     }
                                 }
                                 if let Some(first) = first {
-                                    Ok(ContentItem::Attr(None, first))
+                                    Ok(ContentItem::Attr(None, first.into_owned()))
                                 } else {
                                     Err(())
                                 }
                             }),
                         % endif
                         _ => return Err(())
                     }));
                 }
--- a/servo/components/style/servo/selector_parser.rs
+++ b/servo/components/style/servo/selector_parser.rs
@@ -425,17 +425,17 @@ impl<'a> ::selectors::Parser for Selecto
             },
             _ => return Err(())
         };
 
         Ok(pseudo_element)
     }
 
     fn default_namespace(&self) -> Option<Namespace> {
-        self.namespaces.default.clone()
+        self.namespaces.default.clone().map(|(ns, _)| ns)
     }
 
     fn namespace_for_prefix(&self, prefix: &Prefix) -> Option<Namespace> {
         self.namespaces.prefixes.get(prefix).cloned()
     }
 }
 
 impl SelectorImpl {
--- a/servo/components/style/stylesheets.rs
+++ b/servo/components/style/stylesheets.rs
@@ -91,21 +91,23 @@ pub enum Origin {
     /// http://dev.w3.org/csswg/css-cascade/#cascade-origin-author
     Author,
 
     /// http://dev.w3.org/csswg/css-cascade/#cascade-origin-user
     User,
 }
 
 /// A set of namespaces applying to a given stylesheet.
+///
+/// The u32 is the namespace id, used in gecko
 #[derive(Clone, Default, Debug)]
 #[allow(missing_docs)]
 pub struct Namespaces {
-    pub default: Option<Namespace>,
-    pub prefixes: FnvHashMap<Prefix , Namespace>,
+    pub default: Option<(Namespace, u32)>,
+    pub prefixes: FnvHashMap<Prefix, (Namespace, u32)>,
 }
 
 /// Like gecko_bindings::structs::MallocSizeOf, but without the Option<> wrapper. Note that
 /// functions of this type should not be called via do_malloc_size_of(), rather than directly.
 pub type MallocSizeOfFn = unsafe extern "C" fn(ptr: *const c_void) -> usize;
 
 /// Call malloc_size_of on ptr, first checking that the allocation isn't empty.
 pub unsafe fn do_malloc_size_of<T>(malloc_size_of: MallocSizeOfFn, ptr: *const T) -> usize {
@@ -1521,16 +1523,31 @@ enum AtRulePrelude {
     Keyframes(KeyframesName, Option<VendorPrefix>, SourceLocation),
     /// A @page rule prelude.
     Page(SourceLocation),
     /// A @document rule, with its conditional.
     Document(DocumentCondition, SourceLocation),
 }
 
 
+#[cfg(feature = "gecko")]
+fn register_namespace(ns: &Namespace) -> Result<u32, ()> {
+    let id = unsafe { ::gecko_bindings::bindings::Gecko_RegisterNamespace(ns.0.as_ptr()) };
+    if id == -1 {
+        Err(())
+    } else {
+        Ok(id as u32)
+    }
+}
+
+#[cfg(feature = "servo")]
+fn register_namespace(ns: &Namespace) -> Result<u32, ()> {
+    Ok(1) // servo doesn't use namespace ids
+}
+
 impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
     type Prelude = AtRulePrelude;
     type AtRule = CssRule;
 
     fn parse_prelude(&mut self, name: &str, input: &mut Parser)
                      -> Result<AtRuleType<AtRulePrelude, CssRule>, ()> {
         let location = get_location_with_offset(input.current_source_location(),
                                                 self.context.line_number_offset);
@@ -1579,24 +1596,26 @@ impl<'a> AtRuleParser for TopLevelRulePa
             },
             "namespace" => {
                 if self.state.get() <= State::Namespaces {
                     self.state.set(State::Namespaces);
 
                     let prefix_result = input.try(|input| input.expect_ident());
                     let url = Namespace::from(try!(input.expect_url_or_string()));
 
+                    let id = register_namespace(&url)?;
+
                     let opt_prefix = if let Ok(prefix) = prefix_result {
                         let prefix = Prefix::from(prefix);
                         self.context.namespaces.expect("namespaces must be set whilst parsing rules")
-                                               .write().prefixes.insert(prefix.clone(), url.clone());
+                                               .write().prefixes.insert(prefix.clone(), (url.clone(), id));
                         Some(prefix)
                     } else {
                         self.context.namespaces.expect("namespaces must be set whilst parsing rules")
-                                               .write().default = Some(url.clone());
+                                               .write().default = Some((url.clone(), id));
                         None
                     };
 
                     return Ok(AtRuleType::WithoutBlock(CssRule::Namespace(Arc::new(
                         self.shared_lock.wrap(NamespaceRule {
                             prefix: opt_prefix,
                             url: url,
                             source_location: location,