diff options
24 files changed, 698 insertions, 334 deletions
diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs index f4a929e23f8..f2f9fa00602 100644 --- a/components/servo_arc/lib.rs +++ b/components/servo_arc/lib.rs @@ -21,7 +21,7 @@ //! //! [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1360883 -// The semantics of `Arc` are alread documented in the Rust docs, so we don't +// The semantics of `Arc` are already documented in the Rust docs, so we don't // duplicate those here. #![allow(missing_docs)] @@ -86,10 +86,14 @@ const STATIC_REFCOUNT: usize = usize::MAX; /// See the documentation for [`Arc`] in the standard library. Unlike the /// standard library `Arc`, this `Arc` does not support weak reference counting. /// +/// See the discussion in https://github.com/rust-lang/rust/pull/60594 for the +/// usage of PhantomData. +/// /// [`Arc`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html #[repr(C)] pub struct Arc<T: ?Sized> { p: ptr::NonNull<ArcInner<T>>, + phantom: PhantomData<T>, } /// An `Arc` that is known to be uniquely owned @@ -164,11 +168,12 @@ impl<T> Arc<T> { pub fn new(data: T) -> Self { let x = Box::new(ArcInner { count: atomic::AtomicUsize::new(1), - data: data, + data, }); unsafe { Arc { p: ptr::NonNull::new_unchecked(Box::into_raw(x)), + phantom: PhantomData, } } } @@ -198,6 +203,7 @@ impl<T> Arc<T> { let ptr = (ptr as *const u8).offset(-offset_of!(ArcInner<T>, data)); Arc { p: ptr::NonNull::new_unchecked(ptr as *mut ArcInner<T>), + phantom: PhantomData, } } @@ -224,6 +230,7 @@ impl<T> Arc<T> { Arc { p: ptr::NonNull::new_unchecked(ptr), + phantom: PhantomData, } } @@ -301,6 +308,9 @@ impl<T: ?Sized> Arc<T> { impl<T: ?Sized> Clone for Arc<T> { #[inline] fn clone(&self) -> Self { + // NOTE(emilio): If you change anything here, make sure that the + // implementation in layout/style/ServoStyleConstsInlines.h matches! + // // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since // `count` never changes between STATIC_REFCOUNT and other values. if self.inner().count.load(Relaxed) != STATIC_REFCOUNT { @@ -334,6 +344,7 @@ impl<T: ?Sized> Clone for Arc<T> { unsafe { Arc { p: ptr::NonNull::new_unchecked(self.ptr()), + phantom: PhantomData, } } } @@ -408,6 +419,9 @@ impl<T: ?Sized> Arc<T> { impl<T: ?Sized> Drop for Arc<T> { #[inline] fn drop(&mut self) { + // NOTE(emilio): If you change anything here, make sure that the + // implementation in layout/style/ServoStyleConstsInlines.h matches! + // // Using a relaxed ordering to check for STATIC_REFCOUNT is safe, since // `count` never changes between STATIC_REFCOUNT and other values. if self.inner().count.load(Relaxed) == STATIC_REFCOUNT { @@ -563,6 +577,7 @@ impl<T: Serialize> Serialize for Arc<T> { /// Structure to allow Arc-managing some fixed-sized data and a variably-sized /// slice in a single allocation. #[derive(Debug, Eq, PartialEq, PartialOrd)] +#[repr(C)] pub struct HeaderSlice<H, T: ?Sized> { /// The fixed-sized data. pub header: H, @@ -594,7 +609,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> { F: FnOnce(Layout) -> *mut u8, I: Iterator<Item = T> + ExactSizeIterator, { - use std::mem::size_of; + use std::mem::{align_of, size_of}; assert_ne!(size_of::<T>(), 0, "Need to think about ZST"); // Compute the required size for the allocation. @@ -602,7 +617,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> { let size = { // First, determine the alignment of a hypothetical pointer to a // HeaderSlice. - let fake_slice_ptr_align: usize = mem::align_of::<ArcInner<HeaderSlice<H, [T; 1]>>>(); + let fake_slice_ptr_align: usize = mem::align_of::<ArcInner<HeaderSlice<H, [T; 0]>>>(); // Next, synthesize a totally garbage (but properly aligned) pointer // to a sequence of T. @@ -659,23 +674,28 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> { }; ptr::write(&mut ((*ptr).count), count); ptr::write(&mut ((*ptr).data.header), header); - let mut current: *mut T = &mut (*ptr).data.slice[0]; - for _ in 0..num_items { - ptr::write( - current, - items - .next() - .expect("ExactSizeIterator over-reported length"), + if num_items != 0 { + let mut current: *mut T = &mut (*ptr).data.slice[0]; + for _ in 0..num_items { + ptr::write( + current, + items + .next() + .expect("ExactSizeIterator over-reported length"), + ); + current = current.offset(1); + } + // We should have consumed the buffer exactly, maybe accounting + // for some padding from the alignment. + debug_assert!( + (buffer.offset(size as isize) as usize - current as *mut u8 as usize) < + align_of::<Self>() ); - current = current.offset(1); } assert!( items.next().is_none(), "ExactSizeIterator under-reported length" ); - - // We should have consumed the buffer exactly. - debug_assert_eq!(current as *mut u8, buffer.offset(size as isize)); } // Return the fat Arc. @@ -687,6 +707,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> { unsafe { Arc { p: ptr::NonNull::new_unchecked(ptr), + phantom: PhantomData, } } } @@ -732,6 +753,7 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> { /// Header data with an inline length. Consumers that use HeaderWithLength as the /// Header type in HeaderSlice can take advantage of ThinArc. #[derive(Debug, Eq, PartialEq, PartialOrd)] +#[repr(C)] pub struct HeaderWithLength<H> { /// The fixed-sized data. pub header: H, @@ -763,11 +785,20 @@ type HeaderSliceWithLength<H, T> = HeaderSlice<HeaderWithLength<H>, T>; /// have a thin pointer instead, perhaps for FFI compatibility /// or space efficiency. /// +/// Note that we use `[T; 0]` in order to have the right alignment for `T`. +/// /// `ThinArc` solves this by storing the length in the allocation itself, /// via `HeaderSliceWithLength`. #[repr(C)] pub struct ThinArc<H, T> { - ptr: *mut ArcInner<HeaderSliceWithLength<H, [T; 1]>>, + ptr: ptr::NonNull<ArcInner<HeaderSliceWithLength<H, [T; 0]>>>, + phantom: PhantomData<(H, T)>, +} + +impl<H: fmt::Debug, T: fmt::Debug> fmt::Debug for ThinArc<H, T> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(self.deref(), f) + } } unsafe impl<H: Sync + Send, T: Sync + Send> Send for ThinArc<H, T> {} @@ -777,7 +808,7 @@ unsafe impl<H: Sync + Send, T: Sync + Send> Sync for ThinArc<H, T> {} // // See the comment around the analogous operation in from_header_and_iter. fn thin_to_thick<H, T>( - thin: *mut ArcInner<HeaderSliceWithLength<H, [T; 1]>>, + thin: *mut ArcInner<HeaderSliceWithLength<H, [T; 0]>>, ) -> *mut ArcInner<HeaderSliceWithLength<H, [T]>> { let len = unsafe { (*thin).data.header.length }; let fake_slice: *mut [T] = unsafe { slice::from_raw_parts_mut(thin as *mut T, len) }; @@ -796,7 +827,8 @@ impl<H, T> ThinArc<H, T> { // Synthesize transient Arc, which never touches the refcount of the ArcInner. let transient = unsafe { NoDrop::new(Arc { - p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr)), + p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr.as_ptr())), + phantom: PhantomData, }) }; @@ -841,8 +873,12 @@ impl<H, T> ThinArc<H, T> { } /// Returns the address on the heap of the ThinArc itself -- not the T - /// within it -- for memory reporting. - /// + /// within it -- for memory reporting, and bindings. + #[inline] + pub fn ptr(&self) -> *const c_void { + self.ptr.as_ptr() as *const ArcInner<T> as *const c_void + } + /// If this is a static ThinArc, this returns null. #[inline] pub fn heap_ptr(&self) -> *const c_void { @@ -851,7 +887,7 @@ impl<H, T> ThinArc<H, T> { if is_static { ptr::null() } else { - self.ptr as *const ArcInner<T> as *const c_void + self.ptr() } } } @@ -861,7 +897,7 @@ impl<H, T> Deref for ThinArc<H, T> { #[inline] fn deref(&self) -> &Self::Target { - unsafe { &(*thin_to_thick(self.ptr)).data } + unsafe { &(*thin_to_thick(self.ptr.as_ptr())).data } } } @@ -875,7 +911,10 @@ impl<H, T> Clone for ThinArc<H, T> { impl<H, T> Drop for ThinArc<H, T> { #[inline] fn drop(&mut self) { - let _ = Arc::from_thin(ThinArc { ptr: self.ptr }); + let _ = Arc::from_thin(ThinArc { + ptr: self.ptr, + phantom: PhantomData, + }); } } @@ -893,7 +932,12 @@ impl<H, T> Arc<HeaderSliceWithLength<H, [T]>> { mem::forget(a); let thin_ptr = fat_ptr as *mut [usize] as *mut usize; ThinArc { - ptr: thin_ptr as *mut ArcInner<HeaderSliceWithLength<H, [T; 1]>>, + ptr: unsafe { + ptr::NonNull::new_unchecked( + thin_ptr as *mut ArcInner<HeaderSliceWithLength<H, [T; 0]>>, + ) + }, + phantom: PhantomData, } } @@ -901,11 +945,12 @@ impl<H, T> Arc<HeaderSliceWithLength<H, [T]>> { /// is not modified. #[inline] pub fn from_thin(a: ThinArc<H, T>) -> Self { - let ptr = thin_to_thick(a.ptr); + let ptr = thin_to_thick(a.ptr.as_ptr()); mem::forget(a); unsafe { Arc { p: ptr::NonNull::new_unchecked(ptr), + phantom: PhantomData, } } } @@ -1306,6 +1351,34 @@ mod tests { } #[test] + fn empty_thin() { + let header = HeaderWithLength::new(100u32, 0); + let x = Arc::from_header_and_iter(header, std::iter::empty::<i32>()); + let y = Arc::into_thin(x.clone()); + assert_eq!(y.header.header, 100); + assert!(y.slice.is_empty()); + assert_eq!(x.header.header, 100); + assert!(x.slice.is_empty()); + } + + #[test] + fn thin_assert_padding() { + #[derive(Clone, Default)] + #[repr(C)] + struct Padded { + i: u16, + } + + // The header will have more alignment than `Padded` + let header = HeaderWithLength::new(0i32, 2); + let items = vec![Padded { i: 0xdead }, Padded { i: 0xbeef }]; + let a = ThinArc::from_header_and_iter(header, items.into_iter()); + assert_eq!(a.slice.len(), 2); + assert_eq!(a.slice[0].i, 0xdead); + assert_eq!(a.slice[1].i, 0xbeef); + } + + #[test] fn slices_and_thin() { let mut canary = atomic::AtomicUsize::new(0); let c = Canary(&mut canary as *mut atomic::AtomicUsize); diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 4cd5722bdb0..43082a7c04a 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -532,28 +532,15 @@ impl nsStyleImage { pub mod basic_shape { //! Conversions from and to CSS shape representations. - - use crate::gecko::values::GeckoStyleCoordConvertible; - use crate::gecko_bindings::structs::nsStyleCoord; - use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType}; use crate::gecko_bindings::structs::{ StyleGeometryBox, StyleShapeSource, StyleShapeSourceType, }; use crate::gecko_bindings::sugar::refptr::RefPtr; - use crate::values::computed::basic_shape::{ - BasicShape, ClippingShape, FloatAreaShape, ShapeRadius, - }; - use crate::values::computed::length::LengthPercentage; + use crate::values::computed::basic_shape::{BasicShape, ClippingShape, FloatAreaShape}; use crate::values::computed::motion::OffsetPath; use crate::values::computed::url::ComputedUrl; - use crate::values::generics::basic_shape::{ - BasicShape as GenericBasicShape, InsetRect, Polygon, - }; - use crate::values::generics::basic_shape::{Circle, Ellipse, Path, PolygonCoord}; - use crate::values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource}; - use crate::values::generics::rect::Rect; + use crate::values::generics::basic_shape::{GeometryBox, Path, ShapeBox, ShapeSource}; use crate::values::specified::SVGPathData; - use std::borrow::Borrow; impl StyleShapeSource { /// Convert StyleShapeSource to ShapeSource except URL and Image @@ -569,7 +556,7 @@ pub mod basic_shape { StyleShapeSourceType::Box => Some(ShapeSource::Box(self.mReferenceBox.into())), StyleShapeSourceType::Shape => { let other_shape = unsafe { &*self.__bindgen_anon_1.mBasicShape.as_ref().mPtr }; - let shape = other_shape.into(); + let shape = Box::new(other_shape.clone()); let reference_box = if self.mReferenceBox == StyleGeometryBox::NoBox { None } else { @@ -588,12 +575,10 @@ pub mod basic_shape { /// Generate a SVGPathData from StyleShapeSource if possible. fn to_svg_path(&self) -> Option<SVGPathData> { - use crate::values::specified::svg_path::PathCommand; match self.mType { StyleShapeSourceType::Path => { let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; - let result: Vec<PathCommand> = gecko_path.mPath.iter().cloned().collect(); - Some(SVGPathData::new(result.into_boxed_slice())) + Some(SVGPathData(gecko_path.mPath.clone())) }, _ => None, } @@ -653,67 +638,6 @@ pub mod basic_shape { } } - impl<'a> From<&'a StyleBasicShape> for BasicShape { - fn from(other: &'a StyleBasicShape) -> Self { - match other.mType { - StyleBasicShapeType::Inset => { - let t = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[0]); - let r = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[1]); - let b = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[2]); - let l = LengthPercentage::from_gecko_style_coord(&other.mCoordinates[3]); - let round = other.mRadius; - let rect = Rect::new( - t.expect("inset() offset should be a length, percentage, or calc value"), - r.expect("inset() offset should be a length, percentage, or calc value"), - b.expect("inset() offset should be a length, percentage, or calc value"), - l.expect("inset() offset should be a length, percentage, or calc value"), - ); - GenericBasicShape::Inset(InsetRect { rect, round }) - }, - StyleBasicShapeType::Circle => GenericBasicShape::Circle(Circle { - radius: (&other.mCoordinates[0]).into(), - position: other.mPosition, - }), - StyleBasicShapeType::Ellipse => GenericBasicShape::Ellipse(Ellipse { - semiaxis_x: (&other.mCoordinates[0]).into(), - semiaxis_y: (&other.mCoordinates[1]).into(), - position: other.mPosition, - }), - StyleBasicShapeType::Polygon => { - let mut coords = Vec::with_capacity(other.mCoordinates.len() / 2); - for i in 0..(other.mCoordinates.len() / 2) { - let x = 2 * i; - let y = x + 1; - coords.push(PolygonCoord( - LengthPercentage::from_gecko_style_coord(&other.mCoordinates[x]) - .expect( - "polygon() coordinate should be a length, percentage, \ - or calc value", - ), - LengthPercentage::from_gecko_style_coord(&other.mCoordinates[y]) - .expect( - "polygon() coordinate should be a length, percentage, \ - or calc value", - ), - )) - } - GenericBasicShape::Polygon(Polygon { - fill: other.mFillRule, - coordinates: coords, - }) - }, - } - } - } - - impl<'a> From<&'a nsStyleCoord> for ShapeRadius { - fn from(other: &'a nsStyleCoord) -> Self { - let other = other.borrow(); - ShapeRadius::from_gecko_style_coord(other) - .expect("<shape-radius> should be a length, percentage, calc, or keyword value") - } - } - impl From<ShapeBox> for StyleGeometryBox { fn from(reference: ShapeBox) -> Self { use crate::gecko_bindings::structs::StyleGeometryBox::*; diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index 8b05e1520b4..98fe90fe3d0 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -7,13 +7,11 @@ //! Different kind of helpers to interact with Gecko values. use crate::counter_style::{Symbol, Symbols}; +use crate::gecko_bindings::structs::StyleGridTrackBreadth; use crate::gecko_bindings::structs::{nsStyleCoord, CounterStylePtr}; -use crate::gecko_bindings::structs::{StyleGridTrackBreadth, StyleShapeRadius}; use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue}; -use crate::values::computed::basic_shape::ShapeRadius as ComputedShapeRadius; use crate::values::computed::{Angle, Length, LengthPercentage}; use crate::values::computed::{Number, NumberOrPercentage, Percentage}; -use crate::values::generics::basic_shape::ShapeRadius; use crate::values::generics::gecko::ScrollSnapPoint; use crate::values::generics::grid::{TrackBreadth, TrackKeyword}; use crate::values::generics::length::LengthPercentageOrAuto; @@ -192,35 +190,6 @@ impl<L: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for TrackBreadth< } } -impl GeckoStyleCoordConvertible for ComputedShapeRadius { - fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) { - match *self { - ShapeRadius::ClosestSide => coord.set_value(CoordDataValue::Enumerated( - StyleShapeRadius::ClosestSide as u32, - )), - ShapeRadius::FarthestSide => coord.set_value(CoordDataValue::Enumerated( - StyleShapeRadius::FarthestSide as u32, - )), - ShapeRadius::Length(lp) => lp.to_gecko_style_coord(coord), - } - } - - fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> { - match coord.as_value() { - CoordDataValue::Enumerated(v) => { - if v == StyleShapeRadius::ClosestSide as u32 { - Some(ShapeRadius::ClosestSide) - } else if v == StyleShapeRadius::FarthestSide as u32 { - Some(ShapeRadius::FarthestSide) - } else { - None - } - }, - _ => GeckoStyleCoordConvertible::from_gecko_style_coord(coord).map(ShapeRadius::Length), - } - } -} - impl<T: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for Option<T> { fn to_gecko_style_coord<U: CoordDataMut>(&self, coord: &mut U) { if let Some(ref me) = *self { diff --git a/components/style/lib.rs b/components/style/lib.rs index bd33dbeafe4..4d523d65c4f 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -188,6 +188,9 @@ pub use html5ever::Prefix; #[cfg(feature = "servo")] pub use servo_atoms::Atom; +pub use style_traits::arc_slice::ArcSlice; +pub use style_traits::owned_slice::OwnedSlice; + /// The CSS properties supported by the style system. /// Generated from the properties.mako.rs template by build.rs #[macro_use] diff --git a/components/style/parser.rs b/components/style/parser.rs index f3190f7f72b..f9322a7bcb3 100644 --- a/components/style/parser.rs +++ b/components/style/parser.rs @@ -136,6 +136,12 @@ impl<'a> ParserContext<'a> { .expect("Rule type expected, but none was found.") } + /// Returns whether CSS error reporting is enabled. + #[inline] + pub fn error_reporting_enabled(&self) -> bool { + self.error_reporter.is_some() + } + /// Record a CSS parse error with this context’s error reporting. pub fn log_css_error(&self, location: SourceLocation, error: ContextualParseError) { let error_reporter = match self.error_reporter { diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 7a96402c4a1..ae307077690 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -337,6 +337,7 @@ class Longhand(object): "OverflowWrap", "OverscrollBehavior", "Percentage", + "PositiveIntegerOrNone", "Resize", "SVGOpacity", "SVGPaintOrder", diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 42fc833e4b9..1ce083c944f 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -1240,19 +1240,28 @@ pub fn parse_one_declaration_into( None, ); + let property_id_for_error_reporting = if context.error_reporting_enabled() { + Some(id.clone()) + } else { + None + }; + let mut input = ParserInput::new(input); let mut parser = Parser::new(&mut input); let start_position = parser.position(); parser.parse_entirely(|parser| { PropertyDeclaration::parse_into(declarations, id, &context, parser) }).map_err(|err| { - let location = err.location; - let error = ContextualParseError::UnsupportedPropertyDeclaration( - parser.slice_from(start_position), - err, - None - ); - context.log_css_error(location, error); + if context.error_reporting_enabled() { + report_one_css_error( + &context, + None, + None, + err, + parser.slice_from(start_position), + property_id_for_error_reporting, + ) + } }) } @@ -1260,6 +1269,8 @@ pub fn parse_one_declaration_into( struct PropertyDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, declarations: &'a mut SourcePropertyDeclaration, + /// The last parsed property id if any. + last_parsed_property_id: Option<PropertyId>, } @@ -1289,6 +1300,7 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { let id = match PropertyId::parse(&name, self.context) { Ok(id) => id, Err(..) => { + self.last_parsed_property_id = None; return Err(input.new_custom_error(if is_non_mozilla_vendor_identifier(&name) { StyleParseErrorKind::UnknownVendorProperty } else { @@ -1296,6 +1308,9 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { })); } }; + if self.context.error_reporting_enabled() { + self.last_parsed_property_id = Some(id.clone()); + } input.parse_until_before(Delimiter::Bang, |input| { PropertyDeclaration::parse_into(self.declarations, id, self.context, input) })?; @@ -1309,6 +1324,68 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { } } +type SmallParseErrorVec<'i> = SmallVec<[(ParseError<'i>, &'i str, Option<PropertyId>); 2]>; + +#[cold] +fn report_one_css_error<'i>( + context: &ParserContext, + block: Option<&PropertyDeclarationBlock>, + selectors: Option<&SelectorList<SelectorImpl>>, + mut error: ParseError<'i>, + slice: &str, + property: Option<PropertyId>, +) { + debug_assert!(context.error_reporting_enabled()); + + fn all_properties_in_block(block: &PropertyDeclarationBlock, property: &PropertyId) -> bool { + match *property { + PropertyId::LonghandAlias(id, _) | + PropertyId::Longhand(id) => block.contains(id), + PropertyId::ShorthandAlias(id, _) | + PropertyId::Shorthand(id) => { + id.longhands().all(|longhand| block.contains(longhand)) + }, + // NOTE(emilio): We could do this, but it seems of limited utility, + // and it's linear on the size of the declaration block, so let's + // not. + PropertyId::Custom(..) => false, + } + } + + // If the unrecognized property looks like a vendor-specific property, + // silently ignore it instead of polluting the error output. + if let ParseErrorKind::Custom(StyleParseErrorKind::UnknownVendorProperty) = error.kind { + return; + } + + if let Some(ref property) = property { + if let Some(block) = block { + if all_properties_in_block(block, property) { + return; + } + } + error = match *property { + PropertyId::Custom(ref c) => StyleParseErrorKind::new_invalid(format!("--{}", c), error), + _ => StyleParseErrorKind::new_invalid(property.non_custom_id().unwrap().name(), error), + }; + } + + let location = error.location; + let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, selectors); + context.log_css_error(location, error); +} + +#[cold] +fn report_css_errors( + context: &ParserContext, + block: &PropertyDeclarationBlock, + selectors: Option<&SelectorList<SelectorImpl>>, + errors: &mut SmallParseErrorVec, +) { + for (error, slice, property) in errors.drain() { + report_one_css_error(context, Some(block), selectors, error, slice, property) + } +} /// Parse a list of property declarations and return a property declaration /// block. @@ -1320,10 +1397,12 @@ pub fn parse_property_declaration_list( let mut declarations = SourcePropertyDeclaration::new(); let mut block = PropertyDeclarationBlock::new(); let parser = PropertyDeclarationParser { - context: context, + context, + last_parsed_property_id: None, declarations: &mut declarations, }; let mut iter = DeclarationListParser::new(input, parser); + let mut errors = SmallParseErrorVec::new(); while let Some(declaration) = iter.next() { match declaration { Ok(importance) => { @@ -1335,17 +1414,17 @@ pub fn parse_property_declaration_list( Err((error, slice)) => { iter.parser.declarations.clear(); - // If the unrecognized property looks like a vendor-specific property, - // silently ignore it instead of polluting the error output. - if let ParseErrorKind::Custom(StyleParseErrorKind::UnknownVendorProperty) = error.kind { - continue; + if context.error_reporting_enabled() { + let property = iter.parser.last_parsed_property_id.take(); + errors.push((error, slice, property)); } - - let location = error.location; - let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, selectors); - context.log_css_error(location, error); } } } + + if !errors.is_empty() { + report_css_errors(context, &block, selectors, &mut errors) + } + block } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b73bb073fa6..703cbed2c0b 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2515,7 +2515,7 @@ fn static_assert() { rotate scroll-snap-points-x scroll-snap-points-y scroll-snap-coordinate -moz-binding will-change offset-path shape-outside - translate scale""" %> + translate scale -webkit-line-clamp""" %> <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> #[inline] pub fn generate_combined_transform(&mut self) { @@ -2901,7 +2901,7 @@ fn static_assert() { match v { OffsetPath::None => motion.mOffsetPath.mType = StyleShapeSourceType::None, OffsetPath::Path(p) => { - set_style_svg_path(&mut motion.mOffsetPath, &p, FillRule::Nonzero) + set_style_svg_path(&mut motion.mOffsetPath, p, FillRule::Nonzero) }, } unsafe { Gecko_SetStyleMotion(&mut self.gecko.mMotion, motion) }; @@ -2924,6 +2924,27 @@ fn static_assert() { self.copy_offset_path_from(other); } + #[allow(non_snake_case)] + pub fn set__webkit_line_clamp(&mut self, v: longhands::_webkit_line_clamp::computed_value::T) { + self.gecko.mLineClamp = match v { + Either::First(n) => n.0 as u32, + Either::Second(None_) => 0, + }; + } + + ${impl_simple_copy('_webkit_line_clamp', 'mLineClamp')} + + #[allow(non_snake_case)] + pub fn clone__webkit_line_clamp(&self) -> longhands::_webkit_line_clamp::computed_value::T { + match self.gecko.mLineClamp { + 0 => Either::Second(None_), + n => { + debug_assert!(n <= std::i32::MAX as u32); + Either::First((n as i32).into()) + } + } + } + </%self:impl_trait> <%def name="simple_image_array_property(name, shorthand, field_name)"> @@ -4002,39 +4023,30 @@ fn static_assert() { // Set SVGPathData to StyleShapeSource. fn set_style_svg_path( shape_source: &mut structs::mozilla::StyleShapeSource, - servo_path: &values::specified::svg_path::SVGPathData, + servo_path: values::specified::svg_path::SVGPathData, fill: values::generics::basic_shape::FillRule, ) { - use crate::gecko_bindings::bindings::Gecko_NewStyleSVGPath; - use crate::gecko_bindings::structs::StyleShapeSourceType; - - // Setup type. - shape_source.mType = StyleShapeSourceType::Path; - // Setup path. - let gecko_path = unsafe { - Gecko_NewStyleSVGPath(shape_source); - &mut shape_source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap() - }; - - gecko_path.mPath.assign_from_iter_pod(servo_path.commands().iter().cloned()); - - // Setup fill-rule. - gecko_path.mFillRule = fill; + unsafe { + bindings::Gecko_SetToSVGPath( + shape_source, + servo_path.0.forget(), + fill, + ); + } } <%def name="impl_shape_source(ident, gecko_ffi_name)"> pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { - use crate::gecko_bindings::bindings::{Gecko_NewBasicShape, Gecko_DestroyShapeSource}; - use crate::gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType, StyleShapeSourceType}; - use crate::gecko_bindings::structs::{StyleGeometryBox, StyleShapeSource}; - use crate::gecko::values::GeckoStyleCoordConvertible; - use crate::values::generics::basic_shape::{BasicShape, ShapeSource}; + use crate::values::generics::basic_shape::ShapeSource; + use crate::gecko_bindings::structs::StyleShapeSourceType; + use crate::gecko_bindings::structs::StyleGeometryBox; let ref mut ${ident} = self.gecko.${gecko_ffi_name}; - // clean up existing struct - unsafe { Gecko_DestroyShapeSource(${ident}) }; + // clean up existing struct. + unsafe { bindings::Gecko_DestroyShapeSource(${ident}) }; + ${ident}.mType = StyleShapeSourceType::None; match v { @@ -4061,74 +4073,14 @@ fn set_style_svg_path( ${ident}.mReferenceBox = reference.into(); ${ident}.mType = StyleShapeSourceType::Box; } - ShapeSource::Path(p) => set_style_svg_path(${ident}, &p.path, p.fill), + ShapeSource::Path(p) => set_style_svg_path(${ident}, p.path, p.fill), ShapeSource::Shape(servo_shape, maybe_box) => { - fn init_shape(${ident}: &mut StyleShapeSource, basic_shape_type: StyleBasicShapeType) - -> &mut StyleBasicShape { - unsafe { - // Create StyleBasicShape in StyleShapeSource. mReferenceBox and mType - // will be set manually later. - Gecko_NewBasicShape(${ident}, basic_shape_type); - &mut *${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr - } - } - match servo_shape { - BasicShape::Inset(inset) => { - let shape = init_shape(${ident}, StyleBasicShapeType::Inset); - unsafe { shape.mCoordinates.set_len(4) }; - - // set_len() can't call constructors, so the coordinates - // can contain any value. set_value() attempts to free - // allocated coordinates, so we don't want to feed it - // garbage values which it may misinterpret. - // Instead, we use leaky_set_value to blindly overwrite - // the garbage data without - // attempting to clean up. - shape.mCoordinates[0].leaky_set_null(); - inset.rect.0.to_gecko_style_coord(&mut shape.mCoordinates[0]); - shape.mCoordinates[1].leaky_set_null(); - inset.rect.1.to_gecko_style_coord(&mut shape.mCoordinates[1]); - shape.mCoordinates[2].leaky_set_null(); - inset.rect.2.to_gecko_style_coord(&mut shape.mCoordinates[2]); - shape.mCoordinates[3].leaky_set_null(); - inset.rect.3.to_gecko_style_coord(&mut shape.mCoordinates[3]); - shape.mRadius = inset.round; - } - BasicShape::Circle(circ) => { - let shape = init_shape(${ident}, StyleBasicShapeType::Circle); - unsafe { shape.mCoordinates.set_len(1) }; - shape.mCoordinates[0].leaky_set_null(); - circ.radius.to_gecko_style_coord(&mut shape.mCoordinates[0]); - - shape.mPosition = circ.position.into(); - } - BasicShape::Ellipse(el) => { - let shape = init_shape(${ident}, StyleBasicShapeType::Ellipse); - unsafe { shape.mCoordinates.set_len(2) }; - shape.mCoordinates[0].leaky_set_null(); - el.semiaxis_x.to_gecko_style_coord(&mut shape.mCoordinates[0]); - shape.mCoordinates[1].leaky_set_null(); - el.semiaxis_y.to_gecko_style_coord(&mut shape.mCoordinates[1]); - - shape.mPosition = el.position.into(); - } - BasicShape::Polygon(poly) => { - let shape = init_shape(${ident}, StyleBasicShapeType::Polygon); - unsafe { - shape.mCoordinates.set_len(poly.coordinates.len() as u32 * 2); - } - for (i, coord) in poly.coordinates.iter().enumerate() { - shape.mCoordinates[2 * i].leaky_set_null(); - shape.mCoordinates[2 * i + 1].leaky_set_null(); - coord.0.to_gecko_style_coord(&mut shape.mCoordinates[2 * i]); - coord.1.to_gecko_style_coord(&mut shape.mCoordinates[2 * i + 1]); - } - shape.mFillRule = poly.fill; - } + unsafe { + ${ident}.__bindgen_anon_1.mBasicShape.as_mut().mPtr = + Box::into_raw(servo_shape); } - - ${ident}.mReferenceBox = maybe_box.map(Into::into) - .unwrap_or(StyleGeometryBox::NoBox); + ${ident}.mReferenceBox = + maybe_box.map(Into::into).unwrap_or(StyleGeometryBox::NoBox); ${ident}.mType = StyleShapeSourceType::Shape; } } diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index a7a2cb9e348..6d754964ecc 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -670,3 +670,16 @@ ${helpers.predefined_type( animation_value_type="discrete", spec="https://compat.spec.whatwg.org/#touch-action", )} + +// Note that we only implement -webkit-line-clamp as a single, longhand +// property for now, but the spec defines line-clamp as a shorthand for separate +// max-lines, block-ellipsis, and continue properties. +${helpers.predefined_type( + "-webkit-line-clamp", + "PositiveIntegerOrNone", + "Either::Second(None_)", + gecko_pref="layout.css.webkit-line-clamp.enabled", + animation_value_type="Integer", + products="gecko", + spec="https://drafts.csswg.org/css-overflow-3/#line-clamp", +)} diff --git a/components/style/properties/longhands/svg.mako.rs b/components/style/properties/longhands/svg.mako.rs index 1a84c1d669f..3e4d207397a 100644 --- a/components/style/properties/longhands/svg.mako.rs +++ b/components/style/properties/longhands/svg.mako.rs @@ -87,7 +87,6 @@ ${helpers.predefined_type( "basic_shape::ClippingShape", "generics::basic_shape::ShapeSource::None", products="gecko", - boxed=True, animation_value_type="basic_shape::ClippingShape", flags="CREATES_STACKING_CONTEXT", spec="https://drafts.fxtf.org/css-masking/#propdef-clip-path", diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e7f9c72413e..a9fa35ba444 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2209,13 +2209,9 @@ impl PropertyDeclaration { // This probably affects some test results. let value = match input.try(CSSWideKeyword::parse) { Ok(keyword) => CustomDeclarationValue::CSSWideKeyword(keyword), - Err(()) => match crate::custom_properties::SpecifiedValue::parse(input) { - Ok(value) => CustomDeclarationValue::Value(value), - Err(e) => return Err(StyleParseErrorKind::new_invalid( - format!("--{}", property_name), - e, - )), - } + Err(()) => CustomDeclarationValue::Value( + crate::custom_properties::SpecifiedValue::parse(input)? + ), }; declarations.push(PropertyDeclaration::Custom(CustomDeclaration { name: property_name, @@ -2236,19 +2232,11 @@ impl PropertyDeclaration { .or_else(|err| { while let Ok(_) = input.next() {} // Look for var() after the error. if !input.seen_var_or_env_functions() { - return Err(StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - err, - )); + return Err(err); } input.reset(&start); let (first_token_type, css) = - crate::custom_properties::parse_non_custom_with_var(input).map_err(|e| { - StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - e, - ) - })?; + crate::custom_properties::parse_non_custom_with_var(input)?; Ok(PropertyDeclaration::WithVariables(VariableDeclaration { id, value: Arc::new(UnparsedValue { @@ -2287,20 +2275,12 @@ impl PropertyDeclaration { id.parse_into(declarations, context, input).or_else(|err| { while let Ok(_) = input.next() {} // Look for var() after the error. if !input.seen_var_or_env_functions() { - return Err(StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - err, - )); + return Err(err); } input.reset(&start); let (first_token_type, css) = - crate::custom_properties::parse_non_custom_with_var(input).map_err(|e| { - StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - e, - ) - })?; + crate::custom_properties::parse_non_custom_with_var(input)?; let unparsed = Arc::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 3b83c71e6e2..267ef0c8e6f 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -16,7 +16,6 @@ use crate::values::computed::Image; use crate::values::specified::SVGPathData; use crate::values::CSSFloat; use app_units::Au; -use euclid::Point2D; use smallvec::SmallVec; use std::cmp; @@ -241,16 +240,10 @@ impl Animate for Au { } } -impl<T> Animate for Point2D<T> -where - T: Animate, -{ +impl<T: Animate> Animate for Box<T> { #[inline] fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> { - Ok(Point2D::new( - self.x.animate(&other.x, procedure)?, - self.y.animate(&other.y, procedure)?, - )) + Ok(Box::new((**self).animate(&other, procedure)?)) } } @@ -288,6 +281,66 @@ where } } +impl<T> ToAnimatedValue for Box<T> +where + T: ToAnimatedValue, +{ + type AnimatedValue = Box<<T as ToAnimatedValue>::AnimatedValue>; + + #[inline] + fn to_animated_value(self) -> Self::AnimatedValue { + Box::new((*self).to_animated_value()) + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + Box::new(T::from_animated_value(*animated)) + } +} + +impl<T> ToAnimatedValue for Box<[T]> +where + T: ToAnimatedValue, +{ + type AnimatedValue = Box<[<T as ToAnimatedValue>::AnimatedValue]>; + + #[inline] + fn to_animated_value(self) -> Self::AnimatedValue { + self.into_vec() + .into_iter() + .map(T::to_animated_value) + .collect::<Vec<_>>() + .into_boxed_slice() + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + animated + .into_vec() + .into_iter() + .map(T::from_animated_value) + .collect::<Vec<_>>() + .into_boxed_slice() + } +} + +impl<T> ToAnimatedValue for crate::OwnedSlice<T> +where + T: ToAnimatedValue, +{ + type AnimatedValue = crate::OwnedSlice<<T as ToAnimatedValue>::AnimatedValue>; + + #[inline] + fn to_animated_value(self) -> Self::AnimatedValue { + self.into_box().to_animated_value().into() + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + Self::from(Box::from_animated_value(animated.into_box())) + } +} + impl<T> ToAnimatedValue for SmallVec<[T; 1]> where T: ToAnimatedValue, @@ -407,3 +460,17 @@ where Ok(v.into_boxed_slice()) } } + +impl<T> ToAnimatedZero for crate::ArcSlice<T> +where + T: ToAnimatedZero, +{ + #[inline] + fn to_animated_zero(&self) -> Result<Self, ()> { + let v = self + .iter() + .map(|v| v.to_animated_zero()) + .collect::<Result<Vec<_>, _>>()?; + Ok(crate::ArcSlice::from_iter(v.into_iter())) + } +} diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs index 28067fb0c81..9a245aa77f1 100644 --- a/components/style/values/computed/basic_shape.rs +++ b/components/style/values/computed/basic_shape.rs @@ -23,7 +23,7 @@ pub type ClippingShape = generic::ClippingShape<BasicShape, ComputedUrl>; pub type FloatAreaShape = generic::FloatAreaShape<BasicShape, Image>; /// A computed basic shape. -pub type BasicShape = generic::BasicShape< +pub type BasicShape = generic::GenericBasicShape< LengthPercentage, LengthPercentage, LengthPercentage, @@ -41,7 +41,7 @@ pub type Ellipse = generic::Ellipse<LengthPercentage, LengthPercentage, NonNegativeLengthPercentage>; /// The computed value of `ShapeRadius` -pub type ShapeRadius = generic::ShapeRadius<NonNegativeLengthPercentage>; +pub type ShapeRadius = generic::GenericShapeRadius<NonNegativeLengthPercentage>; impl ToCss for Circle { fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 3eb0c16836c..5bf0333dbfe 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -444,6 +444,30 @@ where } } +impl<T> ToComputedValue for crate::OwnedSlice<T> +where + T: ToComputedValue, +{ + type ComputedValue = crate::OwnedSlice<<T as ToComputedValue>::ComputedValue>; + + #[inline] + fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { + self.iter() + .map(|item| item.to_computed_value(context)) + .collect::<Vec<_>>() + .into() + } + + #[inline] + fn from_computed_value(computed: &Self::ComputedValue) -> Self { + computed + .iter() + .map(T::from_computed_value) + .collect::<Vec<_>>() + .into() + } +} + trivial_to_computed_value!(()); trivial_to_computed_value!(bool); trivial_to_computed_value!(f32); @@ -640,6 +664,9 @@ impl From<CSSInteger> for PositiveInteger { } } +/// A computed positive `<integer>` value or `none`. +pub type PositiveIntegerOrNone = Either<PositiveInteger, None_>; + /// rect(...) pub type ClipRect = generics::ClipRect<LengthOrAuto>; diff --git a/components/style/values/generics/basic_shape.rs b/components/style/values/generics/basic_shape.rs index 5999e914676..8c63d064817 100644 --- a/components/style/values/generics/basic_shape.rs +++ b/components/style/values/generics/basic_shape.rs @@ -7,8 +7,8 @@ use crate::values::animated::{Animate, Procedure, ToAnimatedZero}; use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; -use crate::values::generics::border::BorderRadius; -use crate::values::generics::position::Position; +use crate::values::generics::border::GenericBorderRadius; +use crate::values::generics::position::GenericPosition; use crate::values::generics::rect::Rect; use crate::values::specified::SVGPathData; use crate::Zero; @@ -89,7 +89,7 @@ pub enum ShapeBox { pub enum ShapeSource<BasicShape, ReferenceBox, ImageOrUrl> { #[animation(error)] ImageOrUrl(ImageOrUrl), - Shape(BasicShape, Option<ReferenceBox>), + Shape(Box<BasicShape>, Option<ReferenceBox>), #[animation(error)] Box(ReferenceBox), #[css(function)] @@ -113,7 +113,8 @@ pub enum ShapeSource<BasicShape, ReferenceBox, ImageOrUrl> { ToResolvedValue, ToShmem, )] -pub enum BasicShape<H, V, LengthPercentage, NonNegativeLengthPercentage> { +#[repr(C, u8)] +pub enum GenericBasicShape<H, V, LengthPercentage, NonNegativeLengthPercentage> { Inset( #[css(field_bound)] #[shmem(field_bound)] @@ -129,9 +130,11 @@ pub enum BasicShape<H, V, LengthPercentage, NonNegativeLengthPercentage> { #[shmem(field_bound)] Ellipse<H, V, NonNegativeLengthPercentage>, ), - Polygon(Polygon<LengthPercentage>), + Polygon(GenericPolygon<LengthPercentage>), } +pub use self::GenericBasicShape as BasicShape; + /// <https://drafts.csswg.org/css-shapes/#funcdef-inset> #[allow(missing_docs)] #[css(function = "inset")] @@ -148,10 +151,11 @@ pub enum BasicShape<H, V, LengthPercentage, NonNegativeLengthPercentage> { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct InsetRect<LengthPercentage, NonNegativeLengthPercentage> { pub rect: Rect<LengthPercentage>, #[shmem(field_bound)] - pub round: BorderRadius<NonNegativeLengthPercentage>, + pub round: GenericBorderRadius<NonNegativeLengthPercentage>, } /// <https://drafts.csswg.org/css-shapes/#funcdef-circle> @@ -171,9 +175,10 @@ pub struct InsetRect<LengthPercentage, NonNegativeLengthPercentage> { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct Circle<H, V, NonNegativeLengthPercentage> { - pub position: Position<H, V>, - pub radius: ShapeRadius<NonNegativeLengthPercentage>, + pub position: GenericPosition<H, V>, + pub radius: GenericShapeRadius<NonNegativeLengthPercentage>, } /// <https://drafts.csswg.org/css-shapes/#funcdef-ellipse> @@ -193,10 +198,11 @@ pub struct Circle<H, V, NonNegativeLengthPercentage> { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct Ellipse<H, V, NonNegativeLengthPercentage> { - pub position: Position<H, V>, - pub semiaxis_x: ShapeRadius<NonNegativeLengthPercentage>, - pub semiaxis_y: ShapeRadius<NonNegativeLengthPercentage>, + pub position: GenericPosition<H, V>, + pub semiaxis_x: GenericShapeRadius<NonNegativeLengthPercentage>, + pub semiaxis_y: GenericShapeRadius<NonNegativeLengthPercentage>, } /// <https://drafts.csswg.org/css-shapes/#typedef-shape-radius> @@ -216,7 +222,8 @@ pub struct Ellipse<H, V, NonNegativeLengthPercentage> { ToResolvedValue, ToShmem, )] -pub enum ShapeRadius<NonNegativeLengthPercentage> { +#[repr(C, u8)] +pub enum GenericShapeRadius<NonNegativeLengthPercentage> { Length(NonNegativeLengthPercentage), #[animation(error)] ClosestSide, @@ -224,10 +231,12 @@ pub enum ShapeRadius<NonNegativeLengthPercentage> { FarthestSide, } +pub use self::GenericShapeRadius as ShapeRadius; + /// A generic type for representing the `polygon()` function /// /// <https://drafts.csswg.org/css-shapes/#funcdef-polygon> -#[css(comma, function)] +#[css(comma, function = "polygon")] #[derive( Clone, Debug, @@ -240,15 +249,18 @@ pub enum ShapeRadius<NonNegativeLengthPercentage> { ToResolvedValue, ToShmem, )] -pub struct Polygon<LengthPercentage> { +#[repr(C)] +pub struct GenericPolygon<LengthPercentage> { /// The filling rule for a polygon. #[css(skip_if = "fill_is_default")] pub fill: FillRule, /// A collection of (x, y) coordinates to draw the polygon. #[css(iterable)] - pub coordinates: Vec<PolygonCoord<LengthPercentage>>, + pub coordinates: crate::OwnedSlice<PolygonCoord<LengthPercentage>>, } +pub use self::GenericPolygon as Polygon; + /// Coordinates for Polygon. #[derive( Clone, @@ -262,6 +274,7 @@ pub struct Polygon<LengthPercentage> { ToResolvedValue, ToShmem, )] +#[repr(C)] pub struct PolygonCoord<LengthPercentage>(pub LengthPercentage, pub LengthPercentage); // https://drafts.csswg.org/css-shapes/#typedef-fill-rule @@ -393,7 +406,8 @@ where this.1.animate(&other.1, procedure)?, )) }) - .collect::<Result<Vec<_>, _>>()?; + .collect::<Result<Vec<_>, _>>()? + .into(); Ok(Polygon { fill: self.fill, coordinates, diff --git a/components/style/values/resolved/mod.rs b/components/style/values/resolved/mod.rs index 4003060148c..379ea83ec6b 100644 --- a/components/style/values/resolved/mod.rs +++ b/components/style/values/resolved/mod.rs @@ -193,3 +193,20 @@ where Vec::from_resolved_value(Vec::from(resolved)).into_boxed_slice() } } + +impl<T> ToResolvedValue for crate::OwnedSlice<T> +where + T: ToResolvedValue, +{ + type ResolvedValue = crate::OwnedSlice<<T as ToResolvedValue>::ResolvedValue>; + + #[inline] + fn to_resolved_value(self, context: &Context) -> Self::ResolvedValue { + self.into_box().to_resolved_value(context).into() + } + + #[inline] + fn from_resolved_value(resolved: Self::ResolvedValue) -> Self { + Self::from(Box::from_resolved_value(resolved.into_box())) + } +} diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 014ce439d3c..4f627ceb07f 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -55,7 +55,7 @@ pub type Ellipse = pub type ShapeRadius = generic::ShapeRadius<NonNegativeLengthPercentage>; /// The specified value of `Polygon` -pub type Polygon = generic::Polygon<LengthPercentage>; +pub type Polygon = generic::GenericPolygon<LengthPercentage>; #[cfg(feature = "gecko")] fn is_clip_path_path_enabled(context: &ParserContext) -> bool { @@ -138,11 +138,11 @@ where } if let Some(shp) = shape { - return Ok(ShapeSource::Shape(shp, ref_box)); + return Ok(ShapeSource::Shape(Box::new(shp), ref_box)); } ref_box - .map(|v| ShapeSource::Box(v)) + .map(ShapeSource::Box) .ok_or(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } } @@ -152,7 +152,7 @@ impl Parse for GeometryBox { _context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result<Self, ParseError<'i>> { - if let Ok(shape_box) = input.try(|i| ShapeBox::parse(i)) { + if let Ok(shape_box) = input.try(ShapeBox::parse) { return Ok(GeometryBox::ShapeBox(shape_box)); } @@ -352,17 +352,16 @@ impl Polygon { }) .unwrap_or_default(); - let buf = input.parse_comma_separated(|i| { - Ok(PolygonCoord( - LengthPercentage::parse(context, i)?, - LengthPercentage::parse(context, i)?, - )) - })?; + let coordinates = input + .parse_comma_separated(|i| { + Ok(PolygonCoord( + LengthPercentage::parse(context, i)?, + LengthPercentage::parse(context, i)?, + )) + })? + .into(); - Ok(Polygon { - fill: fill, - coordinates: buf, - }) + Ok(Polygon { fill, coordinates }) } } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 7dbb93f6319..83fc71444bb 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -12,7 +12,7 @@ use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as Generic use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; use super::generics::transform::IsParallelTo; use super::generics::{self, GreaterThanOrEqualToOne, NonNegative}; -use super::{Auto, CSSFloat, CSSInteger, Either}; +use super::{Auto, CSSFloat, CSSInteger, Either, None_}; use crate::context::QuirksMode; use crate::parser::{Parse, ParserContext}; use crate::values::serialize_atom_identifier; @@ -593,6 +593,9 @@ impl Parse for PositiveInteger { } } +/// A specified positive `<integer>` value or `none`. +pub type PositiveIntegerOrNone = Either<PositiveInteger, None_>; + /// The specified value of a grid `<track-breadth>` pub type TrackBreadth = GenericTrackBreadth<LengthPercentage>; diff --git a/components/style/values/specified/svg_path.rs b/components/style/values/specified/svg_path.rs index 5509312256a..e5282e0a164 100644 --- a/components/style/values/specified/svg_path.rs +++ b/components/style/values/specified/svg_path.rs @@ -29,16 +29,14 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; ToResolvedValue, ToShmem, )] -pub struct SVGPathData(Box<[PathCommand]>); +#[repr(C)] +pub struct SVGPathData( + // TODO(emilio): Should probably measure this somehow only from the + // specified values. + #[ignore_malloc_size_of = "Arc"] pub crate::ArcSlice<PathCommand>, +); impl SVGPathData { - /// Return SVGPathData by a slice of PathCommand. - #[inline] - pub fn new(cmd: Box<[PathCommand]>) -> Self { - debug_assert!(!cmd.is_empty()); - SVGPathData(cmd) - } - /// Get the array of PathCommand. #[inline] pub fn commands(&self) -> &[PathCommand] { @@ -46,9 +44,9 @@ impl SVGPathData { &self.0 } - /// Create a normalized copy of this path by converting each relative command to an absolute - /// command. - fn normalize(&self) -> Self { + /// Create a normalized copy of this path by converting each relative + /// command to an absolute command. + fn normalize(&self) -> Box<[PathCommand]> { let mut state = PathTraversalState { subpath_start: CoordPair::new(0.0, 0.0), pos: CoordPair::new(0.0, 0.0), @@ -58,7 +56,7 @@ impl SVGPathData { .iter() .map(|seg| seg.normalize(&mut state)) .collect::<Vec<_>>(); - SVGPathData(result.into_boxed_slice()) + result.into_boxed_slice() } } @@ -71,7 +69,7 @@ impl ToCss for SVGPathData { dest.write_char('"')?; { let mut writer = SequenceWriter::new(dest, " "); - for command in self.0.iter() { + for command in self.commands() { writer.item(command)?; } } @@ -104,7 +102,9 @@ impl Parse for SVGPathData { } } - Ok(SVGPathData::new(path_parser.path.into_boxed_slice())) + Ok(SVGPathData(crate::ArcSlice::from_iter( + path_parser.path.into_iter(), + ))) } } @@ -114,14 +114,17 @@ impl Animate for SVGPathData { return Err(()); } + // FIXME(emilio): This allocates three copies of the path, that's not + // great! Specially, once we're normalized once, we don't need to + // re-normalize again. let result = self .normalize() - .0 .iter() - .zip(other.normalize().0.iter()) + .zip(other.normalize().iter()) .map(|(a, b)| a.animate(&b, procedure)) .collect::<Result<Vec<_>, _>>()?; - Ok(SVGPathData::new(result.into_boxed_slice())) + + Ok(SVGPathData(crate::ArcSlice::from_iter(result.into_iter()))) } } @@ -131,9 +134,8 @@ impl ComputeSquaredDistance for SVGPathData { return Err(()); } self.normalize() - .0 .iter() - .zip(other.normalize().0.iter()) + .zip(other.normalize().iter()) .map(|(this, other)| this.compute_squared_distance(&other)) .sum() } diff --git a/components/style_traits/arc_slice.rs b/components/style_traits/arc_slice.rs new file mode 100644 index 00000000000..dd1b99d3424 --- /dev/null +++ b/components/style_traits/arc_slice.rs @@ -0,0 +1,66 @@ +/* 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 https://mozilla.org/MPL/2.0/. */ + +//! A thin atomically-reference-counted slice. + +use servo_arc::ThinArc; +use std::mem; +use std::ops::Deref; +use std::ptr::NonNull; + +/// A canary that we stash in ArcSlices. +/// +/// Given we cannot use a zero-sized-type for the header, since well, C++ +/// doesn't have zsts, and we want to use cbindgen for this type, we may as well +/// assert some sanity at runtime. +const ARC_SLICE_CANARY: u32 = 0xf3f3f3f3; + +/// A wrapper type for a refcounted slice using ThinArc. +/// +/// cbindgen:derive-eq=false +/// cbindgen:derive-neq=false +#[repr(C)] +#[derive(Clone, Debug, Eq, PartialEq, ToShmem)] +pub struct ArcSlice<T>(#[shmem(field_bound)] ThinArc<u32, T>); + +impl<T> Deref for ArcSlice<T> { + type Target = [T]; + + #[inline] + fn deref(&self) -> &Self::Target { + debug_assert_eq!(self.0.header.header, ARC_SLICE_CANARY); + &self.0.slice + } +} + +/// The inner pointer of an ArcSlice<T>, to be sent via FFI. +/// The type of the pointer is a bit of a lie, we just want to preserve the type +/// but these pointers cannot be constructed outside of this crate, so we're +/// good. +#[repr(C)] +pub struct ForgottenArcSlicePtr<T>(NonNull<T>); + +impl<T> ArcSlice<T> { + /// Creates an Arc for a slice using the given iterator to generate the + /// slice. + #[inline] + pub fn from_iter<I>(items: I) -> Self + where + I: Iterator<Item = T> + ExactSizeIterator, + { + ArcSlice(ThinArc::from_header_and_iter(ARC_SLICE_CANARY, items)) + } + + /// Creates a value that can be passed via FFI, and forgets this value + /// altogether. + #[inline] + #[allow(unsafe_code)] + pub fn forget(self) -> ForgottenArcSlicePtr<T> { + let ret = unsafe { + ForgottenArcSlicePtr(NonNull::new_unchecked(self.0.ptr() as *const _ as *mut _)) + }; + mem::forget(self); + ret + } +} diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index bce0a507f75..d4a907fa956 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -84,11 +84,13 @@ pub enum CSSPixel {} // / hidpi_ratio => DeviceIndependentPixel // / desktop_zoom => CSSPixel +pub mod arc_slice; pub mod specified_value_info; #[macro_use] pub mod values; #[macro_use] pub mod viewport; +pub mod owned_slice; pub use crate::specified_value_info::{CssType, KeywordsCollectFn, SpecifiedValueInfo}; pub use crate::values::{ diff --git a/components/style_traits/owned_slice.rs b/components/style_traits/owned_slice.rs new file mode 100644 index 00000000000..f2b0de4a4a0 --- /dev/null +++ b/components/style_traits/owned_slice.rs @@ -0,0 +1,166 @@ +/* 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 https://mozilla.org/MPL/2.0/. */ + +#![allow(unsafe_code)] + +//! A replacement for `Box<[T]>` that cbindgen can understand. + +use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps}; +use std::marker::PhantomData; +use std::ops::{Deref, DerefMut}; +use std::ptr::NonNull; +use std::{fmt, mem, slice}; +use to_shmem::{SharedMemoryBuilder, ToShmem}; + +/// A struct that basically replaces a `Box<[T]>`, but which cbindgen can +/// understand. +/// +/// We could rely on the struct layout of `Box<[T]>` per: +/// +/// https://github.com/rust-lang/unsafe-code-guidelines/blob/master/reference/src/layout/pointers.md +/// +/// But handling fat pointers with cbindgen both in structs and argument +/// positions more generally is a bit tricky. +/// +/// cbindgen:derive-eq=false +/// cbindgen:derive-neq=false +#[repr(C)] +pub struct OwnedSlice<T: Sized> { + ptr: NonNull<T>, + len: usize, + _phantom: PhantomData<T>, +} + +impl<T: Sized> Default for OwnedSlice<T> { + #[inline] + fn default() -> Self { + Self { + len: 0, + ptr: NonNull::dangling(), + _phantom: PhantomData, + } + } +} + +impl<T: Sized> Drop for OwnedSlice<T> { + #[inline] + fn drop(&mut self) { + if self.len != 0 { + let _ = mem::replace(self, Self::default()).into_vec(); + } + } +} + +unsafe impl<T: Sized + Send> Send for OwnedSlice<T> {} +unsafe impl<T: Sized + Sync> Sync for OwnedSlice<T> {} + +impl<T: Clone> Clone for OwnedSlice<T> { + #[inline] + fn clone(&self) -> Self { + Self::from_slice(&**self) + } +} + +impl<T: fmt::Debug> fmt::Debug for OwnedSlice<T> { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + self.deref().fmt(formatter) + } +} + +impl<T: PartialEq> PartialEq for OwnedSlice<T> { + fn eq(&self, other: &Self) -> bool { + self.deref().eq(other.deref()) + } +} + +impl<T: Eq> Eq for OwnedSlice<T> {} + +impl<T: Sized> OwnedSlice<T> { + /// Convert the OwnedSlice into a boxed slice. + #[inline] + pub fn into_box(self) -> Box<[T]> { + self.into_vec().into_boxed_slice() + } + + /// Convert the OwnedSlice into a Vec. + #[inline] + pub fn into_vec(self) -> Vec<T> { + let ret = unsafe { Vec::from_raw_parts(self.ptr.as_ptr(), self.len, self.len) }; + mem::forget(self); + ret + } + + /// Iterate over all the elements in the slice taking ownership of them. + #[inline] + pub fn into_iter(self) -> impl Iterator<Item = T> { + self.into_vec().into_iter() + } + + /// Convert the regular slice into an owned slice. + #[inline] + pub fn from_slice(s: &[T]) -> Self + where + T: Clone, + { + Self::from(s.to_vec()) + } +} + +impl<T> Deref for OwnedSlice<T> { + type Target = [T]; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + unsafe { slice::from_raw_parts(self.ptr.as_ptr(), self.len) } + } +} + +impl<T> DerefMut for OwnedSlice<T> { + #[inline(always)] + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { slice::from_raw_parts_mut(self.ptr.as_ptr(), self.len) } + } +} + +impl<T> From<Box<[T]>> for OwnedSlice<T> { + #[inline] + fn from(mut b: Box<[T]>) -> Self { + let len = b.len(); + let ptr = unsafe { NonNull::new_unchecked(b.as_mut_ptr()) }; + mem::forget(b); + Self { + len, + ptr, + _phantom: PhantomData, + } + } +} + +impl<T> From<Vec<T>> for OwnedSlice<T> { + #[inline] + fn from(b: Vec<T>) -> Self { + Self::from(b.into_boxed_slice()) + } +} + +impl<T: Sized> MallocShallowSizeOf for OwnedSlice<T> { + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + unsafe { ops.malloc_size_of(self.ptr.as_ptr()) } + } +} + +impl<T: MallocSizeOf + Sized> MallocSizeOf for OwnedSlice<T> { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.shallow_size_of(ops) + (**self).size_of(ops) + } +} + +impl<T: ToShmem + Sized> ToShmem for OwnedSlice<T> { + fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> mem::ManuallyDrop<Self> { + unsafe { + let dest = to_shmem::to_shmem_slice(self.iter(), builder); + mem::ManuallyDrop::new(Self::from(Box::from_raw(dest))) + } + } +} diff --git a/components/style_traits/specified_value_info.rs b/components/style_traits/specified_value_info.rs index c978727b47a..63a7b71d990 100644 --- a/components/style_traits/specified_value_info.rs +++ b/components/style_traits/specified_value_info.rs @@ -4,6 +4,7 @@ //! Value information for devtools. +use crate::arc_slice::ArcSlice; use servo_arc::Arc; use std::ops::Range; use std::sync::Arc as StdArc; @@ -116,6 +117,7 @@ impl_generic_specified_value_info!(Option<T>); impl_generic_specified_value_info!(Vec<T>); impl_generic_specified_value_info!(Arc<T>); impl_generic_specified_value_info!(StdArc<T>); +impl_generic_specified_value_info!(ArcSlice<T>); impl_generic_specified_value_info!(Range<Idx>); impl<T1, T2> SpecifiedValueInfo for (T1, T2) diff --git a/components/to_shmem/lib.rs b/components/to_shmem/lib.rs index 96dfbb3a5e9..07c5fc9ccaa 100644 --- a/components/to_shmem/lib.rs +++ b/components/to_shmem/lib.rs @@ -311,7 +311,7 @@ where /// Writes all the items in `src` into a slice in the shared memory buffer and /// returns a pointer to the slice. -unsafe fn to_shmem_slice<'a, T, I>(src: I, builder: &mut SharedMemoryBuilder) -> *mut [T] +pub unsafe fn to_shmem_slice<'a, T, I>(src: I, builder: &mut SharedMemoryBuilder) -> *mut [T] where T: 'a + ToShmem, I: ExactSizeIterator<Item = &'a T>, |