Bug 1507050 - Store MediaFeatureDescription references as an index into the MEDIA_FEATURES array r=emilio
authorCameron McCormack <cam@mcc.id.au>
Wed, 14 Nov 2018 21:33:01 +0000
changeset 446404 3c32668ebbbe810191359d53f00de1a191b966ac
parent 446232 4e1b2b7e0c37fb3fcdb6795537af53510b9308ef
child 446405 ab3dbc49f94d54897502e6ee9973229fc52f3187
push id35041
push useraiakab@mozilla.com
push dateThu, 15 Nov 2018 09:52:43 +0000
treeherdermozilla-central@48720735b142 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1507050
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1507050 - Store MediaFeatureDescription references as an index into the MEDIA_FEATURES array r=emilio The current use of a static reference is incompatible with sharing style sheet data across processes. Differential Revision: https://phabricator.services.mozilla.com/D11845
servo/components/style/media_queries/media_feature_expression.rs
--- a/servo/components/style/media_queries/media_feature_expression.rs
+++ b/servo/components/style/media_queries/media_feature_expression.rs
@@ -5,18 +5,22 @@
 //! Parsing for media feature expressions, like `(foo: bar)` or
 //! `(width >= 400px)`.
 
 use super::media_feature::{Evaluator, MediaFeatureDescription};
 use super::media_feature::{KeywordDiscriminant, ParsingRequirements};
 use super::Device;
 use crate::context::QuirksMode;
 #[cfg(feature = "gecko")]
+use crate::gecko::media_features::MEDIA_FEATURES;
+#[cfg(feature = "gecko")]
 use crate::gecko_bindings::structs;
 use crate::parser::{Parse, ParserContext};
+#[cfg(feature = "servo")]
+use crate::servo::media_queries::MEDIA_FEATURES;
 use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase};
 use crate::stylesheets::Origin;
 use crate::values::computed::{self, ToComputedValue};
 use crate::values::specified::{Integer, Length, Number, Resolution};
 use crate::values::{serialize_atom_identifier, CSSFloat};
 use crate::Atom;
 use cssparser::{Parser, Token};
 use num_traits::Zero;
@@ -145,53 +149,54 @@ impl RangeOrOperator {
         }
     }
 }
 
 /// A feature expression contains a reference to the media feature, the value
 /// the media query contained, and the range to evaluate.
 #[derive(Clone, Debug, MallocSizeOf)]
 pub struct MediaFeatureExpression {
-    feature: &'static MediaFeatureDescription,
+    feature_index: usize,
     value: Option<MediaExpressionValue>,
     range_or_operator: Option<RangeOrOperator>,
 }
 
 impl PartialEq for MediaFeatureExpression {
     fn eq(&self, other: &Self) -> bool {
-        self.feature as *const _ == other.feature as *const _ &&
+        self.feature_index == other.feature_index &&
             self.value == other.value &&
             self.range_or_operator == other.range_or_operator
     }
 }
 
 impl ToCss for MediaFeatureExpression {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: fmt::Write,
     {
         dest.write_str("(")?;
 
-        if self
-            .feature
+        let feature = self.feature();
+
+        if feature
             .requirements
             .contains(ParsingRequirements::WEBKIT_PREFIX)
         {
             dest.write_str("-webkit-")?;
         }
 
         if let Some(RangeOrOperator::Range(range)) = self.range_or_operator {
             match range {
                 Range::Min => dest.write_str("min-")?,
                 Range::Max => dest.write_str("max-")?,
             }
         }
 
         // NB: CssStringWriter not needed, feature names are under control.
-        write!(dest, "{}", self.feature.name)?;
+        write!(dest, "{}", feature.name)?;
 
         if let Some(RangeOrOperator::Operator(op)) = self.range_or_operator {
             dest.write_char(' ')?;
             op.to_css(dest)?;
             dest.write_char(' ')?;
         } else if self.value.is_some() {
             dest.write_str(": ")?;
         }
@@ -235,27 +240,32 @@ fn consume_operation_or_colon(input: &mu
             }
         },
         _ => return Err(()),
     }))
 }
 
 impl MediaFeatureExpression {
     fn new(
-        feature: &'static MediaFeatureDescription,
+        feature_index: usize,
         value: Option<MediaExpressionValue>,
         range_or_operator: Option<RangeOrOperator>,
     ) -> Self {
+        debug_assert!(feature_index < MEDIA_FEATURES.len());
         Self {
-            feature,
+            feature_index,
             value,
             range_or_operator,
         }
     }
 
+    fn feature(&self) -> &'static MediaFeatureDescription {
+        &MEDIA_FEATURES[self.feature_index]
+    }
+
     /// Parse a media expression of the form:
     ///
     /// ```
     /// (media-feature: media-value)
     /// ```
     pub fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -265,22 +275,18 @@ impl MediaFeatureExpression {
     }
 
     /// Parse a media feature expression where we've already consumed the
     /// parenthesis.
     pub fn parse_in_parenthesis_block<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        #[cfg(feature = "gecko")]
-        use crate::gecko::media_features::MEDIA_FEATURES;
-        #[cfg(feature = "servo")]
-        use crate::servo::media_queries::MEDIA_FEATURES;
-
         // FIXME: remove extra indented block when lifetimes are non-lexical
+        let feature_index;
         let feature;
         let range;
         {
             let location = input.current_source_location();
             let ident = input.expect_ident()?;
 
             let mut requirements = ParsingRequirements::empty();
 
@@ -314,24 +320,25 @@ impl MediaFeatureExpression {
                 } else if starts_with_ignore_ascii_case(feature_name, "max-") {
                     feature_name = &feature_name[4..];
                     Some(Range::Max)
                 } else {
                     None
                 };
 
                 let atom = Atom::from(string_as_ascii_lowercase(feature_name));
-                match MEDIA_FEATURES.iter().find(|f| f.name == atom) {
-                    Some(f) => Ok((f, range)),
+                match MEDIA_FEATURES.iter().enumerate().find(|(_, f)| f.name == atom) {
+                    Some((i, f)) => Ok((i, f, range)),
                     None => Err(()),
                 }
             };
 
             match result {
-                Ok((f, r)) => {
+                Ok((i, f, r)) => {
+                    feature_index = i;
                     feature = f;
                     range = r;
                 },
                 Err(()) => {
                     return Err(location.new_custom_error(
                         StyleParseErrorKind::MediaQueryExpectedFeatureName(ident.clone()),
                     ))
                 },
@@ -360,17 +367,17 @@ impl MediaFeatureExpression {
                 // Gecko doesn't allow ranged expressions without a
                 // value, so just reject them here too.
                 if range.is_some() {
                     return Err(
                         input.new_custom_error(StyleParseErrorKind::RangedExpressionWithNoValue)
                     );
                 }
 
-                return Ok(Self::new(feature, None, None));
+                return Ok(Self::new(feature_index, None, None));
             },
             Ok(operator) => operator,
         };
 
         let range_or_operator = match range {
             Some(range) => {
                 if operator.is_some() {
                     return Err(
@@ -391,33 +398,33 @@ impl MediaFeatureExpression {
             },
         };
 
         let value = MediaExpressionValue::parse(feature, context, input).map_err(|err| {
             err.location
                 .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue)
         })?;
 
-        Ok(Self::new(feature, Some(value), range_or_operator))
+        Ok(Self::new(feature_index, Some(value), range_or_operator))
     }
 
     /// Returns whether this media query evaluates to true for the given device.
     pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
         let value = self.value.as_ref();
 
         macro_rules! expect {
             ($variant:ident) => {
                 value.map(|value| match *value {
                     MediaExpressionValue::$variant(ref v) => v,
                     _ => unreachable!("Unexpected MediaExpressionValue"),
                 })
             };
         }
 
-        match self.feature.evaluator {
+        match self.feature().evaluator {
             Evaluator::Length(eval) => {
                 let computed = expect!(Length).map(|specified| {
                     computed::Context::for_media_query_evaluation(device, quirks_mode, |context| {
                         specified.to_computed_value(context)
                     })
                 });
                 eval(device, computed, self.range_or_operator)
             },
@@ -487,17 +494,17 @@ impl MediaExpressionValue {
         match *self {
             MediaExpressionValue::Length(ref l) => l.to_css(dest),
             MediaExpressionValue::Integer(v) => v.to_css(dest),
             MediaExpressionValue::Float(v) => v.to_css(dest),
             MediaExpressionValue::BoolInteger(v) => dest.write_str(if v { "1" } else { "0" }),
             MediaExpressionValue::IntRatio(ratio) => ratio.to_css(dest),
             MediaExpressionValue::Resolution(ref r) => r.to_css(dest),
             MediaExpressionValue::Ident(ref ident) => serialize_atom_identifier(ident, dest),
-            MediaExpressionValue::Enumerated(value) => match for_expr.feature.evaluator {
+            MediaExpressionValue::Enumerated(value) => match for_expr.feature().evaluator {
                 Evaluator::Enumerated { serializer, .. } => dest.write_str(&*serializer(value)),
                 _ => unreachable!(),
             },
         }
     }
 
     fn parse<'i, 't>(
         for_feature: &MediaFeatureDescription,