Bug 1346693 - Part 3: stylo: Create separate Attr type; r?heycam draft
authorManish Goregaokar <manishearth@gmail.com>
Wed, 31 May 2017 18:03:33 -0700
changeset 587575 356a995dd6ec33f8b309a2b9fc319191c9d062c2
parent 587573 89da042f7ae6effbaeeddf5b2e68880407c1966e
child 631315 1bef6d5185b885548ac545c5e2b6dbe830bf710b
push id61755
push userbmo:manishearth@gmail.com
push dateThu, 01 Jun 2017 09:53:54 +0000
reviewersheycam
bugs1346693
milestone55.0a1
Bug 1346693 - Part 3: stylo: Create separate Attr type; r?heycam MozReview-Commit-ID: I9C7lhY093C
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/counters.mako.rs
servo/components/style/stylesheets.rs
servo/components/style/values/specified/mod.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -4396,22 +4396,22 @@ clip-path
                         ContentItem::String(value) => {
                             self.gecko.mContents[i].mType = eStyleContentType_String;
                             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) => {
+                        ContentItem::Attr(attr) => {
                             self.gecko.mContents[i].mType = eStyleContentType_Attr;
-                            let s = if let Some((_, ns)) = ns {
-                                format!("{}|{}", ns, val)
+                            let s = if let Some((_, ns)) = attr.namespace {
+                                format!("{}|{}", ns, attr.attribute)
                             } else {
-                                val.into()
+                                attr.attribute.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
@@ -10,17 +10,17 @@
                    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;
+    use values::specified::Attr;
 
     #[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 {}
@@ -32,18 +32,19 @@
         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;
+        use values::specified::Attr;
 
         #[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),
@@ -55,17 +56,17 @@
             CloseQuote,
             /// `no-open-quote`.
             NoOpenQuote,
             /// `no-close-quote`.
             NoCloseQuote,
 
             % if product == "gecko":
                 /// `attr([namespace? `|`]? ident)`
-                Attr(Option<(Namespace, u32)>, String),
+                Attr(Attr),
                 /// `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 {
@@ -89,24 +90,18 @@
                         dest.write_str(")")
                     }
                     ContentItem::OpenQuote => dest.write_str("open-quote"),
                     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.0.to_string().into()).to_css(dest)?;
-                                dest.write_str("|")?;
-                            }
-                            cssparser::Token::Ident((&**attr).into()).to_css(dest)?;
-                            dest.write_str(")")
+                        ContentItem::Attr(ref attr) => {
+                            attr.to_css(dest)
                         }
                         ContentItem::Url(ref url) => url.to_css(dest),
                     % endif
                 }
             }
         }
 
         #[derive(Debug, PartialEq, Eq, Clone)]
@@ -202,56 +197,17 @@
                             let name = try!(input.expect_ident()).into_owned();
                             try!(input.expect_comma());
                             let separator = try!(input.expect_string()).into_owned();
                             let style = parse_counter_style(context, input);
                             Ok(ContentItem::Counters(name, separator, style))
                         }),
                         % 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();
-                                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 {
-                                                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.into_owned()))
-                                } else {
-                                    Err(())
-                                }
+                                Ok(ContentItem::Attr(Attr::parse_function(context, input)?))
                             }),
                         % endif
                         _ => return Err(())
                     }));
                 }
                 Ok(Token::Ident(ident)) => {
                     match_ignore_ascii_case! { &ident,
                         "open-quote" => content.push(ContentItem::OpenQuote),
--- a/servo/components/style/stylesheets.rs
+++ b/servo/components/style/stylesheets.rs
@@ -46,16 +46,17 @@ use std::os::raw::c_void;
 use std::slice;
 use std::sync::atomic::{AtomicBool, Ordering};
 use str::starts_with_ignore_ascii_case;
 use style_traits::ToCss;
 use stylearc::Arc;
 use stylist::FnvHashMap;
 use supports::SupportsCondition;
 use values::{CustomIdent, KeyframesName};
+use values::specified::NamespaceId;
 use values::specified::url::SpecifiedUrl;
 use viewport::ViewportRule;
 
 
 /// Extra data that the backend may need to resolve url values.
 #[cfg(not(feature = "gecko"))]
 pub type UrlExtraData = ServoUrl;
 
@@ -92,22 +93,23 @@ pub enum Origin {
     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
+/// The NamespaceId is registered when parsing in Gecko
+/// and used in the style structs
 #[derive(Clone, Default, Debug)]
 #[allow(missing_docs)]
 pub struct Namespaces {
-    pub default: Option<(Namespace, u32)>,
-    pub prefixes: FnvHashMap<Prefix, (Namespace, u32)>,
+    pub default: Option<(Namespace, NamespaceId)>,
+    pub prefixes: FnvHashMap<Prefix, (Namespace, NamespaceId)>,
 }
 
 /// 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 {
@@ -1524,28 +1526,28 @@ enum AtRulePrelude {
     /// 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, ()> {
+fn register_namespace(ns: &Namespace) -> Result<i32, ()> {
     let id = unsafe { ::gecko_bindings::bindings::Gecko_RegisterNamespace(ns.0.as_ptr()) };
     if id == -1 {
         Err(())
     } else {
-        Ok(id as u32)
+        Ok(id)
     }
 }
 
 #[cfg(feature = "servo")]
-fn register_namespace(_: &Namespace) -> Result<u32, ()> {
-    Ok(1) // servo doesn't use namespace ids
+fn register_namespace(_: &Namespace) -> Result<(), ()> {
+    Ok(()) // 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>, ()> {
--- a/servo/components/style/values/specified/mod.rs
+++ b/servo/components/style/values/specified/mod.rs
@@ -1,16 +1,17 @@
 /* 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 http://mozilla.org/MPL/2.0/. */
 
 //! Specified values.
 //!
 //! TODO(emilio): Enhance docs.
 
+use Namespace;
 use app_units::Au;
 use context::QuirksMode;
 use cssparser::{self, Parser, Token};
 use itoa;
 use parser::{ParserContext, Parse};
 use self::grid::TrackSizeOrRepeat;
 use self::url::SpecifiedUrl;
 use std::ascii::AsciiExt;
@@ -19,16 +20,17 @@ use std::fmt;
 use std::io::Write;
 use style_traits::ToCss;
 use style_traits::values::specified::AllowedNumericType;
 use super::{Auto, CSSFloat, CSSInteger, Either, None_};
 use super::computed::{self, Context};
 use super::computed::{Shadow as ComputedShadow, ToComputedValue};
 use super::generics::grid::{TrackBreadth as GenericTrackBreadth, TrackSize as GenericTrackSize};
 use super::generics::grid::TrackList as GenericTrackList;
+use values::computed::ComputedValueAsSpecified;
 use values::specified::calc::CalcNode;
 
 #[cfg(feature = "gecko")]
 pub use self::align::{AlignItems, AlignJustifyContent, AlignJustifySelf, JustifyItems};
 pub use self::background::BackgroundSize;
 pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth};
 pub use self::border::{BorderImageWidthSide, BorderRadius};
 pub use self::color::Color;
@@ -1358,8 +1360,116 @@ pub enum AllowQuirks {
 }
 
 impl AllowQuirks {
     /// Returns `true` if quirks are allowed in this context.
     pub fn allowed(self, quirks_mode: QuirksMode) -> bool {
         self == AllowQuirks::Yes && quirks_mode == QuirksMode::Quirks
     }
 }
+
+#[cfg(feature = "gecko")]
+/// A namespace ID
+pub type NamespaceId = i32;
+
+
+#[cfg(feature = "servo")]
+/// A namespace ID (used by gecko only)
+pub type NamespaceId = ();
+
+/// An attr(...) rule
+///
+/// `[namespace? `|`]? ident`
+#[derive(Clone, PartialEq, Eq, Debug)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+pub struct Attr {
+    /// Optional namespace
+    pub namespace: Option<(Namespace, NamespaceId)>,
+    /// Attribute name
+    pub attribute: String,
+}
+
+impl Parse for Attr {
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Attr, ()> {
+        input.expect_function_matching("attr")?;
+        input.parse_nested_block(|i| Attr::parse_function(context, i))
+    }
+}
+
+#[cfg(feature = "gecko")]
+/// Get the namespace id from the namespace map
+pub fn get_id_for_namespace(namespace: &Namespace, context: &ParserContext) -> Result<NamespaceId, ()> {
+    if let Some(map) = context.namespaces {
+        if let Some(ref entry) = map.read().prefixes.get(&namespace.0) {
+            Ok(entry.1)
+        } else {
+            Err(())
+        }
+    } else {
+        // if we don't have a namespace map (e.g. in inline styles)
+        // we can't parse namespaces
+        Err(())
+    }
+}
+
+
+#[cfg(feature = "servo")]
+/// Get the namespace id from the namespace map
+pub fn get_id_for_namespace(_: &Namespace, _: &ParserContext) -> Result<NamespaceId, ()> {
+    Ok(())
+}
+
+impl Attr {
+    /// Parse contents of attr() assuming we have already parsed `attr` and are
+    /// within a parse_nested_block()
+    pub fn parse_function(context: &ParserContext, input: &mut Parser) -> Result<Attr, ()> {
+        // Syntax is `[namespace? `|`]? ident`
+        // no spaces allowed
+        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 {
+                        let ns_with_id = if let Some(ns) = first {
+                            let ns: Namespace = ns.into();
+                            let id = get_id_for_namespace(&ns, context)?;
+                            Some((ns, id))
+                        } else {
+                            None
+                        };
+                        return Ok(Attr {
+                            namespace: ns_with_id,
+                            attribute: second.into_owned(),
+                        })
+                    } else {
+                        return Err(())
+                    }
+                }
+                _ => return Err(())
+            }
+        }
+        if let Some(first) = first {
+            Ok(Attr {
+                namespace: None,
+                attribute: first.into_owned(),
+            })
+        } else {
+            Err(())
+        }
+    }
+}
+
+
+impl ToCss for Attr {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        dest.write_str("attr(")?;
+        if let Some(ref ns) = self.namespace {
+            cssparser::Token::Ident(ns.0.to_string().into()).to_css(dest)?;
+            dest.write_str("|")?;
+        }
+        cssparser::Token::Ident((&*self.attribute).into()).to_css(dest)?;
+        dest.write_str(")")
+    }
+}
+
+impl ComputedValueAsSpecified for Attr {}