Turn CSSStyleRule into a CSSGroupingRule subclass (#36254)

Note that `StyleRule` may not have the `CssRules` readily available,
they may need to be created. So the previous approach of providing
`CSSGroupingRule` with the `CssRules` is no good: it would require
writing them in advance, just in case they end up being used.

Therefore, this removes the `CSSGroupingRule::rules` field. Instead,
they are lazily obtained in `CSSGroupingRule::rulelist()` by downcasting
and calling the appropriate method for the subclass.

Testing: covered by WPT
Fixes: #36245

Signed-off-by: Oriol Brufau <obrufau@igalia.com>

components/script/dom/cssconditionrule.rs

@@ -18,6 +18,9 @@ use crate::dom::csssupportsrule::CSSSupportsRule;
 #[dom_struct]
 pub(crate) struct CSSConditionRule {
     cssgroupingrule: CSSGroupingRule,
+    #[ignore_malloc_size_of = "Arc"]
+    #[no_trace]
+    rules: Arc<Locked<StyleCssRules>>,
 }
 
 impl CSSConditionRule {
@@ -26,7 +29,8 @@ impl CSSConditionRule {
         rules: Arc<Locked<StyleCssRules>>,
     ) -> CSSConditionRule {
         CSSConditionRule {
-            cssgroupingrule: CSSGroupingRule::new_inherited(parent_stylesheet, rules),
+            cssgroupingrule: CSSGroupingRule::new_inherited(parent_stylesheet),
+            rules,
         }
     }
 
@@ -37,6 +41,10 @@ impl CSSConditionRule {
     pub(crate) fn shared_lock(&self) -> &SharedRwLock {
         self.cssgroupingrule.shared_lock()
     }
+
+    pub(crate) fn clone_rules(&self) -> Arc<Locked<StyleCssRules>> {
+        self.rules.clone()
+    }
 }
 
 impl CSSConditionRuleMethods<crate::DomTypeHolder> for CSSConditionRule {

components/script/dom/cssgroupingrule.rs

@@ -3,9 +3,8 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use dom_struct::dom_struct;
-use servo_arc::Arc;
+use style::shared_lock::SharedRwLock;
-use style::shared_lock::{Locked, SharedRwLock};
+use style::stylesheets::{CssRuleType, CssRuleTypes};
-use style::stylesheets::{CssRuleType, CssRuleTypes, CssRules as StyleCssRules};
 
 use crate::dom::bindings::codegen::Bindings::CSSGroupingRuleBinding::CSSGroupingRuleMethods;
 use crate::dom::bindings::error::{ErrorResult, Fallible};
@@ -13,28 +12,24 @@ use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::DomGlobal;
 use crate::dom::bindings::root::{DomRoot, MutNullableDom};
 use crate::dom::bindings::str::DOMString;
+use crate::dom::cssconditionrule::CSSConditionRule;
+use crate::dom::csslayerblockrule::CSSLayerBlockRule;
 use crate::dom::cssrule::CSSRule;
 use crate::dom::cssrulelist::{CSSRuleList, RulesSource};
+use crate::dom::cssstylerule::CSSStyleRule;
 use crate::dom::cssstylesheet::CSSStyleSheet;
 use crate::script_runtime::CanGc;
 
 #[dom_struct]
 pub(crate) struct CSSGroupingRule {
     cssrule: CSSRule,
-    #[ignore_malloc_size_of = "Arc"]
-    #[no_trace]
-    rules: Arc<Locked<StyleCssRules>>,
     rulelist: MutNullableDom<CSSRuleList>,
 }
 
 impl CSSGroupingRule {
-    pub(crate) fn new_inherited(
+    pub(crate) fn new_inherited(parent_stylesheet: &CSSStyleSheet) -> CSSGroupingRule {
-        parent_stylesheet: &CSSStyleSheet,
-        rules: Arc<Locked<StyleCssRules>>,
-    ) -> CSSGroupingRule {
         CSSGroupingRule {
             cssrule: CSSRule::new_inherited(parent_stylesheet),
-            rules,
             rulelist: MutNullableDom::new(None),
         }
     }
@@ -42,10 +37,19 @@ impl CSSGroupingRule {
     fn rulelist(&self, can_gc: CanGc) -> DomRoot<CSSRuleList> {
         let parent_stylesheet = self.upcast::<CSSRule>().parent_stylesheet();
         self.rulelist.or_init(|| {
+            let rules = if let Some(rule) = self.downcast::<CSSConditionRule>() {
+                rule.clone_rules()
+            } else if let Some(rule) = self.downcast::<CSSLayerBlockRule>() {
+                rule.clone_rules()
+            } else if let Some(rule) = self.downcast::<CSSStyleRule>() {
+                rule.ensure_rules()
+            } else {
+                unreachable!()
+            };
             CSSRuleList::new(
                 self.global().as_window(),
                 parent_stylesheet,
-                RulesSource::Rules(self.rules.clone()),
+                RulesSource::Rules(rules),
                 can_gc,
             )
         })

components/script/dom/csslayerblockrule.rs

@@ -4,8 +4,8 @@
 
 use dom_struct::dom_struct;
 use servo_arc::Arc;
-use style::shared_lock::ToCssWithGuard;
+use style::shared_lock::{Locked, ToCssWithGuard};
-use style::stylesheets::{CssRuleType, LayerBlockRule};
+use style::stylesheets::{CssRuleType, CssRules, LayerBlockRule};
 use style_traits::ToCss;
 
 use crate::dom::bindings::codegen::Bindings::CSSLayerBlockRuleBinding::CSSLayerBlockRuleMethods;
@@ -32,10 +32,7 @@ impl CSSLayerBlockRule {
         layerblockrule: Arc<LayerBlockRule>,
     ) -> CSSLayerBlockRule {
         CSSLayerBlockRule {
-            cssgroupingrule: CSSGroupingRule::new_inherited(
+            cssgroupingrule: CSSGroupingRule::new_inherited(parent_stylesheet),
-                parent_stylesheet,
-                layerblockrule.rules.clone(),
-            ),
             layerblockrule,
         }
     }
@@ -56,6 +53,10 @@ impl CSSLayerBlockRule {
             can_gc,
         )
     }
+
+    pub(crate) fn clone_rules(&self) -> Arc<Locked<CssRules>> {
+        self.layerblockrule.rules.clone()
+    }
 }
 
 impl SpecificCSSRule for CSSLayerBlockRule {

components/script/dom/cssrulelist.rs

@@ -190,20 +190,32 @@ impl CSSRuleList {
         self.dom_rules.borrow().get(idx as usize).map(|rule| {
             rule.or_init(|| {
                 let parent_stylesheet = &self.parent_stylesheet;
-                let guard = parent_stylesheet.shared_lock().read();
+                let lock = parent_stylesheet.shared_lock();
                 match self.rules {
-                    RulesSource::Rules(ref rules) => CSSRule::new_specific(
+                    RulesSource::Rules(ref rules) => {
+                        let rule = {
+                            let guard = lock.read();
+                            rules.read_with(&guard).0[idx as usize].clone()
+                        };
+                        CSSRule::new_specific(
                             self.global().as_window(),
                             parent_stylesheet,
-                        rules.read_with(&guard).0[idx as usize].clone(),
+                            rule,
                             can_gc,
-                    ),
+                        )
-                    RulesSource::Keyframes(ref rules) => DomRoot::upcast(CSSKeyframeRule::new(
+                    },
+                    RulesSource::Keyframes(ref rules) => {
+                        let rule = {
+                            let guard = lock.read();
+                            rules.read_with(&guard).keyframes[idx as usize].clone()
+                        };
+                        DomRoot::upcast(CSSKeyframeRule::new(
                             self.global().as_window(),
                             parent_stylesheet,
-                        rules.read_with(&guard).keyframes[idx as usize].clone(),
+                            rule,
                             can_gc,
-                    )),
+                        ))
+                    },
                 }
             })
         })

components/script/dom/cssstylerule.rs

@@ -10,14 +10,15 @@ use selectors::parser::{ParseRelative, SelectorList};
 use servo_arc::Arc;
 use style::selector_parser::SelectorParser;
 use style::shared_lock::{Locked, ToCssWithGuard};
-use style::stylesheets::{CssRuleType, Origin, StyleRule};
+use style::stylesheets::{CssRuleType, CssRules, Origin, StyleRule};
 
 use crate::dom::bindings::codegen::Bindings::CSSStyleRuleBinding::CSSStyleRuleMethods;
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{DomGlobal, reflect_dom_object};
 use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
 use crate::dom::bindings::str::DOMString;
-use crate::dom::cssrule::{CSSRule, SpecificCSSRule};
+use crate::dom::cssgroupingrule::CSSGroupingRule;
+use crate::dom::cssrule::SpecificCSSRule;
 use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
 use crate::dom::cssstylesheet::CSSStyleSheet;
 use crate::dom::node::NodeTraits;
@@ -26,7 +27,7 @@ use crate::script_runtime::CanGc;
 
 #[dom_struct]
 pub(crate) struct CSSStyleRule {
-    cssrule: CSSRule,
+    cssgroupingrule: CSSGroupingRule,
     #[ignore_malloc_size_of = "Arc"]
     #[no_trace]
     stylerule: Arc<Locked<StyleRule>>,
@@ -39,7 +40,7 @@ impl CSSStyleRule {
         stylerule: Arc<Locked<StyleRule>>,
     ) -> CSSStyleRule {
         CSSStyleRule {
-            cssrule: CSSRule::new_inherited(parent_stylesheet),
+            cssgroupingrule: CSSGroupingRule::new_inherited(parent_stylesheet),
             stylerule,
             style_decl: Default::default(),
         }
@@ -58,6 +59,16 @@ impl CSSStyleRule {
             can_gc,
         )
     }
+
+    pub(crate) fn ensure_rules(&self) -> Arc<Locked<CssRules>> {
+        let lock = self.cssgroupingrule.shared_lock();
+        let mut guard = lock.write();
+        self.stylerule
+            .write_with(&mut guard)
+            .rules
+            .get_or_insert_with(|| CssRules::new(vec![], lock))
+            .clone()
+    }
 }
 
 impl SpecificCSSRule for CSSStyleRule {
@@ -66,7 +77,7 @@ impl SpecificCSSRule for CSSStyleRule {
     }
 
     fn get_css(&self) -> DOMString {
-        let guard = self.cssrule.shared_lock().read();
+        let guard = self.cssgroupingrule.shared_lock().read();
         self.stylerule
             .read_with(&guard)
             .to_css_string(&guard)
@@ -78,7 +89,7 @@ impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule {
     // https://drafts.csswg.org/cssom/#dom-cssstylerule-style
     fn Style(&self) -> DomRoot<CSSStyleDeclaration> {
         self.style_decl.or_init(|| {
-            let guard = self.cssrule.shared_lock().read();
+            let guard = self.cssgroupingrule.shared_lock().read();
             CSSStyleDeclaration::new(
                 self.global().as_window(),
                 CSSStyleOwner::CSSRule(
@@ -94,14 +105,18 @@ impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule {
 
     // https://drafts.csswg.org/cssom/#dom-cssstylerule-selectortext
     fn SelectorText(&self) -> DOMString {
-        let guard = self.cssrule.shared_lock().read();
+        let guard = self.cssgroupingrule.shared_lock().read();
         let stylerule = self.stylerule.read_with(&guard);
         DOMString::from_string(stylerule.selectors.to_css_string())
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstylerule-selectortext
     fn SetSelectorText(&self, value: DOMString) {
-        let contents = &self.cssrule.parent_stylesheet().style_stylesheet().contents;
+        let contents = &self
+            .cssgroupingrule
+            .parent_stylesheet()
+            .style_stylesheet()
+            .contents;
         // It's not clear from the spec if we should use the stylesheet's namespaces.
         // https://github.com/w3c/csswg-drafts/issues/1511
         let namespaces = contents.namespaces.read();
@@ -118,10 +133,10 @@ impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule {
         // rule?
         if let Ok(mut s) = SelectorList::parse(&parser, &mut css_parser, ParseRelative::No) {
             // This mirrors what we do in CSSStyleOwner::mutate_associated_block.
-            let mut guard = self.cssrule.shared_lock().write();
+            let mut guard = self.cssgroupingrule.shared_lock().write();
             let stylerule = self.stylerule.write_with(&mut guard);
             mem::swap(&mut stylerule.selectors, &mut s);
-            if let Some(owner) = self.cssrule.parent_stylesheet().get_owner() {
+            if let Some(owner) = self.cssgroupingrule.parent_stylesheet().get_owner() {
                 owner.stylesheet_list_owner().invalidate_stylesheets();
             }
         }

components/script_bindings/webidls/CSSStyleRule.webidl

@@ -4,7 +4,7 @@
 
 // https://drafts.csswg.org/cssom/#the-cssstylerule-interface
 [Exposed=Window]
-interface CSSStyleRule : CSSRule {
+interface CSSStyleRule : CSSGroupingRule {
   attribute DOMString selectorText;
   [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
 };

tests/wpt/meta/css/css-nesting/cssom.html.ini

@@ -1,33 +1,6 @@
 [cssom.html]
-  [CSSStyleRule is a CSSGroupingRule]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 1]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 2]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 3]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 4]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 5]
-    expected: FAIL
-
-  [Simple CSSOM manipulation of subrules 6]
-    expected: FAIL
-
   [Simple CSSOM manipulation of subrules 7]
     expected: FAIL
 
-  [Simple CSSOM manipulation of subrules 9]
-    expected: FAIL
-
   [Manipulation of nested declarations through CSSOM]
     expected: FAIL

tests/wpt/meta/css/cssom/idlharness.html.ini

@@ -497,12 +497,6 @@
   [CSSImportRule interface: sheet.cssRules[0\] must inherit property "supportsText" with the proper type]
     expected: FAIL
 
-  [CSSStyleRule interface: existence and properties of interface object]
-    expected: FAIL
-
-  [CSSStyleRule interface: existence and properties of interface prototype object]
-    expected: FAIL
-
   [CSSGroupingRule interface: sheet.cssRules[4\] must inherit property "cssRules" with the proper type]
     expected: FAIL