From f1d49d3773846a65cb403af4e041660fad3117be Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 28 Nov 2016 16:33:47 +0100 Subject: [PATCH] Make CSSRule always keep a pointer to its parent stylesheet even when the parentStylesheet IDL attribute returns null. --- components/script/dom/cssfontfacerule.rs | 9 +++-- components/script/dom/cssgroupingrule.rs | 14 +++---- components/script/dom/csskeyframerule.rs | 9 +++-- components/script/dom/csskeyframesrule.rs | 15 ++++---- components/script/dom/cssmediarule.rs | 9 +++-- components/script/dom/cssnamespacerule.rs | 9 +++-- components/script/dom/cssrule.rs | 47 +++++++++++++++-------- components/script/dom/cssrulelist.rs | 22 +++++------ components/script/dom/cssstylerule.rs | 9 +++-- components/script/dom/cssstylesheet.rs | 2 +- components/script/dom/cssviewportrule.rs | 8 ++-- 11 files changed, 84 insertions(+), 69 deletions(-) diff --git a/components/script/dom/cssfontfacerule.rs b/components/script/dom/cssfontfacerule.rs index 60022b7ba8f8..61234f8846aa 100644 --- a/components/script/dom/cssfontfacerule.rs +++ b/components/script/dom/cssfontfacerule.rs @@ -22,17 +22,18 @@ pub struct CSSFontFaceRule { } impl CSSFontFaceRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, fontfacerule: Arc>) -> CSSFontFaceRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, fontfacerule: Arc>) + -> 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>) -> Root { - reflect_dom_object(box CSSFontFaceRule::new_inherited(parent, fontfacerule), + reflect_dom_object(box CSSFontFaceRule::new_inherited(parent_stylesheet, fontfacerule), window, CSSFontFaceRuleBinding::Wrap) } diff --git a/components/script/dom/cssgroupingrule.rs b/components/script/dom/cssgroupingrule.rs index cd55cd358b66..aa381dcbaec4 100644 --- a/components/script/dom/cssgroupingrule.rs +++ b/components/script/dom/cssgroupingrule.rs @@ -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}; @@ -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 { - reflect_dom_object(box CSSGroupingRule::new_inherited(parent, rules), + pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, rules: StyleCssRules) -> Root { + reflect_dom_object(box CSSGroupingRule::new_inherited(parent_stylesheet, rules), window, CSSGroupingRuleBinding::Wrap) } fn rulelist(&self) -> Root { - let sheet = self.upcast::().GetParentStyleSheet(); - let sheet = sheet.as_ref().map(|s| &**s); + let parent_stylesheet = self.upcast::().parent_stylesheet(); self.rulelist.or_init(|| CSSRuleList::new(self.global().as_window(), - sheet, + parent_stylesheet, RulesSource::Rules(self.rules.clone()))) } } diff --git a/components/script/dom/csskeyframerule.rs b/components/script/dom/csskeyframerule.rs index e09099e1135a..78916d339abb 100644 --- a/components/script/dom/csskeyframerule.rs +++ b/components/script/dom/csskeyframerule.rs @@ -22,17 +22,18 @@ pub struct CSSKeyframeRule { } impl CSSKeyframeRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, keyframerule: Arc>) -> CSSKeyframeRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframerule: Arc>) + -> 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>) -> Root { - reflect_dom_object(box CSSKeyframeRule::new_inherited(parent, keyframerule), + reflect_dom_object(box CSSKeyframeRule::new_inherited(parent_stylesheet, keyframerule), window, CSSKeyframeRuleBinding::Wrap) } diff --git a/components/script/dom/csskeyframesrule.rs b/components/script/dom/csskeyframesrule.rs index b7baf6b85a92..bda3ee2ea204 100644 --- a/components/script/dom/csskeyframesrule.rs +++ b/components/script/dom/csskeyframesrule.rs @@ -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}; @@ -32,28 +31,28 @@ pub struct CSSKeyframesRule { } impl CSSKeyframesRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, keyframesrule: Arc>) -> CSSKeyframesRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, keyframesrule: Arc>) + -> 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>) -> Root { - 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 { self.rulelist.or_init(|| { - let sheet = self.upcast::().GetParentStyleSheet(); - let sheet = sheet.as_ref().map(|s| &**s); + let parent_stylesheet = &self.upcast::().parent_stylesheet(); CSSRuleList::new(self.global().as_window(), - sheet, + parent_stylesheet, RulesSource::Keyframes(self.keyframesrule.clone())) }) } diff --git a/components/script/dom/cssmediarule.rs b/components/script/dom/cssmediarule.rs index 67933fccd3b7..e42c1bc0021b 100644 --- a/components/script/dom/cssmediarule.rs +++ b/components/script/dom/cssmediarule.rs @@ -23,18 +23,19 @@ pub struct CSSMediaRule { } impl CSSMediaRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, mediarule: Arc>) -> CSSMediaRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, mediarule: Arc>) + -> 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>) -> Root { - reflect_dom_object(box CSSMediaRule::new_inherited(parent, mediarule), + reflect_dom_object(box CSSMediaRule::new_inherited(parent_stylesheet, mediarule), window, CSSMediaRuleBinding::Wrap) } diff --git a/components/script/dom/cssnamespacerule.rs b/components/script/dom/cssnamespacerule.rs index 4e93eea2e1f7..5f451d3b798c 100644 --- a/components/script/dom/cssnamespacerule.rs +++ b/components/script/dom/cssnamespacerule.rs @@ -23,17 +23,18 @@ pub struct CSSNamespaceRule { } impl CSSNamespaceRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, namespacerule: Arc>) -> CSSNamespaceRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, namespacerule: Arc>) + -> 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>) -> Root { - reflect_dom_object(box CSSNamespaceRule::new_inherited(parent, namespacerule), + reflect_dom_object(box CSSNamespaceRule::new_inherited(parent_stylesheet, namespacerule), window, CSSNamespaceRuleBinding::Wrap) } diff --git a/components/script/dom/cssrule.rs b/components/script/dom/cssrule.rs index ca1e52b77f0f..603354c86212 100644 --- a/components/script/dom/cssrule.rs +++ b/components/script/dom/cssrule.rs @@ -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; @@ -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>, + parent_stylesheet: JS, + + /// 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, } 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 { - reflect_dom_object(box CSSRule::new_inherited(parent), + pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet) -> Root { + reflect_dom_object(box CSSRule::new_inherited(parent_stylesheet), window, CSSRuleBinding::Wrap) } @@ -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 { // 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)), } } @@ -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 { @@ -101,7 +112,11 @@ impl CSSRuleMethods for CSSRule { // https://drafts.csswg.org/cssom/#dom-cssrule-parentstylesheet fn GetParentStyleSheet(&self) -> Option> { - 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 @@ -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 } diff --git a/components/script/dom/cssrulelist.rs b/components/script/dom/cssrulelist.rs index 0f2212319dc5..643cce9a9442 100644 --- a/components/script/dom/cssrulelist.rs +++ b/components/script/dom/cssrulelist.rs @@ -34,7 +34,7 @@ impl From for Error { #[dom_struct] pub struct CSSRuleList { reflector_: Reflector, - sheet: MutNullableHeap>, + parent_stylesheet: JS, #[ignore_heap_size_of = "Arc"] rules: RulesSource, dom_rules: DOMRefCell>>> @@ -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() @@ -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 { - reflect_dom_object(box CSSRuleList::new_inherited(sheet, rules), + reflect_dom_object(box CSSRuleList::new_inherited(parent_stylesheet, rules), window, CSSRuleListBinding::Wrap) } @@ -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)) } @@ -129,17 +128,16 @@ impl CSSRuleList { pub fn item(&self, idx: u32) -> Option> { 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())) diff --git a/components/script/dom/cssstylerule.rs b/components/script/dom/cssstylerule.rs index 7d7766551473..091704f42d5b 100644 --- a/components/script/dom/cssstylerule.rs +++ b/components/script/dom/cssstylerule.rs @@ -22,17 +22,18 @@ pub struct CSSStyleRule { } impl CSSStyleRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, stylerule: Arc>) -> CSSStyleRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, stylerule: Arc>) + -> 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>) -> Root { - reflect_dom_object(box CSSStyleRule::new_inherited(parent, stylerule), + reflect_dom_object(box CSSStyleRule::new_inherited(parent_stylesheet, stylerule), window, CSSStyleRuleBinding::Wrap) } diff --git a/components/script/dom/cssstylesheet.rs b/components/script/dom/cssstylesheet.rs index 32413ea02989..2e643f3e8a20 100644 --- a/components/script/dom/cssstylesheet.rs +++ b/components/script/dom/cssstylesheet.rs @@ -53,7 +53,7 @@ impl CSSStyleSheet { fn rulelist(&self) -> Root { self.rulelist.or_init(|| CSSRuleList::new(self.global().as_window(), - Some(self), + self, RulesSource::Rules(self.style_stylesheet .rules.clone()))) } diff --git a/components/script/dom/cssviewportrule.rs b/components/script/dom/cssviewportrule.rs index b324b0c64f5e..25ca1b292c88 100644 --- a/components/script/dom/cssviewportrule.rs +++ b/components/script/dom/cssviewportrule.rs @@ -22,17 +22,17 @@ pub struct CSSViewportRule { } impl CSSViewportRule { - fn new_inherited(parent: Option<&CSSStyleSheet>, viewportrule: Arc>) -> CSSViewportRule { + fn new_inherited(parent_stylesheet: &CSSStyleSheet, viewportrule: Arc>) -> CSSViewportRule { CSSViewportRule { - cssrule: CSSRule::new_inherited(parent), + cssrule: CSSRule::new_inherited(parent_stylesheet), viewportrule: viewportrule, } } #[allow(unrooted_must_root)] - pub fn new(window: &Window, parent: Option<&CSSStyleSheet>, + pub fn new(window: &Window, parent_stylesheet: &CSSStyleSheet, viewportrule: Arc>) -> Root { - reflect_dom_object(box CSSViewportRule::new_inherited(parent, viewportrule), + reflect_dom_object(box CSSViewportRule::new_inherited(parent_stylesheet, viewportrule), window, CSSViewportRuleBinding::Wrap) }