Bug 1325878: Don't use nsMediaList for loading imports. r?xidorn
MozReview-Commit-ID: HR23bqZcmcA
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -33,26 +33,27 @@
#include "nsNameSpaceManager.h"
#include "nsNetUtil.h"
#include "nsRuleNode.h"
#include "nsString.h"
#include "nsStyleStruct.h"
#include "nsStyleUtil.h"
#include "nsTArray.h"
+#include "mozilla/DeclarationBlockInlines.h"
#include "mozilla/EffectCompositor.h"
#include "mozilla/EffectSet.h"
#include "mozilla/EventStates.h"
#include "mozilla/Keyframe.h"
#include "mozilla/Mutex.h"
#include "mozilla/ServoElementSnapshot.h"
#include "mozilla/ServoRestyleManager.h"
#include "mozilla/StyleAnimationValue.h"
#include "mozilla/SystemGroup.h"
-#include "mozilla/DeclarationBlockInlines.h"
+#include "mozilla/ServoMediaList.h"
#include "mozilla/dom/Element.h"
#include "mozilla/dom/ElementInlines.h"
#include "mozilla/dom/HTMLTableCellElement.h"
#include "mozilla/dom/HTMLBodyElement.h"
#include "mozilla/LookAndFeel.h"
#include "mozilla/URLExtraData.h"
using namespace mozilla;
@@ -1682,35 +1683,24 @@ Gecko_GetFontMetrics(RawGeckoPresContext
void
Gecko_LoadStyleSheet(css::Loader* aLoader,
ServoStyleSheet* aParent,
RawServoStyleSheetBorrowed aChildSheet,
RawGeckoURLExtraData* aBaseURLData,
const uint8_t* aURLString,
uint32_t aURLStringLength,
- const uint8_t* aMediaString,
- uint32_t aMediaStringLength)
+ RawServoMediaListStrong aMediaList)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(aLoader, "Should've catched this before");
MOZ_ASSERT(aParent, "Only used for @import, so parent should exist!");
MOZ_ASSERT(aURLString, "Invalid URLs shouldn't be loaded!");
MOZ_ASSERT(aBaseURLData, "Need base URL data");
- RefPtr<nsMediaList> media = new nsMediaList();
- if (aMediaStringLength) {
- MOZ_ASSERT(aMediaString);
- // TODO(emilio, bug 1325878): This is not great, though this is going away
- // soon anyway, when we can have a Servo-backed nsMediaList.
- nsDependentCSubstring medium(reinterpret_cast<const char*>(aMediaString),
- aMediaStringLength);
- nsCSSParser mediumParser(aLoader);
- mediumParser.ParseMediaList(
- NS_ConvertUTF8toUTF16(medium), nullptr, 0, media);
- }
+ RefPtr<dom::MediaList> media = new ServoMediaList(aMediaList.Consume());
nsDependentCSubstring urlSpec(reinterpret_cast<const char*>(aURLString),
aURLStringLength);
nsCOMPtr<nsIURI> uri;
nsresult rv = NS_NewURI(getter_AddRefs(uri), urlSpec, nullptr,
aBaseURLData->BaseURI());
if (NS_FAILED(rv)) {
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -126,18 +126,17 @@ RawGeckoElementBorrowedOrNull Gecko_GetP
RawGeckoElementBorrowedOrNull Gecko_GetNextSiblingElement(RawGeckoElementBorrowed element);
RawGeckoElementBorrowedOrNull Gecko_GetDocumentElement(RawGeckoDocumentBorrowed document);
void Gecko_LoadStyleSheet(mozilla::css::Loader* loader,
mozilla::ServoStyleSheet* parent,
RawServoStyleSheetBorrowed child_sheet,
RawGeckoURLExtraData* base_url_data,
const uint8_t* url_bytes,
uint32_t url_length,
- const uint8_t* media_bytes,
- uint32_t media_length);
+ RawServoMediaListStrong media_list);
// By default, Servo walks the DOM by traversing the siblings of the DOM-view
// first child. This generally works, but misses anonymous children, which we
// want to traverse during styling. To support these cases, we create an
// optional heap-allocated iterator for nodes that need it. If the creation
// method returns null, Servo falls back to the aforementioned simpler (and
// faster) sibling traversal.
StyleChildrenIteratorOwnedOrNull Gecko_MaybeCreateStyleChildrenIterator(RawGeckoNodeBorrowed node);
--- a/servo/components/script/stylesheet_loader.rs
+++ b/servo/components/script/stylesheet_loader.rs
@@ -266,23 +266,23 @@ impl<'a> StylesheetLoader<'a> {
document.fetch_async(LoadType::Stylesheet(url), request, action_sender);
}
}
impl<'a> StyleStylesheetLoader for StylesheetLoader<'a> {
fn request_stylesheet(
&self,
- media: MediaList,
- make_import: &mut FnMut(MediaList) -> ImportRule,
+ media: Arc<Locked<MediaList>>,
+ make_import: &mut FnMut(Arc<Locked<MediaList>>) -> ImportRule,
make_arc: &mut FnMut(ImportRule) -> Arc<StyleLocked<ImportRule>>,
) -> Arc<StyleLocked<ImportRule>> {
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());
make_arc(import)
}
}
--- a/servo/components/style/stylesheets.rs
+++ b/servo/components/style/stylesheets.rs
@@ -819,29 +819,29 @@ pub trait StylesheetLoader {
///
/// The called code is responsible to update the `stylesheet` rules field
/// when the sheet is done loading.
///
/// The convoluted signature allows impls to look at MediaList and ImportRule
/// before they’re locked, while keeping the trait object-safe.
fn request_stylesheet(
&self,
- media: MediaList,
- make_import: &mut FnMut(MediaList) -> ImportRule,
+ media: Arc<Locked<MediaList>>,
+ make_import: &mut FnMut(Arc<Locked<MediaList>>) -> ImportRule,
make_arc: &mut FnMut(ImportRule) -> Arc<Locked<ImportRule>>,
) -> Arc<Locked<ImportRule>>;
}
struct NoOpLoader;
impl StylesheetLoader for NoOpLoader {
fn request_stylesheet(
&self,
- media: MediaList,
- make_import: &mut FnMut(MediaList) -> ImportRule,
+ media: Arc<Locked<MediaList>>,
+ make_import: &mut FnMut(Arc<Locked<MediaList>>) -> ImportRule,
make_arc: &mut FnMut(ImportRule) -> Arc<Locked<ImportRule>>,
) -> Arc<Locked<ImportRule>> {
make_arc(make_import(media))
}
}
struct TopLevelRuleParser<'a> {
@@ -900,31 +900,32 @@ impl<'a> AtRuleParser for TopLevelRulePa
match_ignore_ascii_case! { name,
"import" => {
if self.state.get() <= State::Imports {
self.state.set(State::Imports);
let url_string = input.expect_url_or_string()?;
let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?;
let media = parse_media_query_list(input);
+ let media = Arc::new(self.shared_lock.wrap(media));
let noop_loader = NoOpLoader;
let loader = if !specified_url.is_invalid() {
self.loader.expect("Expected a stylesheet loader for @import")
} else {
&noop_loader
};
let mut specified_url = Some(specified_url);
let arc = loader.request_stylesheet(media, &mut |media| {
ImportRule {
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(),
namespaces: RwLock::new(Namespaces::default()),
dirty_on_viewport_size_change: AtomicBool::new(false),
disabled: AtomicBool::new(false),
})
}
--- a/servo/ports/geckolib/stylesheet_loader.rs
+++ b/servo/ports/geckolib/stylesheet_loader.rs
@@ -1,60 +1,48 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
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);
impl StylesheetLoader {
pub fn new(loader: *mut Loader, parent: *mut ServoStyleSheet) -> Self {
StylesheetLoader(loader, parent)
}
}
impl StyleStylesheetLoader for StylesheetLoader {
fn request_stylesheet(
&self,
- media: MediaList,
- make_import: &mut FnMut(MediaList) -> ImportRule,
+ media: Arc<Locked<MediaList>>,
+ make_import: &mut FnMut(Arc<Locked<MediaList>>) -> ImportRule,
make_arc: &mut FnMut(ImportRule) -> Arc<Locked<ImportRule>>,
) -> Arc<Locked<ImportRule>> {
- // 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<Url> pointer inside will also move,
// but the Url it points to or the allocating backing the String inside that Url won’t,
// so this raw pointer will still be valid.
let (spec_bytes, spec_len): (*const u8, usize) = import.url.as_slice_components();
let base_url_data = import.url.extra_data.get();
unsafe {
Gecko_LoadStyleSheet(self.0,
self.1,
Stylesheet::arc_as_borrowed(&import.stylesheet),
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)
}
}