Auto merge of #17130 - Manishearth:stylo-attr, r=heycam,emilio

stylo: Handle attr() in `content`

r=heycam https://bugzilla.mozilla.org/show_bug.cgi?id=1346693

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

components/style/gecko/generated/bindings.rs

@@ -1399,6 +1399,9 @@ 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:

components/style/gecko/selector_parser.rs

@@ -273,11 +273,11 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
     }
 
     fn default_namespace(&self) -> Option<Namespace> {
-        self.namespaces.default.clone()
+        self.namespaces.default.clone().as_ref().map(|&(ref ns, _)| ns.clone())
     }
 
     fn namespace_for_prefix(&self, prefix: &Atom) -> Option<Namespace> {
-        self.namespaces.prefixes.get(prefix).cloned()
+        self.namespaces.prefixes.get(prefix).map(|&(ref ns, _)| ns.clone())
     }
 }
 

components/style/parser.rs

@@ -7,8 +7,9 @@
 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};
+use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces};
 
 bitflags! {
     /// The mode to use when parsing values.
@@ -81,6 +82,8 @@ pub struct ParserContext<'a> {
     pub parsing_mode: ParsingMode,
     /// The quirks mode of this stylesheet.
     pub quirks_mode: QuirksMode,
+    /// The list of all namespaces active in the current stylesheet
+    pub namespaces: Option<&'a RwLock<Namespaces>>,
 }
 
 impl<'a> ParserContext<'a> {
@@ -100,6 +103,7 @@ impl<'a> ParserContext<'a> {
             line_number_offset: 0u64,
             parsing_mode: parsing_mode,
             quirks_mode: quirks_mode,
+            namespaces: None,
         }
     }
 
@@ -125,6 +129,7 @@ impl<'a> ParserContext<'a> {
             line_number_offset: context.line_number_offset,
             parsing_mode: context.parsing_mode,
             quirks_mode: context.quirks_mode,
+            namespaces: context.namespaces,
         }
     }
 
@@ -144,6 +149,7 @@ impl<'a> ParserContext<'a> {
             line_number_offset: line_number_offset,
             parsing_mode: parsing_mode,
             quirks_mode: quirks_mode,
+            namespaces: None,
         }
     }
 

components/style/properties/gecko.mako.rs

@@ -4401,12 +4401,12 @@ clip-path
                                     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
+                                attr.attribute.into()
                             };
                             unsafe {
                                 // NB: we share allocators, so doing this is fine.

components/style/properties/longhand/counters.mako.rs

@@ -14,6 +14,8 @@
     use values::generics::CounterStyleOrNone;
     #[cfg(feature = "gecko")]
     use values::specified::url::SpecifiedUrl;
+    #[cfg(feature = "gecko")]
+    use values::specified::Attr;
 
     #[cfg(feature = "servo")]
     use super::list_style_type;
@@ -36,6 +38,9 @@
         #[cfg(feature = "gecko")]
         type CounterStyleType = ::values::generics::CounterStyleOrNone;
 
+        #[cfg(feature = "gecko")]
+        use values::specified::Attr;
+
         #[derive(Debug, PartialEq, Eq, Clone)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
         pub enum ContentItem {
@@ -56,7 +61,7 @@
 
             % if product == "gecko":
                 /// `attr([namespace? `|`]? ident)`
-                Attr(Option<String>, String),
+                Attr(Attr),
                 /// `url(url)`
                 Url(SpecifiedUrl),
             % endif
@@ -90,14 +95,8 @@
                     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)?;
-                                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
@@ -203,31 +202,7 @@
                         }),
                         % 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());
-                                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()))
-                                            } else {
-                                                return Err(())
-                                            }
-                                        }
-                                        _ => return Err(())
-                                    }
-                                }
-                                if let Some(first) = first {
-                                    Ok(ContentItem::Attr(None, first))
-                                } else {
-                                    Err(())
-                                }
+                                Ok(ContentItem::Attr(Attr::parse_function(context, input)?))
                             }),
                         % endif
                         _ => return Err(())

components/style/servo/selector_parser.rs

@@ -430,11 +430,11 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
     }
 
     fn default_namespace(&self) -> Option<Namespace> {
-        self.namespaces.default.clone()
+        self.namespaces.default.as_ref().map(|&(ref ns, _)| ns.clone())
     }
 
     fn namespace_for_prefix(&self, prefix: &Prefix) -> Option<Namespace> {
-        self.namespaces.prefixes.get(prefix).cloned()
+        self.namespaces.prefixes.get(prefix).map(|&(ref ns, _)| ns.clone())
     }
 }
 

components/style/stylesheets.rs

@@ -38,9 +38,9 @@ use servo_config::prefs::PREFS;
 use servo_url::ServoUrl;
 use shared_lock::{SharedRwLock, Locked, ToCssWithGuard, SharedRwLockReadGuard};
 use smallvec::SmallVec;
+use std::{fmt, mem};
 use std::borrow::Borrow;
 use std::cell::Cell;
-use std::fmt;
 use std::mem::align_of;
 use std::os::raw::c_void;
 use std::slice;
@@ -51,6 +51,7 @@ 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;
 
@@ -96,11 +97,13 @@ pub enum Origin {
 }
 
 /// A set of namespaces applying to a given stylesheet.
+///
+/// The namespace id is 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, NamespaceId)>,
+    pub prefixes: FnvHashMap<Prefix, (Namespace, NamespaceId)>,
 }
 
 /// Like gecko_bindings::structs::MallocSizeOf, but without the Option<> wrapper. Note that
@@ -476,13 +479,13 @@ impl CssRule {
                  loader: Option<&StylesheetLoader>)
                  -> Result<(Self, State), SingleRuleParseError> {
         let error_reporter = NullReporter;
-        let mut namespaces = parent_stylesheet.namespaces.write();
-        let context = ParserContext::new(parent_stylesheet.origin,
-                                         &parent_stylesheet.url_data,
-                                         &error_reporter,
-                                         None,
-                                         PARSING_MODE_DEFAULT,
-                                         parent_stylesheet.quirks_mode);
+        let mut context = ParserContext::new(parent_stylesheet.origin,
+                                             &parent_stylesheet.url_data,
+                                             &error_reporter,
+                                             None,
+                                             PARSING_MODE_DEFAULT,
+                                             parent_stylesheet.quirks_mode);
+        context.namespaces = Some(&parent_stylesheet.namespaces);
         let mut input = Parser::new(css);
 
         // nested rules are in the body state
@@ -493,7 +496,6 @@ impl CssRule {
             shared_lock: &parent_stylesheet.shared_lock,
             loader: loader,
             state: Cell::new(state),
-            namespaces: &mut namespaces,
         };
         match parse_one_rule(&mut input, &mut rule_parser) {
             Ok(result) => Ok((result, rule_parser.state.get())),
@@ -1210,14 +1212,14 @@ impl Stylesheet {
                            stylesheet_loader: Option<&StylesheetLoader>,
                            error_reporter: &ParseErrorReporter,
                            line_number_offset: u64) {
-        let mut namespaces = Namespaces::default();
+        let namespaces = RwLock::new(Namespaces::default());
         // FIXME: we really should update existing.url_data with the given url_data,
         // otherwise newly inserted rule may not have the right base url.
         let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules(
-            css, url_data, existing.origin, &mut namespaces,
+            css, url_data, existing.origin, &namespaces,
             &existing.shared_lock, stylesheet_loader, error_reporter,
             existing.quirks_mode, line_number_offset);
-        *existing.namespaces.write() = namespaces;
+        mem::swap(&mut *existing.namespaces.write(), &mut *namespaces.write());
         existing.dirty_on_viewport_size_change
             .store(dirty_on_viewport_size_change, Ordering::Release);
 
@@ -1229,7 +1231,7 @@ impl Stylesheet {
     fn parse_rules(css: &str,
                    url_data: &UrlExtraData,
                    origin: Origin,
-                   namespaces: &mut Namespaces,
+                   namespaces: &RwLock<Namespaces>,
                    shared_lock: &SharedRwLock,
                    stylesheet_loader: Option<&StylesheetLoader>,
                    error_reporter: &ParseErrorReporter,
@@ -1238,14 +1240,16 @@ impl Stylesheet {
                    -> (Vec<CssRule>, bool) {
         let mut rules = Vec::new();
         let mut input = Parser::new(css);
+        let mut context = ParserContext::new_with_line_number_offset(origin, url_data, error_reporter,
+                                                                     line_number_offset,
+                                                                     PARSING_MODE_DEFAULT,
+                                                                     quirks_mode);
+        context.namespaces = Some(namespaces);
         let rule_parser = TopLevelRuleParser {
             stylesheet_origin: origin,
-            namespaces: namespaces,
             shared_lock: shared_lock,
             loader: stylesheet_loader,
-            context: ParserContext::new_with_line_number_offset(origin, url_data, error_reporter,
-                                                                line_number_offset, PARSING_MODE_DEFAULT,
-                                                                quirks_mode),
+            context: context,
             state: Cell::new(State::Start),
         };
 
@@ -1283,15 +1287,15 @@ impl Stylesheet {
                     quirks_mode: QuirksMode,
                     line_number_offset: u64)
                     -> Stylesheet {
-        let mut namespaces = Namespaces::default();
+        let namespaces = RwLock::new(Namespaces::default());
         let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules(
-            css, &url_data, origin, &mut namespaces,
+            css, &url_data, origin, &namespaces,
             &shared_lock, stylesheet_loader, error_reporter, quirks_mode, line_number_offset,
         );
         Stylesheet {
             origin: origin,
             url_data: url_data,
-            namespaces: RwLock::new(namespaces),
+            namespaces: namespaces,
             rules: CssRules::new(rules, &shared_lock),
             media: media,
             shared_lock: shared_lock,
@@ -1470,7 +1474,6 @@ impl StylesheetLoader for NoOpLoader {
 
 struct TopLevelRuleParser<'a> {
     stylesheet_origin: Origin,
-    namespaces: &'a mut Namespaces,
     shared_lock: &'a SharedRwLock,
     loader: Option<&'a StylesheetLoader>,
     context: ParserContext<'a>,
@@ -1483,7 +1486,6 @@ impl<'b> TopLevelRuleParser<'b> {
             stylesheet_origin: self.stylesheet_origin,
             shared_lock: self.shared_lock,
             context: &self.context,
-            namespaces: self.namespaces,
         }
     }
 }
@@ -1528,6 +1530,21 @@ enum AtRulePrelude {
 }
 
 
+#[cfg(feature = "gecko")]
+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)
+    }
+}
+
+#[cfg(feature = "servo")]
+fn register_namespace(_: &Namespace) -> Result<(), ()> {
+    Ok(()) // servo doesn't use namespace ids
+}
+
 impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
     type Prelude = AtRulePrelude;
     type AtRule = CssRule;
@@ -1586,12 +1603,16 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
                     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.namespaces.prefixes.insert(prefix.clone(), url.clone());
+                        self.context.namespaces.expect("namespaces must be set whilst parsing rules")
+                                               .write().prefixes.insert(prefix.clone(), (url.clone(), id));
                         Some(prefix)
                     } else {
-                        self.namespaces.default = Some(url.clone());
+                        self.context.namespaces.expect("namespaces must be set whilst parsing rules")
+                                               .write().default = Some((url.clone(), id));
                         None
                     };
 
@@ -1650,7 +1671,6 @@ struct NestedRuleParser<'a, 'b: 'a> {
     stylesheet_origin: Origin,
     shared_lock: &'a SharedRwLock,
     context: &'a ParserContext<'b>,
-    namespaces: &'b Namespaces,
 }
 
 impl<'a, 'b> NestedRuleParser<'a, 'b> {
@@ -1660,7 +1680,6 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
             stylesheet_origin: self.stylesheet_origin,
             shared_lock: self.shared_lock,
             context: &context,
-            namespaces: self.namespaces,
         };
         let mut iter = RuleListParser::new_for_nested_rule(input, nested_parser);
         let mut rules = Vec::new();
@@ -1836,9 +1855,10 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> {
     type QualifiedRule = CssRule;
 
     fn parse_prelude(&mut self, input: &mut Parser) -> Result<SelectorList<SelectorImpl>, ()> {
+        let ns = self.context.namespaces.expect("namespaces must be set when parsing rules").read();
         let selector_parser = SelectorParser {
             stylesheet_origin: self.stylesheet_origin,
-            namespaces: self.namespaces,
+            namespaces: &*ns,
         };
         SelectorList::parse(&selector_parser, input)
     }