Unify media query and media query list parsing (#36520)

Several places were using identical logic in order to parse a media
queries or media query lists.

This patch centralizes the logic into 2 new helper methods in MediaList.

Testing: not needed (no behavior change)

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

components/script/dom/htmllinkelement.rs

@@ -8,7 +8,6 @@ use std::default::Default;
 
 use base::id::WebViewId;
 use content_security_policy as csp;
-use cssparser::{Parser as CssParser, ParserInput};
 use dom_struct::dom_struct;
 use embedder_traits::EmbedderMsg;
 use html5ever::{LocalName, Prefix, local_name, namespace_url, ns};
@@ -25,10 +24,7 @@ use net_traits::{
 use servo_arc::Arc;
 use servo_url::ServoUrl;
 use style::attr::AttrValue;
-use style::media_queries::MediaList;
+use style::stylesheets::Stylesheet;
-use style::parser::ParserContext as CssParserContext;
-use style::stylesheets::{CssRuleType, Origin, Stylesheet, UrlExtraData};
-use style_traits::ParsingMode;
 use stylo_atoms::Atom;
 
 use crate::dom::attr::Attr;
@@ -50,6 +46,7 @@ use crate::dom::element::{
     set_cross_origin_attribute,
 };
 use crate::dom::htmlelement::HTMLElement;
+use crate::dom::medialist::MediaList;
 use crate::dom::node::{BindContext, Node, NodeTraits, UnbindContext};
 use crate::dom::performanceresourcetiming::InitiatorType;
 use crate::dom::stylesheet::StyleSheet as DOMStyleSheet;
@@ -449,24 +446,7 @@ impl HTMLLinkElement {
             None => "",
         };
 
-        let mut input = ParserInput::new(mq_str);
+        let media = MediaList::parse_media_list(mq_str, document.window());
-        let mut css_parser = CssParser::new(&mut input);
-        let document_url_data = &UrlExtraData(document.url().get_arc());
-        let window = document.window();
-        // FIXME(emilio): This looks somewhat fishy, since we use the context
-        // only to parse the media query list, CssRuleType::Media doesn't make
-        // much sense.
-        let context = CssParserContext::new(
-            Origin::Author,
-            document_url_data,
-            Some(CssRuleType::Media),
-            ParsingMode::DEFAULT,
-            document.quirks_mode(),
-            /* namespaces = */ Default::default(),
-            window.css_error_reporter(),
-            None,
-        );
-        let media = MediaList::parse(&context, &mut css_parser);
 
         let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity"));
         let integrity_val = im_attribute.as_ref().map(|a| a.value());

components/script/dom/htmlstyleelement.rs

@@ -4,16 +4,13 @@
 
 use std::cell::Cell;
 
-use cssparser::{Parser as CssParser, ParserInput};
 use dom_struct::dom_struct;
 use html5ever::{LocalName, Prefix};
 use js::rust::HandleObject;
 use net_traits::ReferrerPolicy;
 use servo_arc::Arc;
-use style::media_queries::MediaList;
+use style::media_queries::MediaList as StyleMediaList;
-use style::parser::ParserContext as CssParserContext;
+use style::stylesheets::{AllowImportRules, Origin, Stylesheet, UrlExtraData};
-use style::stylesheets::{AllowImportRules, CssRuleType, Origin, Stylesheet, UrlExtraData};
-use style_traits::ParsingMode;
 
 use crate::dom::attr::Attr;
 use crate::dom::bindings::cell::DomRefCell;
@@ -26,6 +23,7 @@ use crate::dom::cssstylesheet::CSSStyleSheet;
 use crate::dom::document::Document;
 use crate::dom::element::{AttributeMutation, Element, ElementCreator};
 use crate::dom::htmlelement::HTMLElement;
+use crate::dom::medialist::MediaList;
 use crate::dom::node::{BindContext, ChildrenMutation, Node, NodeTraits, UnbindContext};
 use crate::dom::stylesheet::StyleSheet as DOMStyleSheet;
 use crate::dom::virtualmethods::VirtualMethods;
@@ -83,26 +81,9 @@ impl HTMLStyleElement {
         )
     }
 
-    fn create_media_list(&self, mq_str: &str) -> MediaList {
+    #[inline]
-        if mq_str.is_empty() {
+    fn create_media_list(&self, mq_str: &str) -> StyleMediaList {
-            return MediaList::empty();
+        MediaList::parse_media_list(mq_str, &self.owner_window())
-        }
-
-        let window = self.owner_window();
-        let doc = self.owner_document();
-        let url_data = UrlExtraData(window.get_url().get_arc());
-        let context = CssParserContext::new(
-            Origin::Author,
-            &url_data,
-            Some(CssRuleType::Media),
-            ParsingMode::DEFAULT,
-            doc.quirks_mode(),
-            /* namespaces = */ Default::default(),
-            window.css_error_reporter(),
-            None,
-        );
-        let mut input = ParserInput::new(mq_str);
-        MediaList::parse(&context, &mut CssParser::new(&mut input))
     }
 
     pub(crate) fn parse_own_css(&self) {
@@ -279,7 +260,7 @@ impl VirtualMethods for HTMLStyleElement {
                 let media = stylesheet.media.write_with(&mut guard);
                 match mutation {
                     AttributeMutation::Set(_) => *media = self.create_media_list(&attr.value()),
-                    AttributeMutation::Removed => *media = MediaList::empty(),
+                    AttributeMutation::Removed => *media = StyleMediaList::empty(),
                 };
                 self.owner_document().invalidate_stylesheets();
             }

components/script/dom/medialist.rs

@@ -9,7 +9,7 @@ use style::media_queries::{MediaList as StyleMediaList, MediaQuery};
 use style::parser::ParserContext;
 use style::shared_lock::{Locked, SharedRwLock};
 use style::stylesheets::{CssRuleType, Origin, UrlExtraData};
-use style_traits::{ParsingMode, ToCss};
+use style_traits::{ParseError, ParsingMode, ToCss};
 
 use crate::dom::bindings::codegen::Bindings::MediaListBinding::MediaListMethods;
 use crate::dom::bindings::codegen::Bindings::WindowBinding::Window_Binding::WindowMethods;
@@ -59,30 +59,39 @@ impl MediaList {
     fn shared_lock(&self) -> &SharedRwLock {
         &self.parent_stylesheet.style_stylesheet().shared_lock
     }
-}
 
-impl MediaListMethods<crate::DomTypeHolder> for MediaList {
+    /// <https://drafts.csswg.org/cssom/#parse-a-media-query-list>
-    /// <https://drafts.csswg.org/cssom/#dom-medialist-mediatext>
+    pub(crate) fn parse_media_list(value: &str, window: &Window) -> StyleMediaList {
-    fn MediaText(&self) -> DOMString {
-        let guard = self.shared_lock().read();
-        DOMString::from(self.media_queries.read_with(&guard).to_css_string())
-    }
-
-    /// <https://drafts.csswg.org/cssom/#dom-medialist-mediatext>
-    fn SetMediaText(&self, value: DOMString) {
-        let mut guard = self.shared_lock().write();
-        let media_queries = self.media_queries.write_with(&mut guard);
-        // Step 2
         if value.is_empty() {
-            // Step 1
+            return StyleMediaList::empty();
-            *media_queries = StyleMediaList::empty();
-            return;
         }
-        // Step 3
+        let mut input = ParserInput::new(value);
-        let mut input = ParserInput::new(&value);
+        let mut parser = Parser::new(&mut input);
+        let url_data = UrlExtraData(window.get_url().get_arc());
+        let quirks_mode = window.Document().quirks_mode();
+        // FIXME(emilio): This looks somewhat fishy, since we use the context
+        // only to parse the media query list, CssRuleType::Media doesn't make
+        // much sense.
+        let context = ParserContext::new(
+            Origin::Author,
+            &url_data,
+            Some(CssRuleType::Media),
+            ParsingMode::DEFAULT,
+            quirks_mode,
+            /* namespaces = */ Default::default(),
+            window.css_error_reporter(),
+            None,
+        );
+        StyleMediaList::parse(&context, &mut parser)
+    }
+
+    /// <https://drafts.csswg.org/cssom/#parse-a-media-query>
+    pub(crate) fn parse_media_query<'i>(
+        value: &'i str,
+        window: &Window,
+    ) -> Result<MediaQuery, ParseError<'i>> {
+        let mut input = ParserInput::new(value);
         let mut parser = Parser::new(&mut input);
-        let global = self.global();
-        let window = global.as_window();
         let url_data = UrlExtraData(window.get_url().get_arc());
         let quirks_mode = window.Document().quirks_mode();
         let context = ParserContext::new(
@@ -95,7 +104,23 @@ impl MediaListMethods<crate::DomTypeHolder> for MediaList {
             window.css_error_reporter(),
             None,
         );
-        *media_queries = StyleMediaList::parse(&context, &mut parser);
+        MediaQuery::parse(&context, &mut parser)
+    }
+}
+
+impl MediaListMethods<crate::DomTypeHolder> for MediaList {
+    /// <https://drafts.csswg.org/cssom/#dom-medialist-mediatext>
+    fn MediaText(&self) -> DOMString {
+        let guard = self.shared_lock().read();
+        DOMString::from(self.media_queries.read_with(&guard).to_css_string())
+    }
+
+    /// <https://drafts.csswg.org/cssom/#dom-medialist-mediatext>
+    fn SetMediaText(&self, value: DOMString) {
+        let global = self.global();
+        let mut guard = self.shared_lock().write();
+        let media_queries = self.media_queries.write_with(&mut guard);
+        *media_queries = Self::parse_media_list(&value, global.as_window());
     }
 
     // https://drafts.csswg.org/cssom/#dom-medialist-length
@@ -122,23 +147,8 @@ impl MediaListMethods<crate::DomTypeHolder> for MediaList {
     /// <https://drafts.csswg.org/cssom/#dom-medialist-appendmedium>
     fn AppendMedium(&self, medium: DOMString) {
         // Step 1
-        let mut input = ParserInput::new(&medium);
-        let mut parser = Parser::new(&mut input);
         let global = self.global();
-        let win = global.as_window();
+        let m = Self::parse_media_query(&medium, global.as_window());
-        let url_data = UrlExtraData(win.get_url().get_arc());
-        let quirks_mode = win.Document().quirks_mode();
-        let context = ParserContext::new(
-            Origin::Author,
-            &url_data,
-            Some(CssRuleType::Media),
-            ParsingMode::DEFAULT,
-            quirks_mode,
-            /* namespaces = */ Default::default(),
-            win.css_error_reporter(),
-            None,
-        );
-        let m = MediaQuery::parse(&context, &mut parser);
         // Step 2
         if m.is_err() {
             return;
@@ -161,23 +171,8 @@ impl MediaListMethods<crate::DomTypeHolder> for MediaList {
     /// <https://drafts.csswg.org/cssom/#dom-medialist-deletemedium>
     fn DeleteMedium(&self, medium: DOMString) {
         // Step 1
-        let mut input = ParserInput::new(&medium);
-        let mut parser = Parser::new(&mut input);
         let global = self.global();
-        let win = global.as_window();
+        let m = Self::parse_media_query(&medium, global.as_window());
-        let url_data = UrlExtraData(win.get_url().get_arc());
-        let quirks_mode = win.Document().quirks_mode();
-        let context = ParserContext::new(
-            Origin::Author,
-            &url_data,
-            Some(CssRuleType::Media),
-            ParsingMode::DEFAULT,
-            quirks_mode,
-            /* namespaces = */ Default::default(),
-            win.css_error_reporter(),
-            None,
-        );
-        let m = MediaQuery::parse(&context, &mut parser);
         // Step 2
         if m.is_err() {
             return;

components/script/dom/window.rs

@@ -27,7 +27,7 @@ use constellation_traits::{
     ScriptToConstellationMessage, ScrollState, StructuredSerializedData, WindowSizeType,
 };
 use crossbeam_channel::{Sender, unbounded};
-use cssparser::{Parser, ParserInput, SourceLocation};
+use cssparser::SourceLocation;
 use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarkerType};
 use dom_struct::dom_struct;
 use embedder_traits::user_content_manager::{UserContentManager, UserScript};
@@ -74,15 +74,13 @@ use servo_geometry::{DeviceIndependentIntRect, MaxRect, f32_rect_to_au_rect};
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
 use style::dom::OpaqueNode;
 use style::error_reporting::{ContextualParseError, ParseErrorReporter};
-use style::media_queries;
-use style::parser::ParserContext as CssParserContext;
 use style::properties::PropertyId;
 use style::properties::style_structs::Font;
 use style::queries::values::PrefersColorScheme;
 use style::selector_parser::PseudoElement;
 use style::str::HTML_SPACE_CHARACTERS;
-use style::stylesheets::{CssRuleType, Origin, UrlExtraData};
+use style::stylesheets::UrlExtraData;
-use style_traits::{CSSPixel, ParsingMode};
+use style_traits::CSSPixel;
 use stylo_atoms::Atom;
 use url::Position;
 use webrender_api::units::{DevicePixel, LayoutPixel};
@@ -133,6 +131,7 @@ use crate::dom::history::History;
 use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
 use crate::dom::htmliframeelement::HTMLIFrameElement;
 use crate::dom::location::Location;
+use crate::dom::medialist::MediaList;
 use crate::dom::mediaquerylist::{MediaQueryList, MediaQueryListMatchState};
 use crate::dom::mediaquerylistevent::MediaQueryListEvent;
 use crate::dom::messageevent::MessageEvent;
@@ -1457,21 +1456,7 @@ impl WindowMethods<crate::DomTypeHolder> for Window {
 
     // https://drafts.csswg.org/cssom-view/#dom-window-matchmedia
     fn MatchMedia(&self, query: DOMString) -> DomRoot<MediaQueryList> {
-        let mut input = ParserInput::new(&query);
+        let media_query_list = MediaList::parse_media_list(&query, self);
-        let mut parser = Parser::new(&mut input);
-        let url_data = UrlExtraData(self.get_url().get_arc());
-        let quirks_mode = self.Document().quirks_mode();
-        let context = CssParserContext::new(
-            Origin::Author,
-            &url_data,
-            Some(CssRuleType::Media),
-            ParsingMode::DEFAULT,
-            quirks_mode,
-            /* namespaces = */ Default::default(),
-            self.css_error_reporter(),
-            None,
-        );
-        let media_query_list = media_queries::MediaList::parse(&context, &mut parser);
         let document = self.Document();
         let mql = MediaQueryList::new(&document, media_query_list, CanGc::note());
         self.media_query_lists.track(&*mql);