Skip to content
Permalink
Browse files

style: Use cbindgen for URIs.

This doesn't clean up as much as a whole, but it's a step in the right
direction. In particular, it allows us to start using simple bindings for:

 * Filters
 * Shapes and images, almost. Need to:
   * Get rid of the complex -moz- gradient parsing (let
     layout.css.simple-moz-gradient.enabled get to release).
 * Counters, almost. Need to:
   * Share the Attr representation with Gecko, by not using Option<>.
     * Just another variant should be enough (ContentItem::{Attr,Prefixedattr},
       maybe).

Which in turn allows us to remove a whole lot of bindings in followups to this.

The setup changes a bit. This also removes the double pointer I complained about
while reviewing the shared UA sheet patches. The old setup is:

```
SpecifiedUrl
 * CssUrl
   * Arc<CssUrlData>
     * String
     * UrlExtraData
 * UrlValueSource
   * Arc<CssUrlData>
   * load id
   * resolved uri
   * CORS mode.
   * ...
```

The new one removes the double reference to the url data via URLValue, and looks
like:

```
SpecifiedUrl
 * CssUrl
   * Arc<CssUrlData>
     * String
     * UrlExtraData
     * CorsMode
     * LoadData
       * load id
       * resolved URI
```

The LoadData is the only mutable bit that C++ can change, and is not used from
Rust. Ideally, in the future, we could just use rust-url to resolve the URL
after parsing or something, and make it all immutable. Maybe.

I've verified that this approach still works with the UA sheet patches (via the
LoadDataSource::Lazy).

The reordering of mWillChange is to avoid nsStyleDisplay from going over the
size limit. We want to split it up anyway in bug 1552587, but mBinding gains a
tag member, which means that we were having a bit of extra padding.

One thing I want to explore is to see if we can abuse rustc's non-zero
optimizations to predict the layout from C++, but that's something to explore at
some other point in time and with a lot of care and help from Michael (who sits
next to me and works on rustc ;)).

Differential Revision: https://phabricator.services.mozilla.com/D31742
  • Loading branch information...
emilio committed May 27, 2019
1 parent 8a0cf60 commit ccff9b294f17ff4214e1c488817d48a10c95c2f9
@@ -90,6 +90,9 @@ const STATIC_REFCOUNT: usize = usize::MAX;
/// usage of PhantomData.
///
/// [`Arc`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html
///
/// cbindgen:derive-eq=false
/// cbindgen:derive-neq=false
#[repr(C)]
pub struct Arc<T: ?Sized> {
p: ptr::NonNull<ArcInner<T>>,
@@ -55,7 +55,7 @@ impl OneOrMoreSeparated for Source {
#[repr(u8)]
#[allow(missing_docs)]
pub enum FontFaceSourceListComponent {
Url(*const crate::gecko_bindings::structs::mozilla::css::URLValue),
Url(*const crate::gecko::url::CssUrl),
Local(*mut crate::gecko_bindings::structs::nsAtom),
FormatHint {
length: usize,
@@ -122,11 +122,11 @@ impl nsStyleImage {
match image {
GenericImage::Gradient(boxed_gradient) => self.set_gradient(*boxed_gradient),
GenericImage::Url(ref url) => unsafe {
bindings::Gecko_SetLayerImageImageValue(self, url.url_value_ptr())
bindings::Gecko_SetLayerImageImageValue(self, url);
},
GenericImage::Rect(ref image_rect) => {
unsafe {
bindings::Gecko_SetLayerImageImageValue(self, image_rect.url.url_value_ptr());
bindings::Gecko_SetLayerImageImageValue(self, &image_rect.url);
bindings::Gecko_InitializeImageCropRect(self);

// Set CropRect
@@ -584,9 +584,10 @@ pub mod basic_shape {

impl<'a> From<&'a StyleShapeSource> for ClippingShape {
fn from(other: &'a StyleShapeSource) -> Self {
use crate::values::generics::image::Image as GenericImage;
match other.mType {
StyleShapeSourceType::Image => unsafe {
use crate::values::generics::image::Image as GenericImage;

let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr;
let image = shape_image.into_image().expect("Cannot convert to Image");
match image {

0 comments on commit ccff9b2

Please sign in to comment.
You can’t perform that action at this time.