Auto merge of #14355 - upsuper:rulelist-mutate, r=Manishearth

Move algorithm for insertRule and deleteRule to style component

<!-- Please describe your changes on the following line: -->

r? @Manishearth

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/14355)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-11-27 15:40:19 -08:00 committed by GitHub
commit b18ec28fa7
2 changed files with 103 additions and 112 deletions

View file

@ -15,13 +15,22 @@ use dom::cssstylesheet::CSSStyleSheet;
use dom::window::Window;
use parking_lot::RwLock;
use std::sync::Arc;
use style::parser::ParserContextExtraData;
use style::stylesheets::{CssRules, KeyframesRule, Origin};
use style::stylesheets::CssRule as StyleCssRule;
use style::stylesheets::{CssRules, KeyframesRule, RulesMutateError};
no_jsmanaged_fields!(RulesSource);
no_jsmanaged_fields!(CssRules);
impl From<RulesMutateError> for Error {
fn from(other: RulesMutateError) -> Self {
match other {
RulesMutateError::Syntax => Error::Syntax,
RulesMutateError::IndexSize => Error::IndexSize,
RulesMutateError::HierarchyRequest => Error::HierarchyRequest,
RulesMutateError::InvalidState => Error::InvalidState,
}
}
}
#[dom_struct]
pub struct CSSRuleList {
reflector_: Reflector,
@ -64,21 +73,9 @@ impl CSSRuleList {
CSSRuleListBinding::Wrap)
}
/// https://drafts.csswg.org/cssom/#insert-a-css-rule
///
/// Should only be called for CssRules-backed rules. Use append_lazy_rule
/// for keyframes-backed rules.
pub fn insert_rule(&self, rule: &str, idx: u32, nested: bool) -> Fallible<u32> {
use style::stylesheets::SingleRuleParseError;
/// Insert an item into a vector, appending if it is out of bounds
fn insert<T>(vec: &mut Vec<T>, index: usize, item: T) {
if index >= vec.len() {
vec.push(item);
} else {
vec.insert(index, item);
}
}
let css_rules = if let RulesSource::Rules(ref rules) = self.rules {
rules
} else {
@ -90,92 +87,23 @@ impl CSSRuleList {
let doc = window.Document();
let index = idx as usize;
let new_rule = css_rules.insert_rule(rule, doc.url().clone(), index, nested)?;
let new_rule = {
let rules = css_rules.0.read();
let state = if nested {
None
} else {
Some(CssRules::state_at_index(&rules, index))
};
let rev_state = CssRules::state_at_index_rev(&rules, index);
// Step 1, 2
// XXXManishearth get url from correct location
// XXXManishearth should we also store the namespace map?
let parse_result = StyleCssRule::parse(&rule, Origin::Author,
doc.url().clone(),
ParserContextExtraData::default(),
state);
if let Err(SingleRuleParseError::Syntax) = parse_result {
return Err(Error::Syntax)
}
// Step 3, 4
if index > rules.len() {
return Err(Error::IndexSize);
}
let (new_rule, new_state) = try!(parse_result.map_err(|_| Error::HierarchyRequest));
if new_state > rev_state {
// We inserted a rule too early, e.g. inserting
// a regular style rule before @namespace rules
return Err((Error::HierarchyRequest));
}
// Step 6
if let StyleCssRule::Namespace(..) = new_rule {
if !CssRules::only_ns_or_import(&rules) {
return Err(Error::InvalidState);
}
}
new_rule
};
insert(&mut css_rules.0.write(), index, new_rule.clone());
let sheet = self.sheet.get();
let sheet = sheet.as_ref().map(|sheet| &**sheet);
let dom_rule = CSSRule::new_specific(&window, sheet, new_rule);
insert(&mut self.dom_rules.borrow_mut(),
index, MutNullableHeap::new(Some(&*dom_rule)));
self.dom_rules.borrow_mut().insert(index, MutNullableHeap::new(Some(&*dom_rule)));
Ok((idx))
}
// https://drafts.csswg.org/cssom/#remove-a-css-rule
// https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-deleterule
// In case of a keyframe rule, index must be valid.
pub fn remove_rule(&self, index: u32) -> ErrorResult {
let index = index as usize;
match self.rules {
RulesSource::Rules(ref css_rules) => {
// https://drafts.csswg.org/cssom/#remove-a-css-rule
{
let rules = css_rules.0.read();
// Step 1, 2
if index >= rules.len() {
return Err(Error::IndexSize);
}
// Step 3
let ref rule = rules[index];
// Step 4
if let StyleCssRule::Namespace(..) = *rule {
if !CssRules::only_ns_or_import(&rules) {
return Err(Error::InvalidState);
}
}
}
// Step 5, 6
css_rules.remove_rule(index)?;
let mut dom_rules = self.dom_rules.borrow_mut();
css_rules.0.write().remove(index);
dom_rules[index].get().map(|r| r.detach());
dom_rules.remove(index);
Ok(())

View file

@ -60,6 +60,22 @@ impl From<Vec<CssRule>> for CssRules {
}
}
pub enum RulesMutateError {
Syntax,
IndexSize,
HierarchyRequest,
InvalidState,
}
impl From<SingleRuleParseError> for RulesMutateError {
fn from(other: SingleRuleParseError) -> Self {
match other {
SingleRuleParseError::Syntax => RulesMutateError::Syntax,
SingleRuleParseError::Hierarchy => RulesMutateError::HierarchyRequest,
}
}
}
impl CssRules {
// used in CSSOM
pub fn only_ns_or_import(rules: &[CssRule]) -> bool {
@ -71,36 +87,74 @@ impl CssRules {
})
}
// Provides the parser state at a given insertion index
pub fn state_at_index(rules: &[CssRule], at: usize) -> State {
let mut state = State::Start;
if at > 0 {
if let Some(rule) = rules.get(at - 1) {
state = match *rule {
// CssRule::Charset(..) => State::Start,
// CssRule::Import(..) => State::Imports,
CssRule::Namespace(..) => State::Namespaces,
_ => State::Body,
};
// https://drafts.csswg.org/cssom/#insert-a-css-rule
pub fn insert_rule(&self, rule: &str, base_url: ServoUrl, index: usize, nested: bool)
-> Result<CssRule, RulesMutateError> {
let mut rules = self.0.write();
// Step 1, 2
if index > rules.len() {
return Err(RulesMutateError::IndexSize);
}
// Computes the parser state at the given index
let state = if nested {
None
} else if index == 0 {
Some(State::Start)
} else {
rules.get(index - 1).map(CssRule::rule_state)
};
// Step 3, 4
// XXXManishearth should we also store the namespace map?
let (new_rule, new_state) = try!(CssRule::parse(&rule, Origin::Author, base_url,
ParserContextExtraData::default(), state));
// Step 5
// Computes the maximum allowed parser state at a given index.
let rev_state = rules.get(index).map_or(State::Body, CssRule::rule_state);
if new_state > rev_state {
// We inserted a rule too early, e.g. inserting
// a regular style rule before @namespace rules
return Err(RulesMutateError::HierarchyRequest);
}
// Step 6
if let CssRule::Namespace(..) = new_rule {
if !CssRules::only_ns_or_import(&rules) {
return Err(RulesMutateError::InvalidState);
}
}
state
rules.insert(index, new_rule.clone());
Ok(new_rule)
}
// Provides the maximum allowed parser state at a given index,
// searching in reverse. If inserting at this index, the parser
// must not be in a state greater than this post-insertion.
pub fn state_at_index_rev(rules: &[CssRule], at: usize) -> State {
if let Some(rule) = rules.get(at) {
match *rule {
// CssRule::Charset(..) => State::Start,
// CssRule::Import(..) => State::Imports,
CssRule::Namespace(..) => State::Namespaces,
_ => State::Body,
}
} else {
State::Body
// https://drafts.csswg.org/cssom/#remove-a-css-rule
pub fn remove_rule(&self, index: usize) -> Result<(), RulesMutateError> {
let mut rules = self.0.write();
// Step 1, 2
if index >= rules.len() {
return Err(RulesMutateError::IndexSize);
}
{
// Step 3
let ref rule = rules[index];
// Step 4
if let CssRule::Namespace(..) = *rule {
if !CssRules::only_ns_or_import(&rules) {
return Err(RulesMutateError::InvalidState);
}
}
}
// Step 5, 6
rules.remove(index);
Ok(())
}
}
@ -193,6 +247,15 @@ impl CssRule {
}
}
fn rule_state(&self) -> State {
match *self {
// CssRule::Charset(..) => State::Start,
// CssRule::Import(..) => State::Imports,
CssRule::Namespace(..) => State::Namespaces,
_ => State::Body,
}
}
/// Call `f` with the slice of rules directly contained inside this rule.
///
/// Note that only some types of rules can contain rules. An empty slice is used for others.