style: Follow-up clean-ups and tweaks

<general-enclosed> should be handled in parse_in_parens.

Add some convenience functions to KleeneValue.

Differential Revision: https://phabricator.services.mozilla.com/D163214

components/style/media_queries/media_list.rs

@@ -82,31 +82,17 @@ impl MediaList {
 
         computed::Context::for_media_query_evaluation(device, quirks_mode, |context| {
             self.media_queries.iter().any(|mq| {
-                let media_match = mq.media_type.matches(device.media_type());
+                let mut query_match = if mq.media_type.matches(device.media_type()) {
-
+                    mq.condition.as_ref().map_or(KleeneValue::True, |c| c.matches(context))
-                // Check if the media condition match.
+                } else {
-                let query_match = match media_match {
+                    KleeneValue::False
-                    true => mq.condition.as_ref().map_or(KleeneValue::True, |c| c.matches(context)),
-                    false => KleeneValue::False,
                 };
 
                 // Apply the logical NOT qualifier to the result
-                match mq.qualifier {
+                if matches!(mq.qualifier, Some(Qualifier::Not)) {
-                    Some(Qualifier::Not) => {
+                    query_match = !query_match;
-                        match query_match {
-                            KleeneValue::False => true,
-                            KleeneValue::True => false,
-                            KleeneValue::Unknown => false,
-                        }
-                    },
-                    _ => {
-                        match query_match {
-                            KleeneValue::False => false,
-                            KleeneValue::True => true,
-                            KleeneValue::Unknown => false,
-                        }
-                    },
                 }
+                query_match.to_bool(/* unknown = */ false)
             })
         })
     }

components/style/queries/condition.rs

@@ -8,12 +8,11 @@
 //! https://drafts.csswg.org/css-contain-3/#typedef-container-condition
 
 use super::{FeatureFlags, FeatureType, QueryFeatureExpression};
-use crate::parser::ParserContext;
 use crate::values::computed;
+use crate::{error_reporting::ContextualParseError, parser::ParserContext};
 use cssparser::{Parser, Token};
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
-use core::ops::Not;
 
 /// A binary `and` or `or` operator.
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss, ToShmem)]
@@ -33,15 +32,37 @@ enum AllowOr {
 /// https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToCss)]
 pub enum KleeneValue {
-    /// True
-    True,
     /// False
-    False,
+    False = 0,
+    /// True
+    True = 1,
     /// Either true or false, but we’re not sure which yet.
     Unknown,
 }
 
-impl Not for KleeneValue {
+impl From<bool> for KleeneValue {
+    fn from(b: bool) -> Self {
+        if b {
+            Self::True
+        } else {
+            Self::False
+        }
+    }
+}
+
+impl KleeneValue {
+    /// Turns this Kleene value to a bool, taking the unknown value as an
+    /// argument.
+    pub fn to_bool(self, unknown: bool) -> bool {
+        match self {
+            Self::True => true,
+            Self::False => false,
+            Self::Unknown => unknown,
+        }
+    }
+}
+
+impl std::ops::Not for KleeneValue {
     type Output = Self;
 
     fn not(self) -> Self {
@@ -53,6 +74,48 @@ impl Not for KleeneValue {
     }
 }
 
+// Implements the logical and operation.
+impl std::ops::BitAnd for KleeneValue {
+    type Output = Self;
+
+    fn bitand(self, other: Self) -> Self {
+        if self == Self::False || other == Self::False {
+            return Self::False;
+        }
+        if self == Self::Unknown || other == Self::Unknown {
+            return Self::Unknown;
+        }
+        Self::True
+    }
+}
+
+// Implements the logical or operation.
+impl std::ops::BitOr for KleeneValue {
+    type Output = Self;
+
+    fn bitor(self, other: Self) -> Self {
+        if self == Self::True || other == Self::True {
+            return Self::True;
+        }
+        if self == Self::Unknown || other == Self::Unknown {
+            return Self::Unknown;
+        }
+        Self::False
+    }
+}
+
+impl std::ops::BitOrAssign for KleeneValue {
+    fn bitor_assign(&mut self, other: Self) {
+        *self = *self | other;
+    }
+}
+
+impl std::ops::BitAndAssign for KleeneValue {
+    fn bitand_assign(&mut self, other: Self) {
+        *self = *self & other;
+    }
+}
+
 /// Represents a condition.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToShmem)]
 pub enum QueryCondition {
@@ -104,10 +167,9 @@ impl ToCss for QueryCondition {
 
 /// <https://drafts.csswg.org/css-syntax-3/#typedef-any-value>
 fn consume_any_value<'i, 't>(input: &mut Parser<'i, 't>) -> Result<(), ParseError<'i>> {
-    input.expect_no_error_token().map_err(|err| err.into())
+    input.expect_no_error_token().map_err(Into::into)
 }
 
-/// TODO: style() case needs to be handled.
 impl QueryCondition {
     /// Parse a single condition.
     pub fn parse<'i, 't>(
@@ -115,19 +177,7 @@ impl QueryCondition {
         input: &mut Parser<'i, 't>,
         feature_type: FeatureType,
     ) -> Result<Self, ParseError<'i>> {
-        input.skip_whitespace();
+        Self::parse_internal(context, input, feature_type, AllowOr::Yes)
-        let state = input.state();
-        let start = input.position();
-        match *input.next()? {
-            Token::Function(_) => {
-                input.parse_nested_block(consume_any_value)?;
-                return Ok(QueryCondition::GeneralEnclosed(input.slice_from(start).to_owned()));
-            },
-            _ => {
-                input.reset(&state);
-                Self::parse_internal(context, input, feature_type, AllowOr::Yes)
-            },
-        }
     }
 
     fn visit<F>(&self, visitor: &mut F)
@@ -171,6 +221,9 @@ impl QueryCondition {
         Self::parse_internal(context, input, feature_type, AllowOr::No)
     }
 
+    /// https://drafts.csswg.org/mediaqueries-5/#typedef-media-condition or
+    /// https://drafts.csswg.org/mediaqueries-5/#typedef-media-condition-without-or
+    /// (depending on `allow_or`).
     fn parse_internal<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -214,34 +267,61 @@ impl QueryCondition {
         }
     }
 
-    /// Parse a condition in parentheses.
+    fn parse_in_parenthesis_block<'i>(
+        context: &ParserContext,
+        input: &mut Parser<'i, '_>,
+        feature_type: FeatureType,
+    ) -> Result<Self, ParseError<'i>> {
+        // Base case. Make sure to preserve this error as it's more generally
+        // relevant.
+        let feature_error = match input.try_parse(|input| {
+            QueryFeatureExpression::parse_in_parenthesis_block(context, input, feature_type)
+        }) {
+            Ok(expr) => return Ok(Self::Feature(expr)),
+            Err(e) => e,
+        };
+        if let Ok(inner) = Self::parse(context, input, feature_type) {
+            return Ok(Self::InParens(Box::new(inner)));
+        }
+        Err(feature_error)
+    }
+
+    /// Parse a condition in parentheses, or `<general-enclosed>`.
+    ///
+    /// https://drafts.csswg.org/mediaqueries/#typedef-media-in-parens
     pub fn parse_in_parens<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         feature_type: FeatureType,
     ) -> Result<Self, ParseError<'i>> {
-        input.expect_parenthesis_block()?;
+        input.skip_whitespace();
-        Self::parse_paren_block(context, input, feature_type)
-    }
-
-    fn parse_paren_block<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-        feature_type: FeatureType,
-    ) -> Result<Self, ParseError<'i>> {
         let start = input.position();
-        input.parse_nested_block(|input| {
+        let start_location = input.current_source_location();
-            // Base case.
+        match *input.next()? {
-            if let Ok(inner) = input.try_parse(|i| Self::parse_internal(context, i, feature_type, AllowOr::Yes)) {
+            Token::ParenthesisBlock => {
-                return Ok(QueryCondition::InParens(Box::new(inner)));
+                let nested = input.try_parse(|input| {
-            }
+                    input.parse_nested_block(|input| {
-            if let Ok(expr) = QueryFeatureExpression::parse_in_parenthesis_block(context, input, feature_type) {
+                        Self::parse_in_parenthesis_block(context, input, feature_type)
-                return  Ok(QueryCondition::Feature(expr));
+                    })
-            }
+                });
-
+                match nested {
-            consume_any_value(input)?;
+                    Ok(nested) => return Ok(nested),
-            Ok(QueryCondition::GeneralEnclosed(input.slice_from(start).to_owned()))
+                    Err(e) => {
-        })
+                        // We're about to swallow the error in a `<general-enclosed>`
+                        // condition, so report it while we can.
+                        let loc = e.location;
+                        let error = ContextualParseError::InvalidMediaRule(input.slice_from(start), e);
+                        context.log_css_error(loc, error);
+                    }
+                }
+            },
+            Token::Function(..) => {
+                // TODO: handle `style()` queries, etc.
+            },
+            ref t => return Err(start_location.new_unexpected_token_error(t.clone())),
+        }
+        input.parse_nested_block(consume_any_value)?;
+        Ok(Self::GeneralEnclosed(input.slice_from(start).to_owned()))
     }
 
     /// Whether this condition matches the device and quirks mode.
@@ -249,67 +329,35 @@ impl QueryCondition {
     /// https://drafts.csswg.org/mediaqueries/#typedef-general-enclosed
     /// Kleene 3-valued logic is adopted here due to the introduction of
     /// <general-enclosed>.
-    pub fn matches(&self, context: &computed::Context) ->  KleeneValue {
+    pub fn matches(&self, context: &computed::Context) -> KleeneValue {
         match *self {
-            QueryCondition::Feature(ref f) => {
+            QueryCondition::Feature(ref f) => KleeneValue::from(f.matches(context)),
-                match f.matches(context) {
-                    true => KleeneValue::True,
-                    false => KleeneValue::False,
-                }
-            },
             QueryCondition::GeneralEnclosed(_) => KleeneValue::Unknown,
             QueryCondition::InParens(ref c) => c.matches(context),
-            QueryCondition::Not(ref c) => {
+            QueryCondition::Not(ref c) => !c.matches(context),
-                !c.matches(context)
-            },
             QueryCondition::Operation(ref conditions, op) => {
-                let mut iter = conditions.iter();
+                debug_assert!(!conditions.is_empty(), "We never create an empty op");
                 match op {
                     Operator::And => {
-                        if conditions.is_empty() {
+                        let mut result = KleeneValue::True;
-                            return KleeneValue::True;
+                        for c in conditions.iter() {
-                        }
+                            result &= c.matches(context);
-
+                            if result == KleeneValue::False {
-                        let mut result = iter.next().as_ref().map_or( KleeneValue::True, |c| -> KleeneValue {c.matches(context)});
+                                break;
-                        if result == KleeneValue::False {
-                            return result;
-                        }
-                        while let Some(c) = iter.next() {
-                            match c.matches(context) {
-                                KleeneValue::False => {
-                                    return KleeneValue::False;
-                                },
-                                KleeneValue::Unknown => {
-                                    result = KleeneValue::Unknown;
-                                },
-                                KleeneValue::True => {},
                             }
                         }
-                        return result;
+                        result
-                    }
+                    },
                     Operator::Or => {
-                        if conditions.is_empty() {
+                        let mut result = KleeneValue::False;
-                            return KleeneValue::False;
+                        for c in conditions.iter() {
-                        }
+                            result |= c.matches(context);
-
+                            if result == KleeneValue::True {
-                        let mut result = iter.next().as_ref().map_or( KleeneValue::False, |c| -> KleeneValue {c.matches(context)});
+                                break;
-                        if result == KleeneValue::True {
-                            return KleeneValue::True;
-                        }
-                        while let Some(c) = iter.next() {
-                            match c.matches(context) {
-                                KleeneValue::True => {
-                                    return KleeneValue::True;
-                                },
-                                KleeneValue::Unknown => {
-                                    result = KleeneValue::Unknown;
-                                },
-                                KleeneValue::False => {},
-
                             }
                         }
-                        return result;
+                        result
-                    }
+                    },
                 }
             },
         }

components/style/queries/feature_expression.rs

@@ -17,7 +17,6 @@ use cssparser::{Parser, Token};
 use std::cmp::{Ordering, PartialOrd};
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
-use crate::error_reporting::ContextualParseError;
 
 /// Whether we're parsing a media or container query feature.
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)]
@@ -503,13 +502,11 @@ impl QueryFeatureExpression {
     }
 
     /// Parse a feature expression where we've already consumed the parenthesis.
-    /// For unknown features, we keeps the warning even though they might be parsed "correctly" because of <general-enclosed>
     pub fn parse_in_parenthesis_block<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
         feature_type: FeatureType,
     ) -> Result<Self, ParseError<'i>> {
-        let start_position = input.position();
         let (feature_index, range) =
             match input.try_parse(|input| Self::parse_feature_name(context, input, feature_type)) {
                 Ok(v) => v,
@@ -517,12 +514,6 @@ impl QueryFeatureExpression {
                     if let Ok(expr) = Self::parse_multi_range_syntax(context, input, feature_type) {
                         return Ok(expr);
                     }
-                    let location = e.location;
-                    let error = ContextualParseError::UnsupportedRule(
-                        input.slice_from(start_position),
-                        e.clone(),
-                    );
-                    context.log_css_error(location, error);
                     return Err(e);
                 },
             };

components/style/stylist.rs

@@ -19,7 +19,6 @@ use crate::invalidation::stylesheets::RuleChangeKind;
 use crate::media_queries::Device;
 use crate::properties::{self, CascadeMode, ComputedValues};
 use crate::properties::{AnimationDeclarations, PropertyDeclarationBlock};
-use crate::queries::condition::KleeneValue;
 use crate::rule_cache::{RuleCache, RuleCacheConditions};
 use crate::rule_collector::{containing_shadow_ignoring_svg_use, RuleCollector};
 use crate::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
@@ -2381,13 +2380,8 @@ impl CascadeData {
                 None => return true,
                 Some(ref c) => c,
             };
-            let result = match !condition.matches(stylist.device(), element, &mut context.extra_data.cascade_input_flags) {
+            let matches = condition.matches(stylist.device(), element, &mut context.extra_data.cascade_input_flags).to_bool(/* unknown = */ false);
-                KleeneValue::True => true,
+            if !matches {
-                KleeneValue::False => false,
-                KleeneValue::Unknown => true,
-            };
-
-            if result {
                 return false;
             }
             id = condition_ref.parent;

components/style/values/specified/source_size_list.rs

@@ -9,7 +9,6 @@ use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
 use crate::media_queries::Device;
 use crate::parser::{Parse, ParserContext};
 use crate::queries::{FeatureType, QueryCondition};
-use crate::queries::condition::KleeneValue;
 use crate::values::computed::{self, ToComputedValue};
 use crate::values::specified::{Length, NoCalcLength, ViewportPercentageLength};
 use app_units::Au;
@@ -58,15 +57,10 @@ impl SourceSizeList {
     /// Evaluate this <source-size-list> to get the final viewport length.
     pub fn evaluate(&self, device: &Device, quirks_mode: QuirksMode) -> Au {
         computed::Context::for_media_query_evaluation(device, quirks_mode, |context| {
-                let matching_source_size = self
+            let matching_source_size = self
                 .source_sizes
                 .iter()
-                .find(|source_size| match source_size.condition.matches(context) {
+                .find(|source_size| source_size.condition.matches(context).to_bool(/* unknown = */ false));
-                    KleeneValue::Unknown => false,
-                    KleeneValue::False => false,
-                    KleeneValue::True => true,
-                }
-            );
 
             match matching_source_size {
                 Some(source_size) => source_size.value.to_computed_value(context),