Skip to content

Commit

Permalink
Bug 1864736 - Eliminate redundant URL data. r=zsun
Browse files Browse the repository at this point in the history
Now that registered custom property values contain URL data, URL data is
eliminated as an argument in some places, and the ComputedValue.url_data
field is removed.

Differential Revision: https://phabricator.services.mozilla.com/D203359
  • Loading branch information
zrhoffman committed Mar 11, 2024
1 parent b027e2c commit 9911699
Showing 1 changed file with 6 additions and 19 deletions.
25 changes: 6 additions & 19 deletions servo/components/style/properties_and_values/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl SpecifiedValue {
context,
allow_computationally_dependent,
)?;
Ok(value.to_variable_value(url_data))
Ok(value.to_variable_value())
}

/// Convert a registered custom property to a Computed custom property value, given input and a
Expand Down Expand Up @@ -350,14 +350,14 @@ impl ComputedValue {
}
}

fn to_declared_value(&self, url_data: &UrlExtraData) -> Arc<ComputedPropertyValue> {
fn to_declared_value(&self) -> Arc<ComputedPropertyValue> {
if let ValueInner::Universal(ref var) = self.v {
return Arc::clone(var);
}
Arc::new(self.to_variable_value(url_data))
Arc::new(self.to_variable_value())
}

fn to_variable_value(&self, url_data: &UrlExtraData) -> ComputedPropertyValue {
fn to_variable_value(&self) -> ComputedPropertyValue {
debug_assert!(
!matches!(self.v, ValueInner::Universal(..)),
"Shouldn't be needed"
Expand All @@ -370,7 +370,7 @@ impl ComputedValue {
let serialization_types = self.serialization_types();
ComputedPropertyValue::new(
self.to_css_string(),
url_data,
&self.url_data,
serialization_types.0,
serialization_types.1,
)
Expand Down Expand Up @@ -599,11 +599,6 @@ pub struct CustomAnimatedValue {
pub(crate) name: crate::custom_properties::Name,
/// The computed value of the custom property.
value: ComputedValue,
/// The url data where the value came from.
/// FIXME: This seems like it should not be needed: registered properties don't need it, and
/// unregistered properties animate discretely. But we need it so far because the computed
/// value representation isn't typed.
url_data: UrlExtraData,
}

impl Animate for CustomAnimatedValue {
Expand All @@ -615,9 +610,6 @@ impl Animate for CustomAnimatedValue {
Ok(Self {
name: self.name.clone(),
value,
// NOTE: This is sketchy AF, but it's ~fine, since values that can animate (non-universal)
// don't need it.
url_data: self.url_data.clone(),
})
}
}
Expand All @@ -634,7 +626,6 @@ impl CustomAnimatedValue {
};
Self {
name: name.clone(),
url_data: value.url_data.clone(),
value,
}
}
Expand Down Expand Up @@ -677,24 +668,20 @@ impl CustomAnimatedValue {
.ok()
};

let url_data = value.url_data.clone();
let value = computed_value.unwrap_or_else(|| ComputedValue {
v: ValueInner::Universal(Arc::clone(value)),
url_data: value.url_data.clone(),
});
Some(Self {
name: declaration.name.clone(),
url_data,
value,
})
}

pub(crate) fn to_declaration(&self) -> properties::PropertyDeclaration {
properties::PropertyDeclaration::Custom(properties::CustomDeclaration {
name: self.name.clone(),
value: properties::CustomDeclarationValue::Value(
self.value.to_declared_value(&self.url_data),
),
value: properties::CustomDeclarationValue::Value(self.value.to_declared_value()),
})
}
}

0 comments on commit 9911699

Please sign in to comment.