From 299922243617dc5e15a092b2ea6fd7d6b090dcef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 10 Apr 2017 11:17:51 +0800 Subject: [PATCH] Bug 1325878: Don't use nsMediaList for loading imports. r=xidorn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MozReview-Commit-ID: HR23bqZcmcA Signed-off-by: Emilio Cobos Álvarez --- components/script/stylesheet_loader.rs | 8 ++++---- components/style/stylesheets.rs | 11 ++++++----- ports/geckolib/stylesheet_loader.rs | 22 +++++----------------- 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/components/script/stylesheet_loader.rs b/components/script/stylesheet_loader.rs index cdafde510c9..961d487a330 100644 --- a/components/script/stylesheet_loader.rs +++ b/components/script/stylesheet_loader.rs @@ -271,15 +271,15 @@ impl<'a> StylesheetLoader<'a> { impl<'a> StyleStylesheetLoader for StylesheetLoader<'a> { fn request_stylesheet( &self, - media: MediaList, - make_import: &mut FnMut(MediaList) -> ImportRule, + media: Arc>, + make_import: &mut FnMut(Arc>) -> ImportRule, make_arc: &mut FnMut(ImportRule) -> Arc>, ) -> Arc> { let import = make_import(media); let url = import.url.url().expect("Invalid urls shouldn't enter the loader").clone(); - //TODO (mrnayak) : Whether we should use the original loader's CORS setting? - //Fix this when spec has more details. + // TODO (mrnayak) : Whether we should use the original loader's CORS + // setting? Fix this when spec has more details. let source = StylesheetContextSource::Import(import.stylesheet.clone()); self.load(source, url, None, "".to_owned()); diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 3aa214df9e2..407fe7ee401 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -825,8 +825,8 @@ pub trait StylesheetLoader { /// before they’re locked, while keeping the trait object-safe. fn request_stylesheet( &self, - media: MediaList, - make_import: &mut FnMut(MediaList) -> ImportRule, + media: Arc>, + make_import: &mut FnMut(Arc>) -> ImportRule, make_arc: &mut FnMut(ImportRule) -> Arc>, ) -> Arc>; } @@ -836,8 +836,8 @@ struct NoOpLoader; impl StylesheetLoader for NoOpLoader { fn request_stylesheet( &self, - media: MediaList, - make_import: &mut FnMut(MediaList) -> ImportRule, + media: Arc>, + make_import: &mut FnMut(Arc>) -> ImportRule, make_arc: &mut FnMut(ImportRule) -> Arc>, ) -> Arc> { make_arc(make_import(media)) @@ -906,6 +906,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?; let media = parse_media_query_list(&self.context, input); + let media = Arc::new(self.shared_lock.wrap(media)); let noop_loader = NoOpLoader; let loader = if !specified_url.is_invalid() { @@ -920,7 +921,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { url: specified_url.take().unwrap(), stylesheet: Arc::new(Stylesheet { rules: CssRules::new(Vec::new(), self.shared_lock), - media: Arc::new(self.shared_lock.wrap(media)), + media: media, shared_lock: self.shared_lock.clone(), origin: self.context.stylesheet_origin, url_data: self.context.url_data.clone(), diff --git a/ports/geckolib/stylesheet_loader.rs b/ports/geckolib/stylesheet_loader.rs index 598d778cbdb..a79aa6e192f 100644 --- a/ports/geckolib/stylesheet_loader.rs +++ b/ports/geckolib/stylesheet_loader.rs @@ -5,11 +5,10 @@ use std::sync::Arc; use style::gecko_bindings::bindings::Gecko_LoadStyleSheet; use style::gecko_bindings::structs::{Loader, ServoStyleSheet}; -use style::gecko_bindings::sugar::ownership::HasArcFFI; +use style::gecko_bindings::sugar::ownership::{HasArcFFI, FFIArcHelpers}; use style::media_queries::MediaList; use style::shared_lock::Locked; use style::stylesheets::{ImportRule, Stylesheet, StylesheetLoader as StyleStylesheetLoader}; -use style_traits::ToCss; pub struct StylesheetLoader(*mut Loader, *mut ServoStyleSheet); @@ -22,21 +21,11 @@ impl StylesheetLoader { impl StyleStylesheetLoader for StylesheetLoader { fn request_stylesheet( &self, - media: MediaList, - make_import: &mut FnMut(MediaList) -> ImportRule, + media: Arc>, + make_import: &mut FnMut(Arc>) -> ImportRule, make_arc: &mut FnMut(ImportRule) -> Arc>, ) -> Arc> { - // TODO(emilio): We probably want to share media representation with - // Gecko in Stylo. - // - // This also allows us to get rid of a bunch of extra work to evaluate - // and ensure parity, and shouldn't be much Gecko work given we always - // evaluate them on the main thread. - // - // Meanwhile, this works. - let media_string = media.to_css_string(); - - let import = make_import(media); + let import = make_import(media.clone()); // After we get this raw pointer ImportRule will be moved into a lock and Arc // and so the Arc pointer inside will also move, @@ -52,8 +41,7 @@ impl StyleStylesheetLoader for StylesheetLoader { base_url_data, spec_bytes, spec_len as u32, - media_string.as_bytes().as_ptr(), - media_string.len() as u32); + media.into_strong()) } make_arc(import) }