From ac7bc414d9c6fb087aefc1e79c8db715051ef0d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 11 Apr 2017 22:13:16 +0800
Subject: [PATCH] Bug 1325878: Pass the MediaList down to Servo, making <style
 media> work. r=xidorn
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MozReview-Commit-ID: BUCSQJs2CNI
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
---
 components/layout_thread/lib.rs           |  6 +++---
 components/script/dom/cssrulelist.rs      | 18 ++++++++----------
 components/script/dom/htmlmetaelement.rs  |  3 ++-
 components/script/dom/htmlstyleelement.rs |  3 ++-
 components/script/dom/medialist.rs        |  2 +-
 components/script/stylesheet_loader.rs    |  4 ++--
 components/style/encoding_support.rs      |  3 ++-
 components/style/media_queries.rs         |  7 ++++---
 components/style/stylesheets.rs           |  4 ++--
 ports/geckolib/glue.rs                    | 16 ++++++++++++----
 tests/unit/style/media_queries.rs         |  9 +++++++--
 tests/unit/style/rule_tree/bench.rs       |  9 +++++----
 tests/unit/style/stylesheets.rs           | 17 ++++++++++-------
 tests/unit/style/viewport.rs              |  5 +++--
 14 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index d8a78f685fe..edba3c6ad45 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -113,7 +113,7 @@ use style::data::StoredRestyleHint;
 use style::dom::{ShowSubtree, ShowSubtreeDataAndPrimaryValues, TElement, TNode};
 use style::error_reporting::StdoutErrorReporter;
 use style::logical_geometry::LogicalPoint;
-use style::media_queries::{Device, MediaType};
+use style::media_queries::{Device, MediaList, MediaType};
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW};
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards};
 use style::stylesheets::{Origin, Stylesheet, UserAgentStylesheets};
@@ -1583,7 +1583,7 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
             None,
             None,
             Origin::UserAgent,
-            Default::default(),
+            MediaList::empty(),
             shared_lock.clone(),
             None,
             &StdoutErrorReporter))
@@ -1598,7 +1598,7 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
     }
     for &(ref contents, ref url) in &opts::get().user_stylesheets {
         user_or_user_agent_stylesheets.push(Stylesheet::from_bytes(
-            &contents, url.clone(), None, None, Origin::User, Default::default(),
+            &contents, url.clone(), None, None, Origin::User, MediaList::empty(),
             shared_lock.clone(), None, &StdoutErrorReporter));
     }
 
diff --git a/components/script/dom/cssrulelist.rs b/components/script/dom/cssrulelist.rs
index 4587659dfec..67eac2beda2 100644
--- a/components/script/dom/cssrulelist.rs
+++ b/components/script/dom/cssrulelist.rs
@@ -15,7 +15,7 @@ use dom::window::Window;
 use dom_struct::dom_struct;
 use std::sync::Arc;
 use style::shared_lock::Locked;
-use style::stylesheets::{CssRules, KeyframesRule, RulesMutateError};
+use style::stylesheets::{CssRules, CssRulesHelpers, KeyframesRule, RulesMutateError};
 
 #[allow(unsafe_code)]
 unsafe_no_jsmanaged_fields!(RulesSource);
@@ -90,15 +90,13 @@ impl CSSRuleList {
         let index = idx as usize;
 
         let parent_stylesheet = self.parent_stylesheet.style_stylesheet();
-        let new_rule = {
-            let mut guard = parent_stylesheet.shared_lock.write();
-            // FIXME We should probably pass in a proper StylesheetLoader.
-            //       See servo/servo#16240
-            css_rules.write_with(&mut guard).insert_rule(rule, parent_stylesheet,
-                                                         index, nested, None)?
-            // Drop `guard` here,
-            // CSSRule::new_specific re-acquires the lock for @support and @media.
-        };
+        let new_rule =
+            css_rules.insert_rule(&parent_stylesheet.shared_lock,
+                                  rule,
+                                  parent_stylesheet,
+                                  index,
+                                  nested,
+                                  None)?;
 
         let parent_stylesheet = &*self.parent_stylesheet;
         let dom_rule = CSSRule::new_specific(&window, parent_stylesheet, new_rule);
diff --git a/components/script/dom/htmlmetaelement.rs b/components/script/dom/htmlmetaelement.rs
index 8613f4c8cb0..3c4e817a132 100644
--- a/components/script/dom/htmlmetaelement.rs
+++ b/components/script/dom/htmlmetaelement.rs
@@ -24,6 +24,7 @@ use std::ascii::AsciiExt;
 use std::sync::Arc;
 use std::sync::atomic::AtomicBool;
 use style::attr::AttrValue;
+use style::media_queries::MediaList;
 use style::str::HTML_SPACE_CHARACTERS;
 use style::stylesheets::{Stylesheet, CssRule, CssRules, Origin};
 use style::viewport::ViewportRule;
@@ -107,7 +108,7 @@ impl HTMLMetaElement {
                         shared_lock: shared_lock.clone(),
                         url_data: window_from_node(self).get_url(),
                         namespaces: Default::default(),
-                        media: Arc::new(shared_lock.wrap(Default::default())),
+                        media: Arc::new(shared_lock.wrap(MediaList::empty())),
                         // Viewport constraints are always recomputed on resize; they don't need to
                         // force all styles to be recomputed.
                         dirty_on_viewport_size_change: AtomicBool::new(false),
diff --git a/components/script/dom/htmlstyleelement.rs b/components/script/dom/htmlstyleelement.rs
index fa0cdc92eab..098229d7e72 100644
--- a/components/script/dom/htmlstyleelement.rs
+++ b/components/script/dom/htmlstyleelement.rs
@@ -86,8 +86,9 @@ impl HTMLStyleElement {
         let context = CssParserContext::new_for_cssom(&url,
                                                       win.css_error_reporter(),
                                                       Some(CssRuleType::Media));
-        let mq = parse_media_query_list(&context, &mut CssParser::new(&mq_str));
         let shared_lock = node.owner_doc().style_shared_lock().clone();
+        let mq = Arc::new(shared_lock.wrap(
+                    parse_media_query_list(&context, &mut CssParser::new(&mq_str))));
         let loader = StylesheetLoader::for_element(self.upcast());
         let sheet = Stylesheet::from_str(&data, win.get_url(), Origin::Author, mq,
                                          shared_lock, Some(&loader),
diff --git a/components/script/dom/medialist.rs b/components/script/dom/medialist.rs
index 54604fbd443..da10d5d2a14 100644
--- a/components/script/dom/medialist.rs
+++ b/components/script/dom/medialist.rs
@@ -67,7 +67,7 @@ impl MediaListMethods for MediaList {
         // Step 2
         if value.is_empty() {
             // Step 1
-            *media_queries = StyleMediaList::default();
+            *media_queries = StyleMediaList::empty();
             return;
         }
         // Step 3
diff --git a/components/script/stylesheet_loader.rs b/components/script/stylesheet_loader.rs
index 961d487a330..ec13395a531 100644
--- a/components/script/stylesheet_loader.rs
+++ b/components/script/stylesheet_loader.rs
@@ -271,8 +271,8 @@ impl<'a> StylesheetLoader<'a> {
 impl<'a> StyleStylesheetLoader for StylesheetLoader<'a> {
     fn request_stylesheet(
         &self,
-        media: Arc<Locked<MediaList>>,
-        make_import: &mut FnMut(Arc<Locked<MediaList>>) -> ImportRule,
+        media: Arc<StyleLocked<MediaList>>,
+        make_import: &mut FnMut(Arc<StyleLocked<MediaList>>) -> ImportRule,
         make_arc: &mut FnMut(ImportRule) -> Arc<StyleLocked<ImportRule>>,
     ) -> Arc<StyleLocked<ImportRule>> {
         let import = make_import(media);
diff --git a/components/style/encoding_support.rs b/components/style/encoding_support.rs
index ce7aa43c1fd..cdd6775108a 100644
--- a/components/style/encoding_support.rs
+++ b/components/style/encoding_support.rs
@@ -12,6 +12,7 @@ use media_queries::MediaList;
 use self::encoding::{EncodingRef, DecoderTrap};
 use shared_lock::SharedRwLock;
 use std::str;
+use std::sync::Arc;
 use stylesheets::{Stylesheet, StylesheetLoader, Origin, UrlExtraData};
 
 struct RustEncoding;
@@ -62,7 +63,7 @@ impl Stylesheet {
         Stylesheet::from_str(&string,
                              url_data,
                              origin,
-                             media,
+                             Arc::new(shared_lock.wrap(media)),
                              shared_lock,
                              stylesheet_loader,
                              error_reporter)
diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs
index 9409b464f15..10a29509bd1 100644
--- a/components/style/media_queries.rs
+++ b/components/style/media_queries.rs
@@ -35,8 +35,9 @@ impl ToCss for MediaList {
     }
 }
 
-impl Default for MediaList {
-    fn default() -> MediaList {
+impl MediaList {
+    /// Create an empty MediaList.
+    pub fn empty() -> Self {
         MediaList { media_queries: vec![] }
     }
 }
@@ -251,7 +252,7 @@ impl MediaQuery {
 /// https://drafts.csswg.org/mediaqueries/#error-handling
 pub fn parse_media_query_list(context: &ParserContext, input: &mut Parser) -> MediaList {
     if input.is_exhausted() {
-        return Default::default()
+        return MediaList::empty()
     }
 
     let mut media_queries = vec![];
diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs
index 8bdcc9bce8c..f82924287f2 100644
--- a/components/style/stylesheets.rs
+++ b/components/style/stylesheets.rs
@@ -723,7 +723,7 @@ impl Stylesheet {
     pub fn from_str(css: &str,
                     url_data: UrlExtraData,
                     origin: Origin,
-                    media: MediaList,
+                    media: Arc<Locked<MediaList>>,
                     shared_lock: SharedRwLock,
                     stylesheet_loader: Option<&StylesheetLoader>,
                     error_reporter: &ParseErrorReporter) -> Stylesheet {
@@ -737,7 +737,7 @@ impl Stylesheet {
             url_data: url_data,
             namespaces: RwLock::new(namespaces),
             rules: CssRules::new(rules, &shared_lock),
-            media: Arc::new(shared_lock.wrap(media)),
+            media: media,
             shared_lock: shared_lock,
             dirty_on_viewport_size_change: AtomicBool::new(dirty_on_viewport_size_change),
             disabled: AtomicBool::new(false),
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index e3a5185fd4f..b56368de296 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -30,7 +30,7 @@ use style::gecko::wrapper::GeckoElement;
 use style::gecko_bindings::bindings;
 use style::gecko_bindings::bindings::{RawGeckoKeyframeListBorrowed, RawGeckoKeyframeListBorrowedMut};
 use style::gecko_bindings::bindings::{RawServoDeclarationBlockBorrowed, RawServoDeclarationBlockStrong};
-use style::gecko_bindings::bindings::{RawServoMediaListBorrowed, RawServoMediaListStrong};
+use style::gecko_bindings::bindings::{RawServoMediaList, RawServoMediaListBorrowed, RawServoMediaListStrong};
 use style::gecko_bindings::bindings::{RawServoMediaRule, RawServoMediaRuleBorrowed};
 use style::gecko_bindings::bindings::{RawServoNamespaceRule, RawServoNamespaceRuleBorrowed};
 use style::gecko_bindings::bindings::{RawServoPageRule, RawServoPageRuleBorrowed};
@@ -495,7 +495,8 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyl
     let shared_lock = global_style_data.shared_lock.clone();
     Arc::new(Stylesheet::from_str(
         "", unsafe { dummy_url_data() }.clone(), origin,
-        Default::default(), shared_lock, None, &StdoutErrorReporter)
+        Arc::new(shared_lock.wrap(MediaList::empty())),
+        shared_lock, None, &StdoutErrorReporter)
     ).into_strong()
 }
 
@@ -504,6 +505,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
                                                  stylesheet: *mut ServoStyleSheet,
                                                  data: *const nsACString,
                                                  mode: SheetParsingMode,
+                                                 media_list: *const RawServoMediaList,
                                                  extra_data: *mut URLExtraData)
                                                  -> RawServoStyleSheetStrong {
     let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -529,8 +531,14 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
     };
 
     let shared_lock = global_style_data.shared_lock.clone();
+    let media = if media_list.is_null() {
+        Arc::new(shared_lock.wrap(MediaList::empty()))
+    } else {
+        Locked::<MediaList>::as_arc(unsafe { &&*media_list }).clone()
+    };
+
     Arc::new(Stylesheet::from_str(
-        input, url_data.clone(), origin, Default::default(),
+        input, url_data.clone(), origin, media,
         shared_lock, loader, &StdoutErrorReporter)
     ).into_strong()
 }
@@ -1201,7 +1209,7 @@ pub extern "C" fn Servo_DeclarationBlock_RemovePropertyById(declarations: RawSer
 #[no_mangle]
 pub extern "C" fn Servo_MediaList_Create() -> RawServoMediaListStrong {
     let global_style_data = &*GLOBAL_STYLE_DATA;
-    Arc::new(global_style_data.shared_lock.wrap(MediaList::default())).into_strong()
+    Arc::new(global_style_data.shared_lock.wrap(MediaList::empty())).into_strong()
 }
 
 #[no_mangle]
diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs
index 3682eaa7794..634aa426bf5 100644
--- a/tests/unit/style/media_queries.rs
+++ b/tests/unit/style/media_queries.rs
@@ -6,6 +6,7 @@ use cssparser::{Parser, SourcePosition};
 use euclid::size::TypedSize2D;
 use servo_url::ServoUrl;
 use std::borrow::ToOwned;
+use std::sync::Arc;
 use style::Atom;
 use style::error_reporting::ParseErrorReporter;
 use style::media_queries::*;
@@ -28,8 +29,10 @@ fn test_media_rule<F>(css: &str, callback: F)
 {
     let url = ServoUrl::parse("http://localhost").unwrap();
     let css_str = css.to_owned();
+    let lock = SharedRwLock::new();
+    let media_list = Arc::new(lock.wrap(MediaList::empty()));
     let stylesheet = Stylesheet::from_str(
-        css, url, Origin::Author, Default::default(), SharedRwLock::new(),
+        css, url, Origin::Author, media_list, lock,
         None, &CSSErrorReporterTest);
     let mut rule_count = 0;
     let guard = stylesheet.shared_lock.read();
@@ -55,8 +58,10 @@ fn media_queries<F>(guard: &SharedRwLockReadGuard, rules: &[CssRule], f: &mut F)
 
 fn media_query_test(device: &Device, css: &str, expected_rule_count: usize) {
     let url = ServoUrl::parse("http://localhost").unwrap();
+    let lock = SharedRwLock::new();
+    let media_list = Arc::new(lock.wrap(MediaList::empty()));
     let ss = Stylesheet::from_str(
-        css, url, Origin::Author, Default::default(), SharedRwLock::new(),
+        css, url, Origin::Author, media_list, lock,
         None, &CSSErrorReporterTest);
     let mut rule_count = 0;
     ss.effective_style_rules(device, &ss.shared_lock.read(), |_| rule_count += 1);
diff --git a/tests/unit/style/rule_tree/bench.rs b/tests/unit/style/rule_tree/bench.rs
index f2e29c0ab8e..47cf48e0609 100644
--- a/tests/unit/style/rule_tree/bench.rs
+++ b/tests/unit/style/rule_tree/bench.rs
@@ -42,13 +42,14 @@ impl<'a> Drop for AutoGCRuleTree<'a> {
 }
 
 fn parse_rules(css: &str) -> Vec<(StyleSource, CascadeLevel)> {
+    let lock = SharedRwLock::new();
+    let media = Arc::new(lock.wrap(MediaList::empty()));
+
     let s = Stylesheet::from_str(css,
                                  ServoUrl::parse("http://localhost").unwrap(),
                                  Origin::Author,
-                                 MediaList {
-                                     media_queries: vec![],
-                                 },
-                                 SharedRwLock::new(),
+                                 media,
+                                 lock,
                                  None,
                                  &ErrorringErrorReporter);
     let guard = s.shared_lock.read();
diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs
index 67ecc7dfbd3..37ad9d84f92 100644
--- a/tests/unit/style/stylesheets.rs
+++ b/tests/unit/style/stylesheets.rs
@@ -15,6 +15,7 @@ use std::sync::Mutex;
 use std::sync::atomic::AtomicBool;
 use style::error_reporting::ParseErrorReporter;
 use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage};
+use style::media_queries::MediaList;
 use style::properties::Importance;
 use style::properties::{CSSWideKeyword, DeclaredValueOwned, PropertyDeclaration, PropertyDeclarationBlock};
 use style::properties::longhands;
@@ -61,14 +62,15 @@ fn test_parse_stylesheet() {
             }
         }";
     let url = ServoUrl::parse("about::test").unwrap();
-    let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, Default::default(),
-                                          SharedRwLock::new(), None,
-                                          &CSSErrorReporterTest);
+    let lock = SharedRwLock::new();
+    let media = Arc::new(lock.wrap(MediaList::empty()));
+    let stylesheet = Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock,
+                                          None, &CSSErrorReporterTest);
     let mut namespaces = Namespaces::default();
     namespaces.default = Some(ns!(html));
     let expected = Stylesheet {
         origin: Origin::UserAgent,
-        media: Arc::new(stylesheet.shared_lock.wrap(Default::default())),
+        media: Arc::new(stylesheet.shared_lock.wrap(MediaList::empty())),
         shared_lock: stylesheet.shared_lock.clone(),
         namespaces: RwLock::new(namespaces),
         url_data: url,
@@ -323,9 +325,10 @@ fn test_report_error_stylesheet() {
 
     let errors = error_reporter.errors.clone();
 
-    Stylesheet::from_str(css, url.clone(), Origin::UserAgent, Default::default(),
-                         SharedRwLock::new(), None,
-                         &error_reporter);
+    let lock = SharedRwLock::new();
+    let media = Arc::new(lock.wrap(MediaList::empty()));
+    Stylesheet::from_str(css, url.clone(), Origin::UserAgent, media, lock,
+                         None, &error_reporter);
 
     let mut errors = errors.lock().unwrap();
 
diff --git a/tests/unit/style/viewport.rs b/tests/unit/style/viewport.rs
index 252aa266478..f46a92bdd21 100644
--- a/tests/unit/style/viewport.rs
+++ b/tests/unit/style/viewport.rs
@@ -7,7 +7,8 @@ use euclid::size::TypedSize2D;
 use media_queries::CSSErrorReporterTest;
 use servo_config::prefs::{PREFS, PrefValue};
 use servo_url::ServoUrl;
-use style::media_queries::{Device, MediaType};
+use std::sync::Arc;
+use style::media_queries::{Device, MediaList, MediaType};
 use style::parser::{Parse, ParserContext};
 use style::shared_lock::SharedRwLock;
 use style::stylesheets::{CssRuleType, Stylesheet, Origin};
@@ -27,7 +28,7 @@ macro_rules! stylesheet {
             $css,
             ServoUrl::parse("http://localhost").unwrap(),
             Origin::$origin,
-            Default::default(),
+            Arc::new($shared_lock.wrap(MediaList::empty())),
             $shared_lock,
             None,
             &$error_reporter