Bug 1461858 part 1 - Make creating CssUrl infallible. r?emilio
There were a check in CssUrl::parse_from_string for extra data, which
was removed as part of servo/servo#16241, so it never fails now.
CssUrl::from_url_value_data doesn't seem to need Result from the very
beginning. It is unclear why it was made that way.
MozReview-Commit-ID: LXzKlZ6wPYW
--- a/servo/components/style/gecko/conversions.rs
+++ b/servo/components/style/gecko/conversions.rs
@@ -424,17 +424,16 @@ impl nsStyleImage {
},
_ => panic!("Unexpected image type"),
}
}
unsafe fn get_image_url(self: &nsStyleImage) -> ComputedImageUrl {
let url_value = bindings::Gecko_GetURLValue(self);
ComputedImageUrl::from_url_value_data(url_value.as_ref().unwrap())
- .expect("Could not convert to ComputedUrl")
}
unsafe fn get_gradient(self: &nsStyleImage) -> Box<Gradient> {
use gecko::values::convert_nscolor_to_rgba;
use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_CORNER as CLOSEST_CORNER;
use self::structs::NS_STYLE_GRADIENT_SIZE_CLOSEST_SIDE as CLOSEST_SIDE;
use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER as FARTHEST_CORNER;
use self::structs::NS_STYLE_GRADIENT_SIZE_FARTHEST_SIDE as FARTHEST_SIDE;
@@ -674,17 +673,17 @@ pub mod basic_shape {
}
impl<'a> From<&'a StyleShapeSource> for ClippingShape {
fn from(other: &'a StyleShapeSource) -> Self {
match other.mType {
StyleShapeSourceType::URL => unsafe {
let shape_image = &*other.mShapeImage.mPtr;
let other_url = &(**shape_image.__bindgen_anon_1.mURLValue.as_ref());
- let url = ComputedUrl::from_url_value_data(&other_url._base).unwrap();
+ let url = ComputedUrl::from_url_value_data(&other_url._base);
ShapeSource::ImageOrUrl(url)
},
StyleShapeSourceType::Image => {
unreachable!("ClippingShape doesn't support Image!");
},
_ => other
.into_shape_source()
.expect("Couldn't convert to StyleSource!"),
--- a/servo/components/style/gecko/url.rs
+++ b/servo/components/style/gecko/url.rs
@@ -33,43 +33,38 @@ pub struct CssUrl {
/// The URL extra data.
#[css(skip)]
pub extra_data: RefPtr<URLExtraData>,
}
impl CssUrl {
/// 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: String,
- context: &ParserContext,
- ) -> Result<Self, ParseError<'a>> {
- Ok(CssUrl {
+ pub fn parse_from_string(url: String, context: &ParserContext) -> Self {
+ CssUrl {
serialization: Arc::new(url),
extra_data: context.url_data.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
}
/// Convert from URLValueData to SpecifiedUrl.
- unsafe fn from_url_value_data(url: &URLValueData) -> Result<Self, ()> {
+ unsafe fn from_url_value_data(url: &URLValueData) -> Self {
let arc_type =
&url.mString as *const _ as *const RawOffsetArc<String>;
- Ok(CssUrl {
+ CssUrl {
serialization: Arc::from_raw_offset((*arc_type).clone()),
extra_data: url.mExtraData.to_safe(),
- })
+ }
}
/// 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 == '#')
}
@@ -99,17 +94,17 @@ impl CssUrl {
}
impl Parse for CssUrl {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
let url = input.expect_url()?;
- Self::parse_from_string(url.as_ref().to_owned(), context)
+ Ok(Self::parse_from_string(url.as_ref().to_owned(), context))
}
}
impl Eq for CssUrl {}
impl MallocSizeOf for CssUrl {
fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
// XXX: measure `serialization` once bug 1397971 lands
@@ -183,21 +178,18 @@ pub struct SpecifiedImageUrl {
/// Gecko's ImageValue so that we can reuse it while rematching a
/// property with this specified value.
#[css(skip)]
pub image_value: RefPtr<ImageValue>,
}
impl SpecifiedImageUrl {
/// Parse a URL from a string value. See SpecifiedUrl::parse_from_string.
- pub fn parse_from_string<'a>(
- url: String,
- context: &ParserContext,
- ) -> Result<Self, ParseError<'a>> {
- CssUrl::parse_from_string(url, context).map(Self::from_css_url)
+ pub fn parse_from_string(url: String, context: &ParserContext) -> Self {
+ Self::from_css_url(CssUrl::parse_from_string(url, context))
}
fn from_css_url(url: CssUrl) -> Self {
let image_value = unsafe {
let ptr = bindings::Gecko_ImageValue_Create(url.for_ffi());
// We do not expect Gecko_ImageValue_Create returns null.
debug_assert!(!ptr.is_null());
RefPtr::from_addrefed(ptr)
@@ -291,20 +283,20 @@ impl ToCss for ComputedUrl {
W: Write
{
serialize_computed_url(&self.0.url_value._base, dest)
}
}
impl ComputedUrl {
/// Convert from URLValueData to ComputedUrl.
- pub unsafe fn from_url_value_data(url: &URLValueData) -> Result<Self, ()> {
- Ok(ComputedUrl(
- SpecifiedUrl::from_css_url(CssUrl::from_url_value_data(url)?)
- ))
+ pub unsafe fn from_url_value_data(url: &URLValueData) -> Self {
+ ComputedUrl(
+ SpecifiedUrl::from_css_url(CssUrl::from_url_value_data(url))
+ )
}
}
/// The computed value of a CSS `url()` for image.
#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq)]
pub struct ComputedImageUrl(pub SpecifiedImageUrl);
impl ToCss for ComputedImageUrl {
@@ -313,25 +305,25 @@ impl ToCss for ComputedImageUrl {
W: Write
{
serialize_computed_url(&self.0.image_value._base, dest)
}
}
impl ComputedImageUrl {
/// Convert from URLValueData to SpecifiedUrl.
- pub unsafe fn from_url_value_data(url: &URLValueData) -> Result<Self, ()> {
- Ok(ComputedImageUrl(
- SpecifiedImageUrl::from_css_url(CssUrl::from_url_value_data(url)?)
- ))
+ pub unsafe fn from_url_value_data(url: &URLValueData) -> Self {
+ ComputedImageUrl(
+ SpecifiedImageUrl::from_css_url(CssUrl::from_url_value_data(url))
+ )
}
/// Convert from nsStyleImageReques to ComputedImageUrl.
pub unsafe fn from_image_request(image_request: &nsStyleImageRequest) -> Result<Self, ()> {
if image_request.mImageValue.mRawPtr.is_null() {
return Err(());
}
let image_value = image_request.mImageValue.mRawPtr.as_ref().unwrap();
let url_value_data = &image_value._base;
- Self::from_url_value_data(url_value_data)
+ Ok(Self::from_url_value_data(url_value_data))
}
}
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -757,17 +757,17 @@ def set_gecko_property(ffi_name, expr):
nsStyleSVGPaintType::eStyleSVGPaintType_None => SVGPaintKind::None,
nsStyleSVGPaintType::eStyleSVGPaintType_ContextFill => SVGPaintKind::ContextFill,
nsStyleSVGPaintType::eStyleSVGPaintType_ContextStroke => SVGPaintKind::ContextStroke,
nsStyleSVGPaintType::eStyleSVGPaintType_Server => {
unsafe {
SVGPaintKind::PaintServer(
ComputedUrl::from_url_value_data(
&(**paint.mPaint.mPaintServer.as_ref())._base
- ).unwrap()
+ )
)
}
}
nsStyleSVGPaintType::eStyleSVGPaintType_Color => {
unsafe { SVGPaintKind::Color(convert_nscolor_to_rgba(*paint.mPaint.mColor.as_ref())) }
}
};
SVGPaint {
@@ -966,17 +966,16 @@ def set_gecko_property(ffi_name, expr):
if self.gecko.${gecko_ffi_name}.mRawPtr.is_null() {
return UrlOrNone::none()
}
unsafe {
let gecko_url_value = &*self.gecko.${gecko_ffi_name}.mRawPtr;
UrlOrNone::Url(
ComputedUrl::from_url_value_data(&gecko_url_value._base)
- .expect("${gecko_ffi_name} could not convert to ComputedUrl")
)
}
}
</%def>
<%
transform_functions = [
("Matrix3D", "matrix3d", ["number"] * 16),
@@ -4550,17 +4549,17 @@ fn static_assert() {
Filter::DropShadow(
(**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow(),
)
});
},
NS_STYLE_FILTER_URL => {
filters.push(unsafe {
Filter::Url(
- ComputedUrl::from_url_value_data(&(**filter.__bindgen_anon_1.mURL.as_ref())._base).unwrap()
+ ComputedUrl::from_url_value_data(&(**filter.__bindgen_anon_1.mURL.as_ref())._base)
)
});
}
_ => {},
}
}
longhands::filter::computed_value::T(filters)
}
--- a/servo/components/style/servo/url.rs
+++ b/servo/components/style/servo/url.rs
@@ -35,28 +35,24 @@ pub struct CssUrl {
original: Option<Arc<String>>,
/// The resolved value for the url, if valid.
resolved: Option<ServoUrl>,
}
impl CssUrl {
/// 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: String,
- context: &ParserContext,
- ) -> Result<Self, ParseError<'a>> {
+ /// URL.
+ pub fn parse_from_string(url: String, context: &ParserContext) -> Self {
let serialization = Arc::new(url);
let resolved = context.url_data.join(&serialization).ok();
- Ok(CssUrl {
+ CssUrl {
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()
}
@@ -105,17 +101,17 @@ impl CssUrl {
}
impl Parse for CssUrl {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
let url = input.expect_url()?;
- Self::parse_from_string(url.as_ref().to_owned(), context)
+ Ok(Self::parse_from_string(url.as_ref().to_owned(), context))
}
}
impl PartialEq for CssUrl {
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
--- a/servo/components/style/stylesheets/rule_parser.rs
+++ b/servo/components/style/stylesheets/rule_parser.rs
@@ -153,17 +153,17 @@ impl<'a, 'i, R: ParseErrorReporter> AtRu
"import" => {
if self.state > State::Imports {
// "@import must be before any rule but @charset"
self.had_hierarchy_error = true;
return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportRule))
}
let url_string = input.expect_url_or_string()?.as_ref().to_owned();
- let url = CssUrl::parse_from_string(url_string, &self.context)?;
+ let url = CssUrl::parse_from_string(url_string, &self.context);
let media = parse_media_query_list(&self.context, input,
self.error_context.error_reporter);
let media = Arc::new(self.shared_lock.wrap(media));
let prelude = AtRuleNonBlockPrelude::Import(url, media, location);
return Ok(AtRuleType::WithoutBlock(prelude));
},
--- a/servo/components/style/values/specified/image.rs
+++ b/servo/components/style/values/specified/image.rs
@@ -985,17 +985,17 @@ impl Parse for PaintWorklet {
impl Parse for MozImageRect {
fn parse<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
input.try(|i| i.expect_function_matching("-moz-image-rect"))?;
input.parse_nested_block(|i| {
let string = i.expect_url_or_string()?;
- let url = SpecifiedImageUrl::parse_from_string(string.as_ref().to_owned(), context)?;
+ let url = SpecifiedImageUrl::parse_from_string(string.as_ref().to_owned(), context);
i.expect_comma()?;
let top = NumberOrPercentage::parse_non_negative(context, i)?;
i.expect_comma()?;
let right = NumberOrPercentage::parse_non_negative(context, i)?;
i.expect_comma()?;
let bottom = NumberOrPercentage::parse_non_negative(context, i)?;
i.expect_comma()?;
let left = NumberOrPercentage::parse_non_negative(context, i)?;
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -4166,24 +4166,23 @@ pub extern "C" fn Servo_DeclarationBlock
let string = unsafe { (*value).to_string() };
let context = ParserContext::new(
Origin::Author,
url_data,
Some(CssRuleType::Style),
ParsingMode::DEFAULT,
QuirksMode::NoQuirks,
);
- if let Ok(url) = SpecifiedImageUrl::parse_from_string(string.into(), &context) {
- let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(
- vec![Either::Second(Image::Url(url))]
- ));
- write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
- decls.push(decl, Importance::Normal, DeclarationSource::CssOm);
- })
- }
+ let url = SpecifiedImageUrl::parse_from_string(string.into(), &context);
+ let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(
+ vec![Either::Second(Image::Url(url))]
+ ));
+ write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
+ decls.push(decl, Importance::Normal, DeclarationSource::CssOm);
+ });
}
#[no_mangle]
pub extern "C" fn Servo_DeclarationBlock_SetTextDecorationColorOverride(
declarations: RawServoDeclarationBlockBorrowed,
) {
use style::properties::PropertyDeclaration;
use style::values::specified::text::TextDecorationLine;