Skip to content

Commit

Permalink
Make CSSRule always keep a pointer to its parent stylesheet
Browse files Browse the repository at this point in the history
even when the parentStylesheet IDL attribute returns null.
  • Loading branch information
SimonSapin committed Nov 28, 2016
1 parent 0714e22 commit f1d49d3
Show file tree
Hide file tree
Showing 11 changed files with 84 additions and 69 deletions.
9 changes: 5 additions & 4 deletions components/script/dom/cssfontfacerule.rs
Expand Up @@ -22,17 +22,18 @@ pub struct CSSFontFaceRule {
}

impl CSSFontFaceRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, fontfacerule: Arc<RwLock<FontFaceRule>>) -> CSSFontFaceRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, fontfacerule: Arc<RwLock<FontFaceRule>>)
-> CSSFontFaceRule {
CSSFontFaceRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
fontfacerule: fontfacerule,
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
fontfacerule: Arc<RwLock<FontFaceRule>>) -> Root<CSSFontFaceRule> {
reflect_dom_object(box CSSFontFaceRule::new_inherited(parent, fontfacerule),
reflect_dom_object(box CSSFontFaceRule::new_inherited(parent_stylesheet, fontfacerule),
window,
CSSFontFaceRuleBinding::Wrap)
}
Expand Down
14 changes: 6 additions & 8 deletions components/script/dom/cssgroupingrule.rs
Expand Up @@ -4,7 +4,6 @@

use dom::bindings::codegen::Bindings::CSSGroupingRuleBinding;
use dom::bindings::codegen::Bindings::CSSGroupingRuleBinding::CSSGroupingRuleMethods;
use dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleBinding::CSSRuleMethods;
use dom::bindings::error::{ErrorResult, Fallible};
use dom::bindings::inheritance::Castable;
use dom::bindings::js::{JS, MutNullableHeap, Root};
Expand All @@ -25,27 +24,26 @@ pub struct CSSGroupingRule {
}

impl CSSGroupingRule {
pub fn new_inherited(parent: Option<&CSSStyleSheet>,
pub fn new_inherited(parent_stylesheet: &CSSStyleSheet,
rules: StyleCssRules) -> CSSGroupingRule {
CSSGroupingRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
rules: rules,
rulelist: MutNullableHeap::new(None),
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>, rules: StyleCssRules) -> Root<CSSGroupingRule> {
reflect_dom_object(box CSSGroupingRule::new_inherited(parent, rules),
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, rules: StyleCssRules) -> Root<CSSGroupingRule> {
reflect_dom_object(box CSSGroupingRule::new_inherited(parent_stylesheet, rules),
window,
CSSGroupingRuleBinding::Wrap)
}

fn rulelist(&self) -> Root<CSSRuleList> {
let sheet = self.upcast::<CSSRule>().GetParentStyleSheet();
let sheet = sheet.as_ref().map(|s| &**s);
let parent_stylesheet = self.upcast::<CSSRule>().parent_stylesheet();
self.rulelist.or_init(|| CSSRuleList::new(self.global().as_window(),
sheet,
parent_stylesheet,
RulesSource::Rules(self.rules.clone())))
}
}
Expand Down
9 changes: 5 additions & 4 deletions components/script/dom/csskeyframerule.rs
Expand Up @@ -22,17 +22,18 @@ pub struct CSSKeyframeRule {
}

impl CSSKeyframeRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, keyframerule: Arc<RwLock<Keyframe>>) -> CSSKeyframeRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframerule: Arc<RwLock<Keyframe>>)
-> CSSKeyframeRule {
CSSKeyframeRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
keyframerule: keyframerule,
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
keyframerule: Arc<RwLock<Keyframe>>) -> Root<CSSKeyframeRule> {
reflect_dom_object(box CSSKeyframeRule::new_inherited(parent, keyframerule),
reflect_dom_object(box CSSKeyframeRule::new_inherited(parent_stylesheet, keyframerule),
window,
CSSKeyframeRuleBinding::Wrap)
}
Expand Down
15 changes: 7 additions & 8 deletions components/script/dom/csskeyframesrule.rs
Expand Up @@ -5,7 +5,6 @@
use cssparser::Parser;
use dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding;
use dom::bindings::codegen::Bindings::CSSKeyframesRuleBinding::CSSKeyframesRuleMethods;
use dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleMethods;
use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
use dom::bindings::inheritance::Castable;
use dom::bindings::js::{JS, MutNullableHeap, Root};
Expand All @@ -32,28 +31,28 @@ pub struct CSSKeyframesRule {
}

impl CSSKeyframesRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, keyframesrule: Arc<RwLock<KeyframesRule>>) -> CSSKeyframesRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframesrule: Arc<RwLock<KeyframesRule>>)
-> CSSKeyframesRule {
CSSKeyframesRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
keyframesrule: keyframesrule,
rulelist: MutNullableHeap::new(None),
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
keyframesrule: Arc<RwLock<KeyframesRule>>) -> Root<CSSKeyframesRule> {
reflect_dom_object(box CSSKeyframesRule::new_inherited(parent, keyframesrule),
reflect_dom_object(box CSSKeyframesRule::new_inherited(parent_stylesheet, keyframesrule),
window,
CSSKeyframesRuleBinding::Wrap)
}

fn rulelist(&self) -> Root<CSSRuleList> {
self.rulelist.or_init(|| {
let sheet = self.upcast::<CSSRule>().GetParentStyleSheet();
let sheet = sheet.as_ref().map(|s| &**s);
let parent_stylesheet = &self.upcast::<CSSRule>().parent_stylesheet();
CSSRuleList::new(self.global().as_window(),
sheet,
parent_stylesheet,
RulesSource::Keyframes(self.keyframesrule.clone()))
})
}
Expand Down
9 changes: 5 additions & 4 deletions components/script/dom/cssmediarule.rs
Expand Up @@ -23,18 +23,19 @@ pub struct CSSMediaRule {
}

impl CSSMediaRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, mediarule: Arc<RwLock<MediaRule>>) -> CSSMediaRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, mediarule: Arc<RwLock<MediaRule>>)
-> CSSMediaRule {
let list = mediarule.read().rules.clone();
CSSMediaRule {
cssrule: CSSGroupingRule::new_inherited(parent, list),
cssrule: CSSGroupingRule::new_inherited(parent_stylesheet, list),
mediarule: mediarule,
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
mediarule: Arc<RwLock<MediaRule>>) -> Root<CSSMediaRule> {
reflect_dom_object(box CSSMediaRule::new_inherited(parent, mediarule),
reflect_dom_object(box CSSMediaRule::new_inherited(parent_stylesheet, mediarule),
window,
CSSMediaRuleBinding::Wrap)
}
Expand Down
9 changes: 5 additions & 4 deletions components/script/dom/cssnamespacerule.rs
Expand Up @@ -23,17 +23,18 @@ pub struct CSSNamespaceRule {
}

impl CSSNamespaceRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, namespacerule: Arc<RwLock<NamespaceRule>>) -> CSSNamespaceRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, namespacerule: Arc<RwLock<NamespaceRule>>)
-> CSSNamespaceRule {
CSSNamespaceRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
namespacerule: namespacerule,
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
namespacerule: Arc<RwLock<NamespaceRule>>) -> Root<CSSNamespaceRule> {
reflect_dom_object(box CSSNamespaceRule::new_inherited(parent, namespacerule),
reflect_dom_object(box CSSNamespaceRule::new_inherited(parent_stylesheet, namespacerule),
window,
CSSNamespaceRuleBinding::Wrap)
}
Expand Down
47 changes: 31 additions & 16 deletions components/script/dom/cssrule.rs
Expand Up @@ -5,7 +5,7 @@
use dom::bindings::codegen::Bindings::CSSRuleBinding;
use dom::bindings::codegen::Bindings::CSSRuleBinding::CSSRuleMethods;
use dom::bindings::inheritance::Castable;
use dom::bindings::js::{JS, MutNullableHeap, Root};
use dom::bindings::js::{JS, Root};
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::bindings::str::DOMString;
use dom::cssfontfacerule::CSSFontFaceRule;
Expand All @@ -17,27 +17,34 @@ use dom::cssstylerule::CSSStyleRule;
use dom::cssstylesheet::CSSStyleSheet;
use dom::cssviewportrule::CSSViewportRule;
use dom::window::Window;
use std::cell::Cell;
use style::stylesheets::CssRule as StyleCssRule;


#[dom_struct]
pub struct CSSRule {
reflector_: Reflector,
parent: MutNullableHeap<JS<CSSStyleSheet>>,
parent_stylesheet: JS<CSSStyleSheet>,

/// Whether the parentStyleSheet attribute should return null.
/// We keep parent_stylesheet in that case because insertRule needs it
/// for the stylesheet’s base URL and namespace prefixes.
parent_stylesheet_removed: Cell<bool>,
}

impl CSSRule {
#[allow(unrooted_must_root)]
pub fn new_inherited(parent: Option<&CSSStyleSheet>) -> CSSRule {
pub fn new_inherited(parent_stylesheet: &CSSStyleSheet) -> CSSRule {
CSSRule {
reflector_: Reflector::new(),
parent: MutNullableHeap::new(parent),
parent_stylesheet: JS::from_ref(parent_stylesheet),
parent_stylesheet_removed: Cell::new(false),
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>) -> Root<CSSRule> {
reflect_dom_object(box CSSRule::new_inherited(parent),
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet) -> Root<CSSRule> {
reflect_dom_object(box CSSRule::new_inherited(parent_stylesheet),
window,
CSSRuleBinding::Wrap)
}
Expand All @@ -64,16 +71,16 @@ impl CSSRule {

// Given a StyleCssRule, create a new instance of a derived class of
// CSSRule based on which rule it is
pub fn new_specific(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new_specific(window: &Window, parent_stylesheet: &CSSStyleSheet,
rule: StyleCssRule) -> Root<CSSRule> {
// be sure to update the match in as_specific when this is updated
match rule {
StyleCssRule::Style(s) => Root::upcast(CSSStyleRule::new(window, parent, s)),
StyleCssRule::FontFace(s) => Root::upcast(CSSFontFaceRule::new(window, parent, s)),
StyleCssRule::Keyframes(s) => Root::upcast(CSSKeyframesRule::new(window, parent, s)),
StyleCssRule::Media(s) => Root::upcast(CSSMediaRule::new(window, parent, s)),
StyleCssRule::Namespace(s) => Root::upcast(CSSNamespaceRule::new(window, parent, s)),
StyleCssRule::Viewport(s) => Root::upcast(CSSViewportRule::new(window, parent, s)),
StyleCssRule::Style(s) => Root::upcast(CSSStyleRule::new(window, parent_stylesheet, s)),
StyleCssRule::FontFace(s) => Root::upcast(CSSFontFaceRule::new(window, parent_stylesheet, s)),
StyleCssRule::Keyframes(s) => Root::upcast(CSSKeyframesRule::new(window, parent_stylesheet, s)),
StyleCssRule::Media(s) => Root::upcast(CSSMediaRule::new(window, parent_stylesheet, s)),
StyleCssRule::Namespace(s) => Root::upcast(CSSNamespaceRule::new(window, parent_stylesheet, s)),
StyleCssRule::Viewport(s) => Root::upcast(CSSViewportRule::new(window, parent_stylesheet, s)),
}
}

Expand All @@ -85,12 +92,16 @@ impl CSSRule {

/// Sets owner sheet to null (and does the same for all children)
pub fn deparent(&self) {
self.parent.set(None);
self.parent_stylesheet_removed.set(true);
// https://github.com/w3c/csswg-drafts/issues/722
// Spec doesn't ask us to do this, but it makes sense
// and browsers implement this behavior
self.as_specific().deparent_children();
}

pub fn parent_stylesheet(&self) -> &CSSStyleSheet {
&self.parent_stylesheet
}
}

impl CSSRuleMethods for CSSRule {
Expand All @@ -101,7 +112,11 @@ impl CSSRuleMethods for CSSRule {

// https://drafts.csswg.org/cssom/#dom-cssrule-parentstylesheet
fn GetParentStyleSheet(&self) -> Option<Root<CSSStyleSheet>> {
self.parent.get()
if self.parent_stylesheet_removed.get() {
None
} else {
Some(Root::from_ref(&*self.parent_stylesheet))
}
}

// https://drafts.csswg.org/cssom/#dom-cssrule-csstext
Expand All @@ -118,7 +133,7 @@ impl CSSRuleMethods for CSSRule {
pub trait SpecificCSSRule {
fn ty(&self) -> u16;
fn get_css(&self) -> DOMString;
/// Remove CSSStyleSheet parent from all transitive children
/// Remove parentStylesheet from all transitive children
fn deparent_children(&self) {
// most CSSRules do nothing here
}
Expand Down
22 changes: 10 additions & 12 deletions components/script/dom/cssrulelist.rs
Expand Up @@ -34,7 +34,7 @@ impl From<RulesMutateError> for Error {
#[dom_struct]
pub struct CSSRuleList {
reflector_: Reflector,
sheet: MutNullableHeap<JS<CSSStyleSheet>>,
parent_stylesheet: JS<CSSStyleSheet>,
#[ignore_heap_size_of = "Arc"]
rules: RulesSource,
dom_rules: DOMRefCell<Vec<MutNullableHeap<JS<CSSRule>>>>
Expand All @@ -47,7 +47,7 @@ pub enum RulesSource {

impl CSSRuleList {
#[allow(unrooted_must_root)]
pub fn new_inherited(sheet: Option<&CSSStyleSheet>, rules: RulesSource) -> CSSRuleList {
pub fn new_inherited(parent_stylesheet: &CSSStyleSheet, rules: RulesSource) -> CSSRuleList {
let dom_rules = match rules {
RulesSource::Rules(ref rules) => {
rules.0.read().iter().map(|_| MutNullableHeap::new(None)).collect()
Expand All @@ -59,16 +59,16 @@ impl CSSRuleList {

CSSRuleList {
reflector_: Reflector::new(),
sheet: MutNullableHeap::new(sheet),
parent_stylesheet: JS::from_ref(parent_stylesheet),
rules: rules,
dom_rules: DOMRefCell::new(dom_rules),
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, sheet: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
rules: RulesSource) -> Root<CSSRuleList> {
reflect_dom_object(box CSSRuleList::new_inherited(sheet, rules),
reflect_dom_object(box CSSRuleList::new_inherited(parent_stylesheet, rules),
window,
CSSRuleListBinding::Wrap)
}
Expand All @@ -89,9 +89,8 @@ impl CSSRuleList {

let new_rule = css_rules.insert_rule(rule, doc.url().clone(), index, nested)?;

let sheet = self.sheet.get();
let sheet = sheet.as_ref().map(|sheet| &**sheet);
let dom_rule = CSSRule::new_specific(&window, sheet, new_rule);
let parent_stylesheet = &*self.parent_stylesheet;
let dom_rule = CSSRule::new_specific(&window, parent_stylesheet, new_rule);
self.dom_rules.borrow_mut().insert(index, MutNullableHeap::new(Some(&*dom_rule)));
Ok((idx))
}
Expand Down Expand Up @@ -129,17 +128,16 @@ impl CSSRuleList {
pub fn item(&self, idx: u32) -> Option<Root<CSSRule>> {
self.dom_rules.borrow().get(idx as usize).map(|rule| {
rule.or_init(|| {
let sheet = self.sheet.get();
let sheet = sheet.as_ref().map(|sheet| &**sheet);
let parent_stylesheet = &self.parent_stylesheet;
match self.rules {
RulesSource::Rules(ref rules) => {
CSSRule::new_specific(self.global().as_window(),
sheet,
parent_stylesheet,
rules.0.read()[idx as usize].clone())
}
RulesSource::Keyframes(ref rules) => {
Root::upcast(CSSKeyframeRule::new(self.global().as_window(),
sheet,
parent_stylesheet,
rules.read()
.keyframes[idx as usize]
.clone()))
Expand Down
9 changes: 5 additions & 4 deletions components/script/dom/cssstylerule.rs
Expand Up @@ -22,17 +22,18 @@ pub struct CSSStyleRule {
}

impl CSSStyleRule {
fn new_inherited(parent: Option<&CSSStyleSheet>, stylerule: Arc<RwLock<StyleRule>>) -> CSSStyleRule {
fn new_inherited(parent_stylesheet: &CSSStyleSheet, stylerule: Arc<RwLock<StyleRule>>)
-> CSSStyleRule {
CSSStyleRule {
cssrule: CSSRule::new_inherited(parent),
cssrule: CSSRule::new_inherited(parent_stylesheet),
stylerule: stylerule,
}
}

#[allow(unrooted_must_root)]
pub fn new(window: &Window, parent: Option<&CSSStyleSheet>,
pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet,
stylerule: Arc<RwLock<StyleRule>>) -> Root<CSSStyleRule> {
reflect_dom_object(box CSSStyleRule::new_inherited(parent, stylerule),
reflect_dom_object(box CSSStyleRule::new_inherited(parent_stylesheet, stylerule),
window,
CSSStyleRuleBinding::Wrap)
}
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/cssstylesheet.rs
Expand Up @@ -53,7 +53,7 @@ impl CSSStyleSheet {

fn rulelist(&self) -> Root<CSSRuleList> {
self.rulelist.or_init(|| CSSRuleList::new(self.global().as_window(),
Some(self),
self,
RulesSource::Rules(self.style_stylesheet
.rules.clone())))
}
Expand Down

0 comments on commit f1d49d3

Please sign in to comment.