Bug 1552344 - Add a test that tests computed style diffing using the property database. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 17 May 2019 04:37:44 +0200
changeset 474337 159848724347565e9f92d8049d2a847e754faed6
parent 474336 418d8f9bd4b14313481ceeb41462144aa9c61100
child 474338 865768838154812b6afd52a0ced86fe582953d6f
child 474364 1ae707852b608ea77dc82c892f25e169cbc316b5
push id113147
push useremilio@crisal.io
push dateFri, 17 May 2019 18:37:06 +0000
treeherdermozilla-inbound@159848724347 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1552344
milestone68.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 1552344 - Add a test that tests computed style diffing using the property database. r=jwatt Depends on D31569 Differential Revision: https://phabricator.services.mozilla.com/D31570
layout/style/test/mochitest.ini
layout/style/test/test_computed_style_difference.html
--- a/layout/style/test/mochitest.ini
+++ b/layout/style/test/mochitest.ini
@@ -175,16 +175,17 @@ support-files = file_bug1443344.css
 [test_ch_ex_no_infloops.html]
 [test_change_hint_optimizations.html]
 [test_clip-path_polygon.html]
 [test_color_rounding.html]
 [test_compute_data_with_start_struct.html]
 skip-if = toolkit == 'android'
 [test_computed_style.html]
 [test_computed_style_bfcache_display_none.html]
+[test_computed_style_difference.html]
 [test_computed_style_grid_with_pseudo.html]
 [test_computed_style_in_created_document.html]
 [test_computed_style_min_size_auto.html]
 [test_computed_style_no_flush.html]
 [test_computed_style_no_pseudo.html]
 [test_computed_style_prefs.html]
 [test_condition_text.html]
 [test_condition_text_assignment.html]
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_computed_style_difference.html
@@ -0,0 +1,104 @@
+<!doctype html>
+<title>Test that the difference of the computed style of an element is always correctly propagated</title>
+<!--
+  There are CSS property changes that don't have an effect in computed style.
+
+  It's relatively easy to return `nsChangeHint(0)` for the case where the
+  property changes but it should have no rendering difference.
+
+  That's however incorrect, since if it's an inherited property, or a
+  descendant explicitly inherits it, we should still propagate the change
+  downwards.
+
+  This test tests that computed style diffing is correct.
+-->
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="property_database.js"></script>
+<div id="outer">
+  <div id="inner"></div>
+</div>
+<script>
+// We need to skip checking for properties for which the value returned by
+// getComputedStyle depends on the parent.
+//
+// TODO(emilio): We could test a subset of these, see below.
+const kWhitelist = [
+  // Could test display values that don't force blockification of children.
+  "display",
+
+  // Could avoid testing only the ones that have percentages.
+  "transform",
+  "transform-origin",
+  "perspective-origin",
+
+  "padding-bottom",
+  "padding-left",
+  "padding-right",
+  "padding-top",
+  "padding-inline-end",
+  "padding-inline-start",
+  "padding-block-end",
+  "padding-block-start",
+
+  "margin-bottom",
+  "margin-left",
+  "margin-right",
+  "margin-top",
+  "margin-inline-end",
+  "margin-inline-start",
+  "margin-block-end",
+  "margin-block-start",
+
+  "width",
+  "height",
+  "block-size",
+  "inline-size",
+
+  "min-height",
+  "min-width",
+  "min-block-size",
+  "min-inline-size",
+];
+
+const outer = document.getElementById("outer");
+const inner = document.getElementById("inner");
+
+function testValue(prop, value) {
+  outer.style.setProperty(prop, value);
+  const computed = getComputedStyle(outer).getPropertyValue(prop);
+  assert_equals(
+    getComputedStyle(inner).getPropertyValue(prop), computed,
+    "Didn't handle the inherited change correctly?"
+  )
+}
+
+// Note that we intentionally ignore the "prerequisites" here, since that's
+// the most likely place where the diffing could potentially go wrong.
+function testProperty(prop, info) {
+  // We only care about longhands, changing shorthands is not that interesting,
+  // since we're interested of changing as little as possible, and changing
+  // them would be equivalent to changing all the longhands at the same time.
+  if (info.type !== CSS_TYPE_LONGHAND)
+    return;
+  if (kWhitelist.includes(prop))
+    return;
+
+  inner.style.setProperty(prop, "inherit");
+  for (const v of info.initial_values)
+    testValue(prop, v);
+  for (const v of info.other_values)
+    testValue(prop, v);
+  // Test again the first value so that we test changing to it, not just from
+  // it.
+  //
+  // TODO(emilio): We could test every value against every-value if we wanted,
+  // might be worth it.
+  testValue(prop, info.initial_values[0]);
+
+  inner.style.removeProperty(prop);
+}
+
+for (let prop in gCSSProperties)
+  test(() => testProperty(prop, gCSSProperties[prop]), "Diffing for " + prop);
+</script>