Skip to content

Commit

Permalink
Don't resolve URLs at parse time for Stylo.
Browse files Browse the repository at this point in the history
It's a bit unfortunate the use separate implementations of SpecifiedUrl for Servo
and Gecko, but they're different enough at this point that I don't think it really
makes sense to try to share everything. Splitting them out has some nice
simplifications as well.

I recognize that there's still some potential correctness issues for Servo using
the resolved URI in various places where the original URI may be the right thing,
but I've got too much on my plate to look into that for now.

MozReview-Commit-ID: BeDu93TQ4Ow
  • Loading branch information
bholley committed Mar 23, 2017
1 parent 16e0404 commit 63e8367
Show file tree
Hide file tree
Showing 14 changed files with 278 additions and 244 deletions.
2 changes: 1 addition & 1 deletion components/script/dom/element.rs
Expand Up @@ -429,7 +429,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
PropertyDeclaration::BackgroundImage(
background_image::SpecifiedValue(vec![
background_image::single_value::SpecifiedValue(Some(
specified::Image::for_cascade(url.into(), specified::url::UrlExtraData { })
specified::Image::for_cascade(url.into())
))
]))));
}
Expand Down
2 changes: 2 additions & 0 deletions components/style/build_gecko.rs
Expand Up @@ -353,6 +353,7 @@ mod bindings {
"nsCursorImage",
"nsFont",
"nsIAtom",
"nsIURI",
"nsMainThreadPtrHandle",
"nsMainThreadPtrHolder",
"nsMargin",
Expand Down Expand Up @@ -613,6 +614,7 @@ mod bindings {
"nsCursorImage",
"nsFont",
"nsIAtom",
"nsIURI",
"nsMediaFeature",
"nsRestyleHint",
"nsStyleBackground",
Expand Down
1 change: 1 addition & 0 deletions components/style/gecko/mod.rs
Expand Up @@ -17,5 +17,6 @@ pub mod selector_parser;
pub mod snapshot;
pub mod snapshot_helpers;
pub mod traversal;
pub mod url;
pub mod values;
pub mod wrapper;
104 changes: 104 additions & 0 deletions components/style/gecko/url.rs
@@ -0,0 +1,104 @@
/* 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/. */

//! Common handling for the specified value CSS url() values.

use cssparser::CssStringWriter;
use gecko_bindings::structs::ServoBundledURI;
use gecko_bindings::sugar::refptr::{GeckoArcPrincipal, GeckoArcURI};
use parser::ParserContext;
use std::borrow::Cow;
use std::fmt::{self, Write};
use std::sync::Arc;
use style_traits::ToCss;

/// A specified url() value for gecko. Gecko does not eagerly resolve SpecifiedUrls.
#[derive(Clone, Debug, PartialEq)]
pub struct SpecifiedUrl {
/// The URL in unresolved string form.
///
/// Refcounted since cloning this should be cheap and data: uris can be
/// really large.
serialization: Arc<String>,

/// The base URI.
pub base: GeckoArcURI,
/// The referrer.
pub referrer: GeckoArcURI,
/// The principal that originated this URI.
pub principal: GeckoArcPrincipal,
}

impl SpecifiedUrl {
/// Try to parse a URL from a string value that is a valid CSS token for a
/// URL.
///
/// Returns `Err` in the case that extra_data is incomplete.
pub fn parse_from_string<'a>(url: Cow<'a, str>,
context: &ParserContext)
-> Result<Self, ()> {
let extra = &context.extra_data;
if extra.base.is_none() || extra.referrer.is_none() || extra.principal.is_none() {
// FIXME(heycam) should ensure we always have a principal, etc.,
// when parsing style attributes and re-parsing due to CSS
// Variables.
warn!("stylo: skipping declaration without ParserContextExtraData");
return Err(())
}

Ok(SpecifiedUrl {
serialization: Arc::new(url.into_owned()),
base: extra.base.as_ref().unwrap().clone(),
referrer: extra.referrer.as_ref().unwrap().clone(),
principal: extra.principal.as_ref().unwrap().clone(),
})
}

/// Returns true if the URL is definitely invalid. We don't eagerly resolve
/// URLs in gecko, so we just return false here.
/// use its |resolved| status.
pub fn is_invalid(&self) -> bool {
false
}

/// Returns true if this URL looks like a fragment.
/// See https://drafts.csswg.org/css-values/#local-urls
pub fn is_fragment(&self) -> bool {
self.as_str().chars().next().map_or(false, |c| c == '#')
}

/// Return the resolved url as string, or the empty string if it's invalid.
///
/// FIXME(bholley): This returns the unresolved URL while the servo version
/// returns the resolved URL.
pub fn as_str(&self) -> &str {
&*self.serialization
}

/// Little helper for Gecko's ffi.
pub fn as_slice_components(&self) -> (*const u8, usize) {
(self.serialization.as_str().as_ptr(), self.serialization.as_str().len())
}

/// Create a bundled URI suitable for sending to Gecko
/// to be constructed into a css::URLValue
pub fn for_ffi(&self) -> ServoBundledURI {
let (ptr, len) = self.as_slice_components();
ServoBundledURI {
mURLString: ptr,
mURLStringLength: len as u32,
mBaseURI: self.base.get(),
mReferrer: self.referrer.get(),
mPrincipal: self.principal.get(),
}
}
}

impl ToCss for SpecifiedUrl {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(dest.write_str("url(\""));
try!(CssStringWriter::new(dest).write_str(&*self.serialization));
dest.write_str("\")")
}
}
6 changes: 4 additions & 2 deletions components/style/gecko_bindings/bindings.rs
Expand Up @@ -40,6 +40,7 @@ use gecko_bindings::structs::nsChangeHint;
use gecko_bindings::structs::nsCursorImage;
use gecko_bindings::structs::nsFont;
use gecko_bindings::structs::nsIAtom;
use gecko_bindings::structs::nsIURI;
use gecko_bindings::structs::nsMediaFeature;
use gecko_bindings::structs::nsRestyleHint;
use gecko_bindings::structs::nsStyleBackground;
Expand Down Expand Up @@ -437,8 +438,9 @@ extern "C" {
pub fn Gecko_LoadStyleSheet(loader: *mut Loader,
parent: *mut ServoStyleSheet,
import_rule: RawServoImportRuleBorrowed,
url_bytes: *const u8, url_length: u32,
media_bytes: *const u8, media_length: u32);
base_uri: *mut nsIURI, url_bytes: *const u8,
url_length: u32, media_bytes: *const u8,
media_length: u32);
}
extern "C" {
pub fn Gecko_MaybeCreateStyleChildrenIterator(node: RawGeckoNodeBorrowed)
Expand Down
7 changes: 1 addition & 6 deletions components/style/properties/longhand/svg.mako.rs
Expand Up @@ -259,12 +259,7 @@ ${helpers.single_keyword("mask-composite",
let image = try!(Image::parse(context, input));
match image {
Image::Url(url_value) => {
let has_valid_url = match url_value.url() {
Some(url) => url.fragment().is_some(),
None => false,
};

if has_valid_url {
if url_value.is_fragment() {
Ok(SpecifiedValue::Url(url_value))
} else {
Ok(SpecifiedValue::Image(Image::Url(url_value)))
Expand Down
1 change: 1 addition & 0 deletions components/style/servo/mod.rs
Expand Up @@ -9,3 +9,4 @@
pub mod media_queries;
pub mod restyle_damage;
pub mod selector_parser;
pub mod url;
127 changes: 127 additions & 0 deletions components/style/servo/url.rs
@@ -0,0 +1,127 @@
/* 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/. */

//! Common handling for the specified value CSS url() values.

use cssparser::CssStringWriter;
use parser::ParserContext;
use servo_url::ServoUrl;
use std::borrow::Cow;
use std::fmt::{self, Write};
use std::sync::Arc;
use style_traits::ToCss;

/// A specified url() value for servo.
///
/// Servo eagerly resolves SpecifiedUrls, which it can then take advantage of
/// when computing values. In contrast, Gecko uses a different URL backend, so
/// eagerly resolving with rust-url would be duplicated work.
///
/// However, this approach is still not necessarily optimal: See
/// https://bugzilla.mozilla.org/show_bug.cgi?id=1347435#c6
#[derive(Clone, Debug, HeapSizeOf, Serialize, Deserialize)]
pub struct SpecifiedUrl {
/// The original URI. This might be optional since we may insert computed
/// values of images into the cascade directly, and we don't bother to
/// convert their serialization.
///
/// Refcounted since cloning this should be cheap and data: uris can be
/// really large.
original: Option<Arc<String>>,

/// The resolved value for the url, if valid.
resolved: Option<ServoUrl>,
}

impl SpecifiedUrl {
/// Try to parse a URL from a string value that is a valid CSS token for a
/// URL. Never fails - the API is only fallible to be compatible with the
/// gecko version.
pub fn parse_from_string<'a>(url: Cow<'a, str>,
context: &ParserContext)
-> Result<Self, ()> {
let serialization = Arc::new(url.into_owned());
let resolved = context.base_url.join(&serialization).ok();
Ok(SpecifiedUrl {
original: Some(serialization),
resolved: resolved,
})
}

/// Returns true if the URL is definitely invalid. For Servo URLs, we can
/// use its |resolved| status.
pub fn is_invalid(&self) -> bool {
self.resolved.is_none()
}

/// Returns true if this URL looks like a fragment.
/// See https://drafts.csswg.org/css-values/#local-urls
///
/// Since Servo currently stores resolved URLs, this is hard to implement. We
/// either need to change servo to lazily resolve (like Gecko), or note this
/// information in the tokenizer.
pub fn is_fragment(&self) -> bool {
error!("Can't determine whether the url is a fragment.");
false
}

/// Returns the resolved url if it was valid.
pub fn url(&self) -> Option<&ServoUrl> {
self.resolved.as_ref()
}

/// Return the resolved url as string, or the empty string if it's invalid.
///
/// TODO(emilio): Should we return the original one if needed?
pub fn as_str(&self) -> &str {
match self.resolved {
Some(ref url) => url.as_str(),
None => "",
}
}

/// Creates an already specified url value from an already resolved URL
/// for insertion in the cascade.
pub fn for_cascade(url: ServoUrl) -> Self {
SpecifiedUrl {
original: None,
resolved: Some(url),
}
}

/// Gets a new url from a string for unit tests.
pub fn new_for_testing(url: &str) -> Self {
SpecifiedUrl {
original: Some(Arc::new(url.into())),
resolved: ServoUrl::parse(url).ok(),
}
}
}

impl PartialEq for SpecifiedUrl {
fn eq(&self, other: &Self) -> bool {
// TODO(emilio): maybe we care about equality of the specified values if
// present? Seems not.
self.resolved == other.resolved
}
}

impl ToCss for SpecifiedUrl {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(dest.write_str("url(\""));
let string = match self.original {
Some(ref original) => &**original,
None => match self.resolved {
Some(ref url) => url.as_str(),
// This can only happen if the url wasn't specified by the
// user *and* it's an invalid url that has been transformed
// back to specified value via the "uncompute" functionality.
None => "about:invalid",
}
};

try!(CssStringWriter::new(dest).write_str(string));
dest.write_str("\")")
}
}
3 changes: 1 addition & 2 deletions components/style/stylesheets.rs
Expand Up @@ -839,8 +839,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
let media = parse_media_query_list(input);

let noop_loader = NoOpLoader;
let is_valid_url = specified_url.url().is_some();
let loader = if is_valid_url {
let loader = if !specified_url.is_invalid() {
self.loader.expect("Expected a stylesheet loader for @import")
} else {
&noop_loader
Expand Down
2 changes: 1 addition & 1 deletion components/style/values/computed/mod.rs
Expand Up @@ -21,7 +21,7 @@ pub use super::{Auto, Either, None_};
#[cfg(feature = "gecko")]
pub use super::specified::{AlignItems, AlignJustifyContent, AlignJustifySelf, JustifyItems};
pub use super::specified::{Angle, BorderStyle, GridLine, Time, UrlOrNone};
pub use super::specified::url::{SpecifiedUrl, UrlExtraData};
pub use super::specified::url::SpecifiedUrl;
pub use self::length::{CalcLengthOrPercentage, Length, LengthOrNumber, LengthOrPercentage, LengthOrPercentageOrAuto};
pub use self::length::{LengthOrPercentageOrAutoOrContent, LengthOrPercentageOrNone, LengthOrNone};
pub use self::length::{MaxLength, MinLength};
Expand Down
8 changes: 5 additions & 3 deletions components/style/values/specified/image.rs
Expand Up @@ -9,12 +9,13 @@

use cssparser::Parser;
use parser::{Parse, ParserContext};
#[cfg(feature = "servo")]
use servo_url::ServoUrl;
use std::fmt;
use style_traits::ToCss;
use values::specified::{Angle, CSSColor, Length, LengthOrPercentage};
use values::specified::position::Position;
use values::specified::url::{SpecifiedUrl, UrlExtraData};
use values::specified::url::SpecifiedUrl;

/// Specified values for an image according to CSS-IMAGES.
/// https://drafts.csswg.org/css-images/#image-values
Expand Down Expand Up @@ -48,8 +49,9 @@ impl Image {

/// Creates an already specified image value from an already resolved URL
/// for insertion in the cascade.
pub fn for_cascade(url: ServoUrl, extra_data: UrlExtraData) -> Self {
Image::Url(SpecifiedUrl::for_cascade(url, extra_data))
#[cfg(feature = "servo")]
pub fn for_cascade(url: ServoUrl) -> Self {
Image::Url(SpecifiedUrl::for_cascade(url))
}
}

Expand Down
28 changes: 27 additions & 1 deletion components/style/values/specified/mod.rs
Expand Up @@ -42,7 +42,33 @@ pub mod grid;
pub mod image;
pub mod length;
pub mod position;
pub mod url;

/// Common handling for the specified value CSS url() values.
pub mod url {
use cssparser::Parser;
use parser::{Parse, ParserContext};
use values::HasViewportPercentage;
use values::computed::ComputedValueAsSpecified;

#[cfg(feature = "servo")]
pub use ::servo::url::*;
#[cfg(feature = "gecko")]
pub use ::gecko::url::*;

impl Parse for SpecifiedUrl {
fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
let url = try!(input.expect_url());
Self::parse_from_string(url, context)
}
}

impl Eq for SpecifiedUrl {}

// TODO(emilio): Maybe consider ComputedUrl to save a word in style structs?
impl ComputedValueAsSpecified for SpecifiedUrl {}

no_viewport_percentage!(SpecifiedUrl);
}

no_viewport_percentage!(i32); // For PropertyDeclaration::Order

Expand Down

0 comments on commit 63e8367

Please sign in to comment.