Auto merge of #15953 - bholley:size_of_stylo, r=Manishearth

Make size_of tests measure stylo and reduce the size of stylo PropertyDeclarations by 16 bytes

Right now they don't, which means that we have four properties making PropertyDeclaration 16 bytes bigger than it should be.

I'm not sure if there's a better way to get these tests to run against stylo than to hoist them into the properties file, but I couldn't figure it out. This seems good enough.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15953)
<!-- Reviewable:end -->

10 files changed, 81 insertions, 51 deletions

diff --git a/components/style/properties/data.py b/components/style/properties/data.py
index 0597662d520..57cf0eaee21 100644
--- a/components/style/properties/data.py
+++ b/components/style/properties/data.py
@@ -211,11 +211,13 @@ class PropertiesData(object):
 
         In this situation, the `product` value is ignored while choosing
         which shorthands and longhands to generate; and instead all properties for
-        which code exists for either servo or stylo are generated.
+        which code exists for either servo or stylo are generated. Note that we skip
+        this behavior when the style crate is being built in gecko mode, because we
+        need manual glue for such properties and we don't have it.
     """
     def __init__(self, product, testing):
         self.product = product
-        self.testing = testing
+        self.testing = testing and product != "gecko"
         self.style_structs = []
         self.current_style_struct = None
         self.longhands = []
diff --git a/components/style/properties/longhand/inherited_svg.mako.rs b/components/style/properties/longhand/inherited_svg.mako.rs
index 937b9237911..5c2fee9a427 100644
--- a/components/style/properties/longhand/inherited_svg.mako.rs
+++ b/components/style/properties/longhand/inherited_svg.mako.rs
@@ -119,16 +119,19 @@ ${helpers.single_keyword("clip-rule", "nonzero evenodd",
 
 ${helpers.predefined_type("marker-start", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-mid", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-end", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed = product == "gecko",
                           animatable="False",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs
index 81ce62a32e1..8049c06d05c 100644
--- a/components/style/properties/longhand/list.mako.rs
+++ b/components/style/properties/longhand/list.mako.rs
@@ -38,6 +38,7 @@ ${helpers.single_keyword("list-style-type", """
     spec="https://drafts.csswg.org/css-lists/#propdef-list-style-type")}
 
 ${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_)",
+                          boxed = product == "gecko",
                           initial_specified_value="Either::Second(None_)", animatable=False,
                           spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image")}
 
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index 7855b3e1c79..59e91f6ce8c 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -2387,17 +2387,62 @@ macro_rules! longhand_properties_idents {
     }
 }
 
-/// Retuns all longhands SpecifiedValue sizes. This is used in unit tests.
+/// Testing function to check the size of a PropertyDeclaration. We implement
+/// this here so that the code can be used by both servo and stylo unit tests.
+/// This is important because structs can have different sizes in stylo and
+/// servo.
 #[cfg(feature = "testing")]
-pub fn specified_value_sizes() -> Vec<(&'static str, usize, bool)> {
+pub fn test_size_of_property_declaration() {
     use std::mem::size_of;
-    let mut sizes = vec![];
 
+    let old = 48;
+    let new = size_of::<PropertyDeclaration>();
+    if new < old {
+        panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \
+                Good work! Please update the size in components/style/properties/properties.mako.rs.",
+                old, new)
+    } else if new > old {
+        panic!("Your changes have increased the stack size of PropertyDeclaration enum from {} to {}. \
+                These enum is present in large quantities in the style, and increasing the size \
+                may negatively affect style system performance. Please consider using `boxed=\"True\"` in \
+                the longhand If you feel that the increase is necessary, update to the new size in \
+                components/style/properties/properties.mako.rs.",
+                old, new)
+    }
+}
+
+/// Testing function to check the size of all SpecifiedValues.
+#[cfg(feature = "testing")]
+pub fn test_size_of_specified_values() {
+    use std::mem::size_of;
+    let threshold = 32;
+
+    let mut longhands = vec![];
     % for property in data.longhands:
-        sizes.push(("${property.name}",
-                    size_of::<longhands::${property.ident}::SpecifiedValue>(),
-                    ${"true" if property.boxed else "false"}));
+        longhands.push(("${property.name}",
+                       size_of::<longhands::${property.ident}::SpecifiedValue>(),
+                       ${"true" if property.boxed else "false"}));
     % endfor
 
-    sizes
+    let mut failing_messages = vec![];
+
+    for specified_value in longhands {
+        if specified_value.1 > threshold && !specified_value.2 {
+            failing_messages.push(
+                format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \
+                        currently {}. SpecifiedValues affect size of PropertyDeclaration enum and \
+                        increasing the size may negative affect style system performance. Please consider \
+                        using `boxed=\"True\"` in this longhand.",
+                        specified_value.0, specified_value.1, threshold));
+        } else if specified_value.1 <= threshold && specified_value.2 {
+            failing_messages.push(
+                format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \
+                        The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.",
+                        specified_value.0, specified_value.1, threshold));
+        }
+    }
+
+    if !failing_messages.is_empty() {
+        panic!("{}", failing_messages.join("\n\n"));
+    }
 }
diff --git a/ports/geckolib/Cargo.toml b/ports/geckolib/Cargo.toml
index a787ce685b3..9ed52128eaf 100644
--- a/ports/geckolib/Cargo.toml
+++ b/ports/geckolib/Cargo.toml
@@ -11,6 +11,7 @@ crate-type = ["staticlib", "rlib"]
 
 [features]
 bindgen = ["style/use_bindgen"]
+testing = ["style/testing"]
 
 [dependencies]
 atomic_refcell = "0.1"
diff --git a/python/servo/testing_commands.py b/python/servo/testing_commands.py
index 0733911d2b4..c00bcc42057 100644
--- a/python/servo/testing_commands.py
+++ b/python/servo/testing_commands.py
@@ -300,7 +300,7 @@ class MachCommands(CommandBase):
         release = ["--release"] if release else []
         ret = 0
         with cd(path.join("ports", "geckolib")):
-            ret = call(["cargo", "test", "-p", "stylo_tests"] + release, env=env)
+            ret = call(["cargo", "test", "-p", "stylo_tests", "--features", "testing"] + release, env=env)
         if ret != 0:
             return ret
         with cd(path.join("ports", "geckolib")):
diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs
index f50b853e827..f43163b82c6 100644
--- a/tests/unit/style/size_of.rs
+++ b/tests/unit/style/size_of.rs
@@ -2,51 +2,12 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use std::mem::size_of;
-use style::properties::{PropertyDeclaration, specified_value_sizes};
-
 #[test]
 fn size_of_property_declaration() {
-    let old = 48;
-    let new = size_of::<PropertyDeclaration>();
-    if new < old {
-        panic!("Your changes have decreased the stack size of PropertyDeclaration enum from {} to {}. \
-                Good work! Please update the size in tests/unit/style/size_of.rs.",
-                old, new)
-    } else if new > old {
-        panic!("Your changes have increased the stack size of PropertyDeclaration enum from {} to {}. \
-                These enum is present in large quantities in the style, and increasing the size \
-                may dramatically affect our memory footprint. Please consider using `boxed=\"True\"` in \
-                the longhand If you feel that the increase is necessary, update to the new size in \
-                tests/unit/style/size_of.rs.",
-                old, new)
-    }
+    ::style::properties::test_size_of_property_declaration();
 }
 
 #[test]
 fn size_of_specified_values() {
-    let threshold = 40;
-    let longhands = specified_value_sizes();
-
-    let mut failing_messages = vec![];
-
-    for specified_value in longhands {
-        if specified_value.1 >= threshold && !specified_value.2 {
-            failing_messages.push(
-                format!("Your changes have increased the size of {} SpecifiedValue to {}. The threshold is \
-                        currently {}. SpecifiedValues are affect size of PropertyDeclaration enum and \
-                        increasing the size may dramatically affect our memory footprint. Please consider \
-                        using `boxed=\"True\"` in this longhand.",
-                        specified_value.0, specified_value.1, threshold));
-        } else if specified_value.1 < threshold && specified_value.2 {
-            failing_messages.push(
-                format!("Your changes have decreased the size of {} SpecifiedValue to {}. Good work! \
-                        The threshold is currently {}. Please consider removing `boxed=\"True\"` from this longhand.",
-                        specified_value.0, specified_value.1, threshold));
-        }
-    }
-
-    if !failing_messages.is_empty() {
-        panic!("{}", failing_messages.join("\n\n"));
-    }
+    ::style::properties::test_size_of_specified_values();
 }
diff --git a/tests/unit/stylo/Cargo.toml b/tests/unit/stylo/Cargo.toml
index ef4e4669924..769e6f8e1b5 100644
--- a/tests/unit/stylo/Cargo.toml
+++ b/tests/unit/stylo/Cargo.toml
@@ -11,6 +11,9 @@ name = "stylo_tests"
 path = "lib.rs"
 doctest = false
 
+[features]
+testing = ["style/testing"]
+
 [dependencies]
 app_units = "0.4"
 atomic_refcell = "0.1"
diff --git a/tests/unit/stylo/lib.rs b/tests/unit/stylo/lib.rs
index bf09699f7ce..c89687918ef 100644
--- a/tests/unit/stylo/lib.rs
+++ b/tests/unit/stylo/lib.rs
@@ -19,6 +19,7 @@ extern crate servo_url;
 extern crate style_traits;
 
 mod sanity_checks;
+mod size_of;
 
 #[path = "../../../ports/geckolib/stylesheet_loader.rs"]
 mod stylesheet_loader;
diff --git a/tests/unit/stylo/size_of.rs b/tests/unit/stylo/size_of.rs
new file mode 100644
index 00000000000..f43163b82c6
--- /dev/null
+++ b/tests/unit/stylo/size_of.rs
@@ -0,0 +1,13 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#[test]
+fn size_of_property_declaration() {
+    ::style::properties::test_size_of_property_declaration();
+}
+
+#[test]
+fn size_of_specified_values() {
+    ::style::properties::test_size_of_specified_values();
+}