Bug 1455576 part 3 - Use Servo side data to back InspectorUtils::CssPropertySupportsType. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Mon, 23 Apr 2018 20:48:34 +1000
changeset 787543 76eea9b3819f17b1338e137bd19ec8feba0bca58
parent 787542 36b938680d729709725a5872276cdf5b545891b3
child 787544 a6244c102be4c93ddb621af82ff0a915d2d368af
push id107742
push userxquan@mozilla.com
push dateTue, 24 Apr 2018 23:00:03 +0000
reviewersemilio
bugs1455576
milestone61.0a1
Bug 1455576 part 3 - Use Servo side data to back InspectorUtils::CssPropertySupportsType. r?emilio The only difference in the final result is "all" shorthand, for which the original result is wrong because "all" shorthand doesn't accept any value other than the CSS-wide keywords. MozReview-Commit-ID: BmT7kGwC0ZQ
devtools/shared/css/generated/properties-db.js
layout/inspector/InspectorUtils.cpp
layout/style/ServoBindingList.h
servo/ports/geckolib/glue.rs
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -2989,21 +2989,17 @@ exports.CSS_PROPERTIES = {
       "-moz-window-shadow",
       "-moz-window-transform",
       "-moz-window-transform-origin",
       "word-break",
       "word-spacing",
       "writing-mode",
       "z-index"
     ],
-    "supports": [
-      2,
-      4,
-      10
-    ],
+    "supports": [],
     "values": [
       "inherit",
       "initial",
       "unset"
     ]
   },
   "animation": {
     "isInherited": false,
--- a/layout/inspector/InspectorUtils.cpp
+++ b/layout/inspector/InspectorUtils.cpp
@@ -582,174 +582,30 @@ InspectorUtils::CssPropertyIsShorthand(G
   bool found;
   bool isShorthand = Servo_Property_IsShorthand(&prop, &found);
   if (!found) {
     aRv.Throw(NS_ERROR_FAILURE);
   }
   return isShorthand;
 }
 
-// A helper function that determines whether the given property
-// supports the given type.
-static bool
-PropertySupportsVariant(nsCSSPropertyID aPropertyID, uint32_t aVariant)
-{
-  if (nsCSSProps::IsShorthand(aPropertyID)) {
-    // We need a special case for border here, because while it resets
-    // border-image, it can't actually parse an image.
-    if (aPropertyID == eCSSProperty_border) {
-      return (aVariant & (VARIANT_COLOR | VARIANT_LENGTH)) != 0;
-    }
-
-    for (const nsCSSPropertyID* props = nsCSSProps::SubpropertyEntryFor(aPropertyID);
-         *props != eCSSProperty_UNKNOWN; ++props) {
-      if (PropertySupportsVariant(*props, aVariant)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  uint32_t supported = nsCSSProps::ParserVariant(aPropertyID);
-
-  // For the time being, properties that are parsed by functions must
-  // have some of their attributes hand-maintained here.
-  if (nsCSSProps::PropHasFlags(aPropertyID, CSS_PROPERTY_VALUE_PARSER_FUNCTION) ||
-      nsCSSProps::PropertyParseType(aPropertyID) == CSS_PROPERTY_PARSE_FUNCTION) {
-    // These must all be special-cased.
-    switch (aPropertyID) {
-      case eCSSProperty_border_image_slice:
-      case eCSSProperty_grid_template:
-      case eCSSProperty_grid:
-        supported |= VARIANT_PN;
-        break;
-
-      case eCSSProperty_border_image_outset:
-        supported |= VARIANT_LN;
-        break;
-
-      case eCSSProperty_border_image_width:
-      case eCSSProperty_stroke_dasharray:
-        supported |= VARIANT_LPN;
-        break;
-
-      case eCSSProperty_border_top_left_radius:
-      case eCSSProperty_border_top_right_radius:
-      case eCSSProperty_border_bottom_left_radius:
-      case eCSSProperty_border_bottom_right_radius:
-      case eCSSProperty_background_position:
-      case eCSSProperty_background_position_x:
-      case eCSSProperty_background_position_y:
-      case eCSSProperty_background_size:
-      case eCSSProperty_mask_position:
-      case eCSSProperty_mask_position_x:
-      case eCSSProperty_mask_position_y:
-      case eCSSProperty_mask_size:
-      case eCSSProperty_grid_auto_columns:
-      case eCSSProperty_grid_auto_rows:
-      case eCSSProperty_grid_template_columns:
-      case eCSSProperty_grid_template_rows:
-      case eCSSProperty_object_position:
-      case eCSSProperty_scroll_snap_coordinate:
-      case eCSSProperty_scroll_snap_destination:
-      case eCSSProperty_transform_origin:
-      case eCSSProperty_translate:
-      case eCSSProperty_perspective_origin:
-      case eCSSProperty__moz_outline_radius_topleft:
-      case eCSSProperty__moz_outline_radius_topright:
-      case eCSSProperty__moz_outline_radius_bottomleft:
-      case eCSSProperty__moz_outline_radius_bottomright:
-      case eCSSProperty__moz_window_transform_origin:
-        supported |= VARIANT_LP;
-        break;
-
-      case eCSSProperty_text_shadow:
-      case eCSSProperty_box_shadow:
-        supported |= VARIANT_LENGTH | VARIANT_COLOR;
-        break;
-
-      case eCSSProperty_border_spacing:
-        supported |= VARIANT_LENGTH;
-        break;
-
-      case eCSSProperty_cursor:
-        supported |= VARIANT_URL;
-        break;
-
-      case eCSSProperty_shape_outside:
-        supported |= VARIANT_IMAGE;
-        break;
-
-      case eCSSProperty_fill:
-      case eCSSProperty_stroke:
-        supported |= VARIANT_COLOR | VARIANT_URL;
-        break;
-
-      case eCSSProperty_image_orientation:
-      case eCSSProperty_rotate:
-        supported |= VARIANT_ANGLE;
-        break;
-
-      case eCSSProperty_filter:
-        supported |= VARIANT_URL;
-        break;
-
-      case eCSSProperty_grid_column_start:
-      case eCSSProperty_grid_column_end:
-      case eCSSProperty_grid_row_start:
-      case eCSSProperty_grid_row_end:
-      case eCSSProperty_font_weight:
-      case eCSSProperty_initial_letter:
-      case eCSSProperty_scale:
-        supported |= VARIANT_NUMBER;
-        break;
-
-      default:
-        break;
-    }
-  }
-
-  return (supported & aVariant) != 0;
-}
-
 bool
 InspectorUtils::CssPropertySupportsType(GlobalObject& aGlobalObject,
                                         const nsAString& aProperty,
                                         uint32_t aType,
                                         ErrorResult& aRv)
 {
-  nsCSSPropertyID propertyID =
-    nsCSSProps::LookupProperty(aProperty, CSSEnabledState::eForAllContent);
-  if (propertyID == eCSSProperty_UNKNOWN) {
+  NS_ConvertUTF16toUTF8 property(aProperty);
+  bool found;
+  bool result = Servo_Property_SupportsType(&property, aType, &found);
+  if (!found) {
     aRv.Throw(NS_ERROR_FAILURE);
     return false;
   }
-
-  if (propertyID >= eCSSProperty_COUNT) {
-    return false;
-  }
-
-  uint32_t variant;
-  switch (aType) {
-  case InspectorUtilsBinding::TYPE_COLOR:
-    variant = VARIANT_COLOR;
-    break;
-  case InspectorUtilsBinding::TYPE_GRADIENT:
-    variant = VARIANT_GRADIENT;
-    break;
-  case InspectorUtilsBinding::TYPE_TIMING_FUNCTION:
-    variant = VARIANT_TIMING_FUNCTION;
-    break;
-  default:
-    // Unknown type
-    aRv.Throw(NS_ERROR_NOT_AVAILABLE);
-    return false;
-  }
-
-  return PropertySupportsVariant(propertyID, variant);
+  return result;
 }
 
 /* static */ void
 InspectorUtils::GetCSSValuesForProperty(GlobalObject& aGlobalObject,
                                         const nsAString& aProperty,
                                         nsTArray<nsString>& aResult,
                                         ErrorResult& aRv)
 {
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -871,16 +871,18 @@ SERVO_BINDING_FUNC(Servo_ParseFontShorth
                    nsCSSValueBorrowedMut style,
                    nsCSSValueBorrowedMut stretch,
                    nsCSSValueBorrowedMut weight);
 
 SERVO_BINDING_FUNC(Servo_Property_IsShorthand, bool,
                    const nsACString* name, bool* found);
 SERVO_BINDING_FUNC(Servo_Property_IsInherited, bool,
                    const nsACString* name);
+SERVO_BINDING_FUNC(Servo_Property_SupportsType, bool,
+                   const nsACString* name, uint32_t ty, bool* found);
 SERVO_BINDING_FUNC(Servo_PseudoClass_GetStates, uint64_t,
                    const nsACString* name)
 
 // AddRef / Release functions
 #define SERVO_ARC_TYPE(name_, type_)                                \
   SERVO_BINDING_FUNC(Servo_##name_##_AddRef, void, type_##Borrowed) \
   SERVO_BINDING_FUNC(Servo_##name_##_Release, void, type_##Borrowed)
 #include "mozilla/ServoArcTypeList.h"
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -159,17 +159,17 @@ use style::traversal_flags::{self, Trave
 use style::values::{CustomIdent, KeyframesName};
 use style::values::animated::{Animate, Procedure, ToAnimatedZero};
 use style::values::computed::{Context, ToComputedValue};
 use style::values::distance::ComputeSquaredDistance;
 use style::values::generics::rect::Rect;
 use style::values::specified;
 use style::values::specified::gecko::{IntersectionObserverRootMargin, PixelOrPercentage};
 use style::values::specified::source_size_list::SourceSizeList;
-use style_traits::{CssWriter, ParsingMode, StyleParseErrorKind, ToCss};
+use style_traits::{CssType, CssWriter, ParsingMode, StyleParseErrorKind, ToCss};
 use super::error_reporter::ErrorReporter;
 use super::stylesheet_loader::{AsyncStylesheetParser, StylesheetLoader};
 
 /*
  * For Gecko->Servo function calls, we need to redeclare the same signature that was declared in
  * the C header in Gecko. In order to catch accidental mismatches, we run rust-bindgen against
  * those signatures as well, giving us a second declaration of all the Servo_* functions in this
  * crate. If there's a mismatch, LLVM will assert and abort, which is a rather awful thing to
@@ -970,16 +970,45 @@ pub unsafe extern "C" fn Servo_Property_
         PropertyId::LonghandAlias(id, _) => id,
         PropertyId::Shorthand(id) |
         PropertyId::ShorthandAlias(id, _) => id.longhands().next().unwrap(),
     };
     longhand_id.inherited()
 }
 
 #[no_mangle]
+pub unsafe extern "C" fn Servo_Property_SupportsType(
+    prop_name: *const nsACString,
+    ty: u32,
+    found: *mut bool,
+) -> bool {
+    let prop_id = PropertyId::parse(prop_name.as_ref().unwrap().as_str_unchecked());
+    let prop_id = match prop_id {
+        Ok(ref p) if p.enabled_for_all_content() => p,
+        _ => {
+            *found = false;
+            return false;
+        }
+    };
+
+    *found = true;
+    // This should match the constants in InspectorUtils.
+    // (Let's don't bother importing InspectorUtilsBinding into bindings
+    // because it is not used anywhere else, and issue here would be
+    // caught by the property-db test anyway.)
+    let ty = match ty {
+        1 => CssType::COLOR,
+        2 => CssType::GRADIENT,
+        3 => CssType::TIMING_FUNCTION,
+        _ => unreachable!("unknown CSS type {}", ty),
+    };
+    prop_id.supports_type(ty)
+}
+
+#[no_mangle]
 pub extern "C" fn Servo_Property_IsAnimatable(property: nsCSSPropertyID) -> bool {
     use style::properties::animated_properties;
     animated_properties::nscsspropertyid_is_animatable(property)
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_Property_IsTransitionable(property: nsCSSPropertyID) -> bool {
     use style::properties::animated_properties;