Chromium Code Reviews Chromium Code Reviews
Issue 2614683002:
CSS Animations: Fix flaky change-keyframes-name.html (Closed)
| Index: third_party/WebKit/LayoutTests/animations/change-keyframes-name.html |
| diff --git a/third_party/WebKit/LayoutTests/animations/change-keyframes-name.html b/third_party/WebKit/LayoutTests/animations/change-keyframes-name.html |
| index 909f7b16a1436c6ba4db90436e158f362d9cd956..c6625c8e95ab58c6a6dae93a5eaf91c7681282db 100644 |
| --- a/third_party/WebKit/LayoutTests/animations/change-keyframes-name.html |
| +++ b/third_party/WebKit/LayoutTests/animations/change-keyframes-name.html |
| @@ -1,81 +1,62 @@ |
| -<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" |
| - "http://www.w3.org/TR/html4/loose.dtd"> |
| - |
| -<html lang="en"> |
| +<!DOCTYPE html> |
| +<html> |
| <head> |
| - <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> |
| + <meta charset="utf-8"> |
| <title>Test Changing Name of A Keyframes Rule Using CSSOM</title> |
| - <style type="text/css" media="screen"> |
| - #box { |
| - position: relative; |
| - left: 0; |
| - top: 0; |
| - height: 100px; |
| - width: 100px; |
| - background-color: blue; |
| - -webkit-animation-duration: 0.5s; |
| - -webkit-animation-timing-function: linear; |
| - -webkit-animation-name: bar; |
| + <script src="../resources/testharness.js"></script> |
| + <script src="../resources/testharnessreport.js"></script> |
| + <style> |
| + #target { |
| + position: absolute; |
| + left: 0; |
| + top: 0; |
| + height: 100px; |
| + width: 100px; |
| + background-color: blue; |
| + animation-duration: 6s; |
| + animation-timing-function: linear; |
| + animation-name: bar; |
|
suzyh_UTC10 (ex-contributor)
2017/01/05 05:53:43
Is this animation-name property actually needed?
Eric Willigers
2017/01/05 06:11:59
The different name makes clear that we are later c
|
| } |
| - @-webkit-keyframes foo { |
| - from { left: 100px; } |
| - 40% { left: 200px; } |
| - 60% { left: 200px; } |
| - to { left: 300px; } |
| + @keyframes foo { |
| + from { left: 100px; } |
| + 50% { left: 400px; } |
| + to { left: 100px; } |
| } |
| - </style> |
| - <script src="resources/animation-test-helpers.js" type="text/javascript" charset="utf-8"></script> |
| - <script type="text/javascript" charset="utf-8"> |
| - |
| - const expectedValues = [ |
| - // [time, element-id, property, expected-value, tolerance] |
| - [0.25, "box", "left", 200, 20], |
| - ]; |
| + </style> |
| +</head> |
| +<body> |
| + <div id="target"></div> |
| + <script> |
| + 'use strict'; |
| function findKeyframesRule(rule) |
| { |
| - var ss = document.styleSheets; |
| - for (var i = 0; i < ss.length; ++i) { |
| - for (var j = 0; j < ss[i].cssRules.length; ++j) { |
| - if (ss[i].cssRules[j].type == window.CSSRule.WEBKIT_KEYFRAMES_RULE && ss[i].cssRules[j].name == rule) |
| - return ss[i].cssRules[j]; |
| - } |
| + var ss = document.styleSheets; |
| + for (var i = 0; i < ss.length; ++i) { |
| + for (var j = 0; j < ss[i].cssRules.length; ++j) { |
| + if (ss[i].cssRules[j].type == window.CSSRule.KEYFRAMES_RULE && ss[i].cssRules[j].name == rule) |
| + return ss[i].cssRules[j]; |
| } |
| - |
| - return null; |
| - } |
| - |
| - function change() |
| - { |
| - // change keyframes name |
| - var keyframes = findKeyframesRule("foo"); |
| - keyframes.name = "anim"; |
| - document.getElementById('box').style.webkitAnimationName = "anim"; |
| + } |
| - runAnimationTest(expectedValues, null, null, 'do-not-use-pause-api'); |
| + return null; |
| } |
| - |
| - function setup() |
| - { |
| - if (window.testRunner) { |
| - testRunner.dumpAsText(); |
| - testRunner.waitUntilDone(); |
| - } |
| - requestAnimationFrame(() => { |
| - requestAnimationFrame(change); |
| - }); |
| - } |
| - |
| + |
| + async_test(t => { |
| + target.offsetTop; // Force style recalc |
| + |
| + // change keyframes name |
| + var keyframes = findKeyframesRule('foo'); |
| + keyframes.name = 'anim'; |
|
suzyh_UTC10 (ex-contributor)
2017/01/05 05:53:43
Why does this get set here? Isn't 'foo' the existi
Eric Willigers
2017/01/05 06:11:59
I think much of the purpose of this test is updati
|
| + target.style.animationName = 'anim'; |
| + |
| + // Wait so the animation can progress. |
| + setTimeout(function() { |
| + requestAnimationFrame(t.step_func_done(() => { |
|
suzyh_UTC10 (ex-contributor)
2017/01/05 05:53:43
I thought the combination of setTimeout and reques
Eric Willigers
2017/01/05 06:11:59
setTimeout alone often leads to flakiness. Without
|
| + assert_greater_than(parseFloat(getComputedStyle(target).left), 100); |
| + })); |
| + }, 50); |
| + }, 'Changing Name of A Keyframes Rule Using CSSOM starts animation'); |
| </script> |
| -</head> |
| -<body onload="setup()"> |
| -This test changes the name of the @keyframes rule so that it matches |
| -and makes sure the animation starts running. |
| -<div id="box"> |
| -</div> |
| -<div id="pre-result"> |
| -</div> |
| -<div id="result"> |
| -</div> |
| </body> |
| </html> |
