Bug 1830656 - Refactor ImportLayer into enum r=emilio

Refactored ImportLayer into an enum instead of a struct and using
Option everywhere.

Differential Revision: https://phabricator.services.mozilla.com/D176793

servo/components/style/stylesheets/import_rule.rs

@@ -138,11 +138,17 @@ impl DeepCloneWithLock for ImportSheet {
     }
 }
 
-/// The layer keyword or function in an import rule.
+/// The layer specified in an import rule (can be none, anonymous, or named).
 #[derive(Debug, Clone)]
-pub struct ImportLayer {
-    /// The layer name, or None for an anonymous layer.
-    pub name: Option<LayerName>,
+pub enum ImportLayer {
+    /// No layer specified
+    None,
+
+    /// Anonymous layer (`layer`)
+    Anonymous,
+
+    /// Named layer (`layer(name)`)
+    Named(LayerName),
 }
 
 /// The supports condition in an import rule.
@@ -160,9 +166,10 @@ impl ToCss for ImportLayer {
     where
         W: Write,
     {
-        match self.name {
-            None => dest.write_str("layer"),
-            Some(ref name) => {
+        match *self {
+            ImportLayer::None => Ok(()),
+            ImportLayer::Anonymous => dest.write_str("layer"),
+            ImportLayer::Named(ref name) => {
                 dest.write_str("layer(")?;
                 name.to_css(dest)?;
                 dest.write_char(')')
@@ -188,7 +195,7 @@ pub struct ImportRule {
     pub supports: Option<ImportSupportsCondition>,
 
     /// A `layer()` function name.
-    pub layer: Option<ImportLayer>,
+    pub layer: ImportLayer,
 
     /// The line and column of the rule's source code.
     pub source_location: SourceLocation,
@@ -207,21 +214,21 @@ impl ImportRule {
         input: &mut Parser<'i, 't>,
         context: &ParserContext,
         namespaces: &Namespaces,
-    ) -> (Option<ImportLayer>, Option<ImportSupportsCondition>) {
+    ) -> (ImportLayer, Option<ImportSupportsCondition>) {
         let layer = if input
             .try_parse(|input| input.expect_ident_matching("layer"))
             .is_ok()
         {
-            Some(ImportLayer { name: None })
+            ImportLayer::Anonymous
         } else {
             input
                 .try_parse(|input| {
                     input.expect_function_matching("layer")?;
                     input
                         .parse_nested_block(|input| LayerName::parse(context, input))
-                        .map(|name| ImportLayer { name: Some(name) })
+                        .map(|name| ImportLayer::Named(name))
                 })
-                .ok()
+                .ok().unwrap_or(ImportLayer::None)
         };
 
         let supports = if !static_prefs::pref!("layout.css.import-supports.enabled") {
@@ -272,9 +279,9 @@ impl ToCssWithGuard for ImportRule {
         dest.write_str("@import ")?;
         self.url.to_css(&mut CssWriter::new(dest))?;
 
-        if let Some(ref layer) = self.layer {
+        if !matches!(self.layer, ImportLayer::None) {
             dest.write_char(' ')?;
-            layer.to_css(&mut CssWriter::new(dest))?;
+            self.layer.to_css(&mut CssWriter::new(dest))?;
         }
 
         if let Some(ref supports) = self.supports {

servo/components/style/stylesheets/loader.rs

@@ -26,6 +26,6 @@ pub trait StylesheetLoader {
         lock: &SharedRwLock,
         media: Arc<Locked<MediaList>>,
         supports: Option<ImportSupportsCondition>,
-        layer: Option<ImportLayer>,
+        layer: ImportLayer,
     ) -> Arc<Locked<ImportRule>>;
 }

servo/components/style/stylesheets/rule_parser.rs

@@ -204,7 +204,7 @@ pub enum AtRulePrelude {
     /// A @document rule, with its conditional.
     Document(DocumentCondition),
     /// A @import rule prelude.
-    Import(CssUrl, Arc<Locked<MediaList>>, Option<ImportSupportsCondition>, Option<ImportLayer>),
+    Import(CssUrl, Arc<Locked<MediaList>>, Option<ImportSupportsCondition>, ImportLayer),
     /// A @namespace rule prelude.
     Namespace(Option<Prefix>, Namespace),
     /// A @layer rule prelude.

servo/components/style/stylist.rs

@@ -28,6 +28,7 @@ use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind};
 use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher};
 use crate::stylesheets::container_rule::ContainerCondition;
+use crate::stylesheets::import_rule::ImportLayer;
 use crate::stylesheets::keyframes_rule::KeyframesAnimation;
 use crate::stylesheets::layer_rule::{LayerName, LayerOrder};
 use crate::stylesheets::viewport_rule::{self, MaybeNew, ViewportRule};
@@ -2936,8 +2937,10 @@ impl CascadeData {
                         self.effective_media_query_results
                             .saw_effective(import_rule);
                     }
-                    if let Some(ref layer) = import_rule.layer {
-                        maybe_register_layers(self, layer.name.as_ref(), containing_rule_state);
+                    match import_rule.layer {
+                        ImportLayer::Named(ref name) => maybe_register_layers(self, Some(name), containing_rule_state),
+                        ImportLayer::Anonymous => maybe_register_layers(self, None, containing_rule_state),
+                        ImportLayer::None => {},
                     }
                 },
                 CssRule::Media(ref lock) => {

servo/ports/geckolib/glue.rs

@@ -120,7 +120,7 @@ use style::shared_lock::{
 use style::string_cache::{Atom, WeakAtom};
 use style::style_adjuster::StyleAdjuster;
 use style::stylesheets::container_rule::ContainerSizeQuery;
-use style::stylesheets::import_rule::ImportSheet;
+use style::stylesheets::import_rule::{ImportSheet, ImportLayer};
 use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue};
 use style::stylesheets::layer_rule::LayerOrder;
 use style::stylesheets::supports_rule::parse_condition_or_declaration;
@@ -2710,9 +2710,11 @@ pub extern "C" fn Servo_ImportRule_GetLayerName(
     rule: &RawServoImportRule,
     result: &mut nsACString,
 ) {
+    // https://w3c.github.io/csswg-drafts/cssom/#dom-cssimportrule-layername
     read_locked_arc(rule, |rule: &ImportRule| match rule.layer {
-        Some(ref layer) => layer.name.to_css(&mut CssWriter::new(result)).unwrap(),
-        None => result.set_is_void(true),
+        ImportLayer::Named(ref name) => name.to_css(&mut CssWriter::new(result)).unwrap(), // "return the layer name declared in the at-rule itself"
+        ImportLayer::Anonymous => {}, // "or an empty string if the layer is anonymous"
+        ImportLayer::None => result.set_is_void(true), // "or null if the at-rule does not declare a layer"
     })
 }
 

servo/ports/geckolib/stylesheet_loader.rs

@@ -53,7 +53,7 @@ impl StyleStylesheetLoader for StylesheetLoader {
         lock: &SharedRwLock,
         media: Arc<Locked<MediaList>>,
         supports: Option<ImportSupportsCondition>,
-        layer: Option<ImportLayer>,
+        layer: ImportLayer,
     ) -> Arc<Locked<ImportRule>> {
         // Ensure the supports conditions for this @import are true, if not, refuse to load
         if !supports.as_ref().map_or(true, |s| s.enabled) {
@@ -175,7 +175,7 @@ impl StyleStylesheetLoader for AsyncStylesheetParser {
         lock: &SharedRwLock,
         media: Arc<Locked<MediaList>>,
         supports: Option<ImportSupportsCondition>,
-        layer: Option<ImportLayer>,
+        layer: ImportLayer,
     ) -> Arc<Locked<ImportRule>> {
         // Ensure the supports conditions for this @import are true, if not, refuse to load
         if !supports.as_ref().map_or(true, |s| s.enabled) {