Bug 1743150 - Shorthands in computed style shouldn't be enumerable. r=layout-reviewers,jfkthame

This is as per spec, and causes failures once we enable the pref to expose
shorthands in the computed style. We need to decouple the "enabled-ness" from
the enumerable-ness, which we do by keeping two separate arrays, and leaving
ComputedStyleMap for the enumeration.

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

layout/style/GenerateComputedDOMStyleGenerated.py

@@ -4,21 +4,24 @@
 
 import runpy
 
-
-def generate(output, dataFile):
-    output.write(
-        """/* THIS IS AN AUTOGENERATED FILE.  DO NOT EDIT */
+FILE = """/* THIS IS AN AUTOGENERATED FILE.  DO NOT EDIT */
 
 /* processed file that defines entries for nsComputedDOMStyle, designed
    to be #included in nsComputedDOMStyle.cpp */
 
-static constexpr Entry kEntries[] = {
+static constexpr size_t kEntryIndices[eCSSProperty_COUNT] = {{
+  {indices}
+}};
+
+{asserts}
+
+static constexpr Entry kEntries[eCSSProperty_COUNT] = {{
+  {entries}
+}};
 """
-    )
 
-    def exposed_on_getcs(p):
-        return "ExposedOnGetCS" in p.flags
 
+def generate(output, dataFile):
     def order_key(p):
         # Put prefixed properties after normal properties.
         # The spec is unclear about this, and Blink doesn't have any sensible
@@ -28,6 +31,8 @@ static constexpr Entry kEntries[] = {
         return (order, p.name)
 
     def has_cpp_getter(p):
+        if not "ExposedOnGetCS" in p.flags:
+            return False
         if "SerializedByServo" in p.flags:
             return False
         if p.type() == "longhand" and "IsLogical" in p.flags:
@@ -43,10 +48,34 @@ static constexpr Entry kEntries[] = {
         return "DummyGetter"
 
     properties = runpy.run_path(dataFile)["data"]
-    properties = filter(exposed_on_getcs, properties)
 
-    TEMPLATE = "  {{ eCSSProperty_{}, &nsComputedDOMStyle::{} }},\n"
-    for p in sorted(properties, key=order_key):
-        output.write(TEMPLATE.format(p.id, getter_entry(p)))
+    entries = []
+    indices = []
+    asserts = []
+    index_map = {}
+    non_aliases = list(filter(lambda p: p.type() != "alias", properties))
+    for i, p in enumerate(sorted(non_aliases, key=order_key)):
+        can_be_exposed = "true" if "ExposedOnGetCS" in p.flags else "false"
+        entries.append(
+            "{{ eCSSProperty_{}, {}, &nsComputedDOMStyle::{}}}".format(
+                p.id, can_be_exposed, getter_entry(p)
+            )
+        )
+        index_map[p.id] = i
+        i += 1
 
-    output.write("};\n")
+    for i, p in enumerate(non_aliases):
+        indices.append(str(index_map[p.id]))
+        asserts.append(
+            'static_assert(size_t(eCSSProperty_{}) == {}, "");'.format(p.id, i)
+        )
+
+    assert len(indices) == len(entries)
+
+    output.write(
+        FILE.format(
+            indices=", ".join(indices),
+            entries=",\n    ".join(entries),
+            asserts="\n".join(asserts),
+        )
+    )

layout/style/nsComputedDOMStyle.cpp

@@ -206,13 +206,24 @@ struct ComputedStyleMap {
 
   struct Entry {
     // Create a pointer-to-member-function type.
-    typedef already_AddRefed<CSSValue> (nsComputedDOMStyle::*ComputeMethod)();
+    using ComputeMethod = already_AddRefed<CSSValue> (nsComputedDOMStyle::*)();
 
     nsCSSPropertyID mProperty;
-    ComputeMethod mGetter;
+
+    // Whether the property can ever be exposed in getComputedStyle(). For
+    // example, @page descriptors implemented as CSS properties or other
+    // internal properties, would have this flag set to `false`.
+    bool mCanBeExposed = false;
+
+    ComputeMethod mGetter = nullptr;
+
+    bool IsEnumerable() const {
+      return IsEnabled() && !nsCSSProps::IsShorthand(mProperty);
+    }
 
     bool IsEnabled() const {
-      if (!nsCSSProps::IsEnabled(mProperty, CSSEnabledState::ForAllContent)) {
+      if (!mCanBeExposed ||
+          !nsCSSProps::IsEnabled(mProperty, CSSEnabledState::ForAllContent)) {
         return false;
       }
       if (nsCSSProps::IsShorthand(mProperty) &&
@@ -234,7 +245,7 @@ struct ComputedStyleMap {
    */
   uint32_t Length() {
     Update();
-    return mExposedPropertyCount;
+    return mEnumerablePropertyCount;
   }
 
   /**
@@ -253,21 +264,23 @@ struct ComputedStyleMap {
    * or is currently disabled.
    */
   const Entry* FindEntryForProperty(nsCSSPropertyID aPropID) {
-    Update();
-    for (uint32_t i = 0; i < mExposedPropertyCount; i++) {
-      const Entry* entry = &kEntries[EntryIndex(i)];
-      if (entry->mProperty == aPropID) {
-        return entry;
-      }
+    if (size_t(aPropID) >= ArrayLength(kEntryIndices)) {
+      MOZ_ASSERT(aPropID == eCSSProperty_UNKNOWN);
+      return nullptr;
     }
-    return nullptr;
+    MOZ_ASSERT(kEntryIndices[aPropID] < ArrayLength(kEntries));
+    const auto& entry = kEntries[kEntryIndices[aPropID]];
+    if (!entry.IsEnabled()) {
+      return nullptr;
+    }
+    return &entry;
   }
 
   /**
    * Records that mIndexMap needs updating, due to prefs changing that could
    * affect the set of properties exposed on an nsComputedDOMStyle.
    */
-  void MarkDirty() { mExposedPropertyCount = 0; }
+  void MarkDirty() { mEnumerablePropertyCount = 0; }
 
   // The member variables are public so that we can use an initializer in
   // nsComputedDOMStyle::GetComputedStyleMap.  Use the member functions
@@ -279,7 +292,7 @@ struct ComputedStyleMap {
    * prefs are disabled.  A value of 0 indicates that it and mIndexMap are out
    * of date.
    */
-  uint32_t mExposedPropertyCount;
+  uint32_t mEnumerablePropertyCount = 0;
 
   /**
    * A map of indexes on the nsComputedDOMStyle object to indexes into kEntries.
@@ -288,13 +301,13 @@ struct ComputedStyleMap {
 
  private:
   /**
-   * Returns whether mExposedPropertyCount and mIndexMap are out of date.
+   * Returns whether mEnumerablePropertyCount and mIndexMap are out of date.
    */
-  bool IsDirty() { return mExposedPropertyCount == 0; }
+  bool IsDirty() { return mEnumerablePropertyCount == 0; }
 
   /**
-   * Updates mExposedPropertyCount and mIndexMap to take into account properties
-   * whose prefs are currently disabled.
+   * Updates mEnumerablePropertyCount and mIndexMap to take into account
+   * properties whose prefs are currently disabled.
    */
   void Update();
 
@@ -302,7 +315,7 @@ struct ComputedStyleMap {
    * Maps an nsComputedDOMStyle indexed getter index to an index into kEntries.
    */
   uint32_t EntryIndex(uint32_t aIndex) const {
-    MOZ_ASSERT(aIndex < mExposedPropertyCount);
+    MOZ_ASSERT(aIndex < mEnumerablePropertyCount);
     return mIndexMap[aIndex];
   }
 };
@@ -310,6 +323,8 @@ struct ComputedStyleMap {
 constexpr ComputedStyleMap::Entry
     ComputedStyleMap::kEntries[ArrayLength(kEntries)];
 
+constexpr size_t ComputedStyleMap::kEntryIndices[ArrayLength(kEntries)];
+
 void ComputedStyleMap::Update() {
   if (!IsDirty()) {
     return;
@@ -317,11 +332,11 @@ void ComputedStyleMap::Update() {
 
   uint32_t index = 0;
   for (uint32_t i = 0; i < ArrayLength(kEntries); i++) {
-    if (kEntries[i].IsEnabled()) {
+    if (kEntries[i].IsEnumerable()) {
       mIndexMap[index++] = i;
     }
   }
-  mExposedPropertyCount = index;
+  mEnumerablePropertyCount = index;
 }
 
 nsComputedDOMStyle::nsComputedDOMStyle(dom::Element* aElement,