Bug 1705517 [wpt PR 28525] - Revert "Include custom properties on computed CSSStyleDeclaration", a=testonly
authorAnders Hartvoll Ruud <andruud@chromium.org>
Fri, 23 Apr 2021 10:20:05 +0000
changeset 577247 128bb9ccf9c9da8e4c55388779c39e01b38e2092
parent 577246 cb179cb7a5739ccc91d8acbff69600de5950c63a
child 577248 3aa2e813a4e3b2ae70351c636e7226f8d8bd3012
push id38403
push usercsabou@mozilla.com
push dateSat, 24 Apr 2021 09:00:50 +0000
treeherdermozilla-central@3a7d9d49c316 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1705517, 28525, 949807, 1199142, 2822260, 872446, 2829151, 873035
milestone90.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 1705517 [wpt PR 28525] - Revert "Include custom properties on computed CSSStyleDeclaration", a=testonly Automatic update from web-platform-tests Revert "Include custom properties on computed CSSStyleDeclaration" This reverts commit 3b8999adcdc905f13b22bbde6b198f158cb9d6c7. Reason for revert: Huge performance regressions. Bug: 949807 Fixed: 1199142 Original change's description: > Include custom properties on computed CSSStyleDeclaration > > Since we need a "stable" list of custom properties for length() > and item(), and we also need not-abysmal performance when calling > those functions, this CL caches a vector with the variables names > on ComputedStyle itself. > > In CSSComputedStyleDeclaration::item(), we check if the incoming > index is in the range of the standard properties, and if so return > the appropriate one. Otherwise, we're in the variable range (which > is defined per spec to appear after the standard properties), and > we'll fetch the list of variable names. > > Unfortunately the inclusion of custom properties requires a clean > style, which means length()/item() now updates style/layout as needed. > This might cause performance regressions, but I don't see a way around > this. > > Note: ComputedStyle::StyleInheritedVariables/NonInheritedVariables > were changed to return const pointers, as to not "leak mutability", > which would have made it hard to invalidate the cache. (Nothing > requires those return values to be non-const after StyleCascade > anyway). > > Note: ComputedStyle::GetVariableNames() has pretty good test coverage > in ComputedStyleTest already. Added a couple of new ones that target > cache invalidation. > > Fixed: 949807 > Change-Id: I6b181af5c4f025c4fd433de0a6ca221055c16693 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822260 > Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org> > Reviewed-by: Rune Lillesveen <futhark@chromium.org> > Cr-Commit-Position: refs/heads/master@{#872446} Change-Id: I071cc2e00838a17407fc0d1e5745db9894d59ceb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2829151 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#873035} -- wpt-commits: f3cbe18b3432c3cd16d6e3ca35c5277cf4fb92d4 wpt-pr: 28525
testing/web-platform/tests/css/css-typed-om/the-stylepropertymap/computed/computed.tentative.html
testing/web-platform/tests/css/cssom/cssstyledeclaration-registered-custom-properties.html
--- a/testing/web-platform/tests/css/css-typed-om/the-stylepropertymap/computed/computed.tentative.html
+++ b/testing/web-platform/tests/css/css-typed-om/the-stylepropertymap/computed/computed.tentative.html
@@ -14,17 +14,18 @@
 
 const target = document.getElementById('target');
 const styleMap = target.computedStyleMap();
 
 test(() => {
   const computedStyle = [...getComputedStyle(target)].sort();
   const properties = [...styleMap.keys()];
 
-  assert_equals(properties.length, computedStyle.length);
+  // Two extra entries for custom properties
+  assert_equals(properties.length, computedStyle.length + 2);
   for (let i = 0; i < computedStyle.length; i++) {
     assert_true(properties.includes(computedStyle[i]));
     assert_not_equals(styleMap.get(computedStyle[i]), null);
     assert_not_equals(styleMap.getAll(computedStyle[i]).length, 0);
     assert_true(styleMap.has(computedStyle[i]));
   }
 }, 'Computed StylePropertyMap contains every CSS property');
 
deleted file mode 100644
--- a/testing/web-platform/tests/css/cssom/cssstyledeclaration-registered-custom-properties.html
+++ /dev/null
@@ -1,58 +0,0 @@
-<!DOCTYPE html>
-<title>Computed CSSStyleDeclaration includes registered custom properties</title>
-<link rel="help" href="https://drafts.csswg.org/cssom/#dom-window-getcomputedstyle">
-<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/1316">
-<script src="/resources/testharness.js"></script>
-<script src="/resources/testharnessreport.js"></script>
-<style>
-  @property --non-inherited-length {
-    syntax: "<length>";
-    inherits: false;
-    initial-value: 0px;
-  }
-  @property --inherited-length {
-    syntax: "<length>";
-    inherits: true;
-    initial-value: 0px;
-  }
-  @property --universal-with-initial {
-    syntax: "*";
-    inherits: false;
-    initial-value: foo;
-  }
-  @property --universal-without-initial {
-    syntax: "*";
-    inherits: false;
-  }
-  #outer { --non-registered-outer: 1px; }
-  #inner { --non-registered-inner: 2px; }
-  #sibling { --universal-without-initial: bar; }
-</style>
-<div id=outer>
-  <div id=inner></div>
-  <div id=sibling></div>
-</div>
-<script>
-  const assert_present = (props, name) => assert_not_equals(props.indexOf(name), -1);
-  const assert_absent = (props, name) => assert_equals(props.indexOf(name), -1);
-
-  test(function() {
-    let props = Array.from(getComputedStyle(inner));
-    assert_present(props, '--non-inherited-length');
-    assert_present(props, '--inherited-length');
-    assert_present(props, '--non-registered-outer');
-    assert_present(props, '--non-registered-inner');
-    assert_present(props, '--universal-with-initial');
-    assert_absent(props, '--universal-without-initial');
-  }, 'Registered custom properties are included in CSSComputedStyleDeclaration');
-
-  test(function() {
-    let props = Array.from(getComputedStyle(sibling));
-    assert_present(props, '--non-inherited-length');
-    assert_present(props, '--inherited-length');
-    assert_present(props, '--non-registered-outer');
-    assert_present(props, '--universal-with-initial');
-    assert_present(props, '--universal-without-initial');
-    assert_absent(props, '--non-registered-inner');
-  }, 'Only relevant custom properties are included');
-</script>