<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[190879] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/190879">190879</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2015-10-12 12:15:28 -0700 (Mon, 12 Oct 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Clip-path transitions sometimes trigger endless animation timers
https://bugs.webkit.org/show_bug.cgi?id=150018
Reviewed by Tim Horton.
Source/WebCore:
Transitioning -webkit-clip-path could trigger endless animation
timers, because when CompositeAnimation::updateTransitions() calls
isTargetPropertyEqual(), a false negative answer triggers canceling the
current transition and starting a new one over and over.
This happened because StyleRareNonInheritedData simply tested pointer
equality for m_clipPath and m_shapeOutside. Both of these need to do deep
equality testing, requiring the implementation of operator== in BasicShapes
classes.
In addition, the PropertyWrappers in CSSPropertyAnimation need equals()
implementations that also do more than pointer equality testing.
Tests: transitions/clip-path-transitions.html
transitions/shape-outside-transitions.html
* page/animation/CSSPropertyAnimation.cpp:
(WebCore::PropertyWrapperClipPath::equals):
(WebCore::PropertyWrapperShape::equals):
* rendering/ClipPathOperation.h:
* rendering/style/BasicShapes.cpp:
(WebCore::BasicShapeCircle::operator==):
(WebCore::BasicShapeEllipse::operator==):
(WebCore::BasicShapePolygon::operator==):
(WebCore::BasicShapeInset::operator==):
* rendering/style/BasicShapes.h:
(WebCore::BasicShapeCenterCoordinate::operator==):
(WebCore::BasicShapeRadius::operator==):
* rendering/style/ShapeValue.cpp:
(WebCore::pointersOrValuesEqual):
(WebCore::ShapeValue::operator==):
* rendering/style/ShapeValue.h:
(WebCore::ShapeValue::operator!=):
(WebCore::ShapeValue::operator==): Deleted.
(WebCore::ShapeValue::ShapeValue): Deleted.
* rendering/style/StyleRareNonInheritedData.cpp:
(WebCore::StyleRareNonInheritedData::operator==):
(WebCore::StyleRareNonInheritedData::clipPathOperationsEquivalent):
(WebCore::StyleRareNonInheritedData::shapeOutsideDataEquivalent):
* rendering/style/StyleRareNonInheritedData.h:
LayoutTests:
New tests for transitions of clip-path and shape-outside.
* transitions/clip-path-transitions-expected.txt: Added.
* transitions/clip-path-transitions.html: Added.
* transitions/resources/transition-test-helpers.js:
(parseClipPath):
(checkExpectedValue):
* transitions/shape-outside-transitions-expected.txt: Added.
* transitions/shape-outside-transitions.html: Added.
* transitions/svg-transitions-expected.txt:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTeststransitionsresourcestransitiontesthelpersjs">trunk/LayoutTests/transitions/resources/transition-test-helpers.js</a></li>
<li><a href="#trunkLayoutTeststransitionssvgtransitionsexpectedtxt">trunk/LayoutTests/transitions/svg-transitions-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageanimationCSSPropertyAnimationcpp">trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingClipPathOperationh">trunk/Source/WebCore/rendering/ClipPathOperation.h</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleBasicShapescpp">trunk/Source/WebCore/rendering/style/BasicShapes.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleBasicShapesh">trunk/Source/WebCore/rendering/style/BasicShapes.h</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleShapeValuecpp">trunk/Source/WebCore/rendering/style/ShapeValue.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleShapeValueh">trunk/Source/WebCore/rendering/style/ShapeValue.h</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleStyleRareNonInheritedDatacpp">trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingstyleStyleRareNonInheritedDatah">trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTeststransitionsclippathtransitionsexpectedtxt">trunk/LayoutTests/transitions/clip-path-transitions-expected.txt</a></li>
<li><a href="#trunkLayoutTeststransitionsclippathtransitionshtml">trunk/LayoutTests/transitions/clip-path-transitions.html</a></li>
<li><a href="#trunkLayoutTeststransitionsshapeoutsidetransitionsexpectedtxt">trunk/LayoutTests/transitions/shape-outside-transitions-expected.txt</a></li>
<li><a href="#trunkLayoutTeststransitionsshapeoutsidetransitionshtml">trunk/LayoutTests/transitions/shape-outside-transitions.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/LayoutTests/ChangeLog 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -1,3 +1,21 @@
</span><ins>+2015-10-12 Simon Fraser <simon.fraser@apple.com>
+
+ Clip-path transitions sometimes trigger endless animation timers
+ https://bugs.webkit.org/show_bug.cgi?id=150018
+
+ Reviewed by Tim Horton.
+
+ New tests for transitions of clip-path and shape-outside.
+
+ * transitions/clip-path-transitions-expected.txt: Added.
+ * transitions/clip-path-transitions.html: Added.
+ * transitions/resources/transition-test-helpers.js:
+ (parseClipPath):
+ (checkExpectedValue):
+ * transitions/shape-outside-transitions-expected.txt: Added.
+ * transitions/shape-outside-transitions.html: Added.
+ * transitions/svg-transitions-expected.txt:
+
</ins><span class="cx"> 2015-10-12 Ryan Haddad <ryanhaddad@apple.com>
</span><span class="cx">
</span><span class="cx"> Marking storage/domstorage/events/basic-body-attribute.html as flaky
</span></span></pre></div>
<a id="trunkLayoutTeststransitionsclippathtransitionsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/transitions/clip-path-transitions-expected.txt (0 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/clip-path-transitions-expected.txt (rev 0)
+++ trunk/LayoutTests/transitions/clip-path-transitions-expected.txt 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -0,0 +1,5 @@
</span><ins>+PASS - "-webkit-clip-path" property for "circle" element at 0.5s saw something close to: circle(60px at 50px 40px)
+PASS - "-webkit-clip-path" property for "ellipse" element at 0.5s saw something close to: ellipse(15px 30px at 50px 40px)
+PASS - "-webkit-clip-path" property for "inset" element at 0.5s saw something close to: inset(15px 22px)
+PASS - "-webkit-clip-path" property for "polygon" element at 0.5s saw something close to: polygon(15px 0px, 75px 0, 60px 45px, 0 45px)
+
</ins></span></pre></div>
<a id="trunkLayoutTeststransitionsclippathtransitionshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/transitions/clip-path-transitions.html (0 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/clip-path-transitions.html (rev 0)
+++ trunk/LayoutTests/transitions/clip-path-transitions.html 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -0,0 +1,76 @@
</span><ins>+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .box {
+ height: 100px;
+ width: 100px;
+ margin: 50px;
+ background-color: gray;
+ -webkit-transition-duration: 1s;
+ -webkit-transition-timing-function: linear;
+ -webkit-transition-property: -webkit-clip-path;
+ }
+
+ #circle {
+ -webkit-clip-path: circle(100px at 50px 30px);
+ }
+
+ body.final #circle {
+ -webkit-clip-path: circle(20px at 50px 50px);
+ }
+
+ #ellipse {
+ -webkit-clip-path: ellipse(10px 20px at 50px 30px);
+ }
+
+ body.final #ellipse {
+ -webkit-clip-path: ellipse(20px 40px at 50px 50px);
+ }
+
+ #inset {
+ -webkit-clip-path: inset(10px 20px);
+ }
+
+ body.final #inset {
+ -webkit-clip-path: inset(20px 24px);
+ }
+
+ #polygon {
+ -webkit-clip-path: polygon(20px 0px, 100px 0, 100px 50px, 0 50px);
+ }
+
+ body.final #polygon {
+ -webkit-clip-path: polygon(10px 0px, 50px 0, 20px 40px, 0 40px);
+ }
+ </style>
+ <script src="resources/transition-test-helpers.js"></script>
+ <script type="text/javascript">
+
+ const expectedValues = [
+ // [time, element-id, property, expected-value, tolerance]
+ [0.5, 'circle', '-webkit-clip-path', 'circle(60px at 50px 40px)', 1],
+ [0.5, 'ellipse', '-webkit-clip-path', 'ellipse(15px 30px at 50px 40px)', 1],
+ [0.5, 'inset', '-webkit-clip-path', 'inset(15px 22px)', 1],
+ [0.5, 'polygon', '-webkit-clip-path', 'polygon(15px 0px, 75px 0, 60px 45px, 0 45px)', 1],
+ ];
+
+ function setupTest()
+ {
+ document.body.classList.add('final');
+ }
+
+ runTransitionTest(expectedValues, setupTest, usePauseAPI);
+ </script>
+</head>
+<body>
+ <div id="circle" class="box"></div>
+ <div id="ellipse" class="box"></div>
+ <div id="inset" class="box"></div>
+ <div id="polygon" class="box"></div>
+
+ <div id="result"></div>
+
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTeststransitionsresourcestransitiontesthelpersjs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/transitions/resources/transition-test-helpers.js (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/resources/transition-test-helpers.js 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/LayoutTests/transitions/resources/transition-test-helpers.js 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -69,6 +69,29 @@
</span><span class="cx"> return {"from": matches[1], "to": matches[2], "percent": parseFloat(matches[3])}
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+function parseClipPath(s)
+{
+ // FIXME: This only matches a subset of the shape syntax, and the polygon expects 4 points.
+ var patterns = [
+ /inset\(([\d.]+)\w+ ([\d.]+)\w+\)/,
+ /circle\(([\d.]+)\w+ at ([\d.]+)\w+ ([\d.]+)\w+\)/,
+ /ellipse\(([\d.]+)\w+ ([\d.]+)\w+ at ([\d.]+)\w+ ([\d.]+)\w+\)/,
+ /polygon\(([\d.]+)\w* ([\d.]+)\w*\, ([\d.]+)\w* ([\d.]+)\w*\, ([\d.]+)\w* ([\d.]+)\w*\, ([\d.]+)\w* ([\d.]+)\w*\)/
+ ];
+
+ for (pattern of patterns) {
+ if (matchResult = s.match(pattern)) {
+ var result = [];
+ for (var i = 1; i < matchResult.length; ++i)
+ result.push(parseFloat(matchResult[i]));
+ return result;
+ }
+ }
+
+ window.console.log('failed to match ' + s);
+ return null;
+}
+
</ins><span class="cx"> function checkExpectedValue(expected, index)
</span><span class="cx"> {
</span><span class="cx"> var time = expected[index][0];
</span><span class="lines">@@ -136,6 +159,18 @@
</span><span class="cx"> } else {
</span><span class="cx"> pass = isCloseEnough(computedCrossFade.percent, expectedValue, tolerance);
</span><span class="cx"> }
</span><ins>+ } else if (property == "-webkit-clip-path" || property == "-webkit-shape-outside") {
+ computedValue = window.getComputedStyle(document.getElementById(elementId)).getPropertyCSSValue(property).cssText;
+
+ var expectedValues = parseClipPath(expectedValue);
+ var values = parseClipPath(computedValue);
+
+ pass = false;
+ if (values && values.length == expectedValues.length) {
+ pass = true
+ for (var i = 0; i < values.length; ++i)
+ pass &= isCloseEnough(values[i], expectedValues[i], tolerance);
+ }
</ins><span class="cx"> } else {
</span><span class="cx"> var computedStyle = window.getComputedStyle(document.getElementById(elementId)).getPropertyCSSValue(property);
</span><span class="cx"> if (computedStyle.cssValueType == CSSValue.CSS_VALUE_LIST) {
</span></span></pre></div>
<a id="trunkLayoutTeststransitionsshapeoutsidetransitionsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/transitions/shape-outside-transitions-expected.txt (0 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/shape-outside-transitions-expected.txt (rev 0)
+++ trunk/LayoutTests/transitions/shape-outside-transitions-expected.txt 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -0,0 +1,37 @@
</span><ins>+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+x
+PASS - "-webkit-shape-outside" property for "circle" element at 0.5s saw something close to: circle(60px at 50px 40px)
+PASS - "-webkit-shape-outside" property for "ellipse" element at 0.5s saw something close to: ellipse(15px 30px at 50px 40px)
+PASS - "-webkit-shape-outside" property for "inset" element at 0.5s saw something close to: inset(15px 22px)
+PASS - "-webkit-shape-outside" property for "polygon" element at 0.5s saw something close to: polygon(15px 0px, 75px 0, 60px 45px, 0 45px)
+
</ins></span></pre></div>
<a id="trunkLayoutTeststransitionsshapeoutsidetransitionshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/transitions/shape-outside-transitions.html (0 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/shape-outside-transitions.html (rev 0)
+++ trunk/LayoutTests/transitions/shape-outside-transitions.html 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -0,0 +1,97 @@
</span><ins>+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .container {
+ width: 150px;
+ height: 150px;
+ border: 1px solid black;
+ }
+ .box {
+ height: 100px;
+ width: 100px;
+ margin: 10px;
+ background-color: silver;
+ -webkit-transition-duration: 1s;
+ -webkit-transition-timing-function: linear;
+ -webkit-transition-property: -webkit-shape-outside;
+ float: left;
+ }
+
+ #circle {
+ -webkit-shape-outside: circle(100px at 50px 30px);
+ }
+
+ body.final #circle {
+ -webkit-shape-outside: circle(20px at 50px 50px);
+ }
+
+ #ellipse {
+ -webkit-shape-outside: ellipse(10px 20px at 50px 30px);
+ }
+
+ body.final #ellipse {
+ -webkit-shape-outside: ellipse(20px 40px at 50px 50px);
+ }
+
+ #inset {
+ -webkit-shape-outside: inset(10px 20px);
+ }
+
+ body.final #inset {
+ -webkit-shape-outside: inset(20px 24px);
+ }
+
+ #polygon {
+ -webkit-shape-outside: polygon(20px 0px, 100px 0, 100px 50px, 0 50px);
+ }
+
+ body.final #polygon {
+ -webkit-shape-outside: polygon(10px 0px, 50px 0, 20px 40px, 0 40px);
+ }
+ </style>
+ <script src="resources/transition-test-helpers.js"></script>
+ <script type="text/javascript">
+
+ const expectedValues = [
+ // [time, element-id, property, expected-value, tolerance]
+ [0.5, 'circle', '-webkit-shape-outside', 'circle(60px at 50px 40px)', 1],
+ [0.5, 'ellipse', '-webkit-shape-outside', 'ellipse(15px 30px at 50px 40px)', 1],
+ [0.5, 'inset', '-webkit-shape-outside', 'inset(15px 22px)', 1],
+ [0.5, 'polygon', '-webkit-shape-outside', 'polygon(15px 0px, 75px 0, 60px 45px, 0 45px)', 1],
+ ];
+
+ function setupTest()
+ {
+ document.body.classList.add('final');
+ }
+
+ runTransitionTest(expectedValues, setupTest, usePauseAPI);
+ </script>
+</head>
+<body>
+ <div class="container">
+ <div id="circle" class="box"></div>
+ x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
+ </div>
+
+ <div class="container">
+ <div id="ellipse" class="box"></div>
+ x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
+ </div>
+
+ <div class="container">
+ <div id="inset" class="box"></div>
+ x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
+ </div>
+
+ <div class="container">
+ <div id="polygon" class="box"></div>
+ x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
+ </div>
+
+ <div id="result"></div>
+
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTeststransitionssvgtransitionsexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/transitions/svg-transitions-expected.txt (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/transitions/svg-transitions-expected.txt 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/LayoutTests/transitions/svg-transitions-expected.txt 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -1,4 +1,4 @@
</span><del>-CONSOLE MESSAGE: line 240: Failed to pause 'fill' transition on element 'rect7'
</del><ins>+CONSOLE MESSAGE: line 275: Failed to pause 'fill' transition on element 'rect7'
</ins><span class="cx"> Example
</span><span class="cx"> PASS - "fill-opacity" property for "rect1" element at 1s saw something close to: 0.6
</span><span class="cx"> PASS - "stroke-width" property for "rect1" element at 1s saw something close to: 3
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/ChangeLog 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -1,3 +1,51 @@
</span><ins>+2015-10-12 Simon Fraser <simon.fraser@apple.com>
+
+ Clip-path transitions sometimes trigger endless animation timers
+ https://bugs.webkit.org/show_bug.cgi?id=150018
+
+ Reviewed by Tim Horton.
+
+ Transitioning -webkit-clip-path could trigger endless animation
+ timers, because when CompositeAnimation::updateTransitions() calls
+ isTargetPropertyEqual(), a false negative answer triggers canceling the
+ current transition and starting a new one over and over.
+
+ This happened because StyleRareNonInheritedData simply tested pointer
+ equality for m_clipPath and m_shapeOutside. Both of these need to do deep
+ equality testing, requiring the implementation of operator== in BasicShapes
+ classes.
+
+ In addition, the PropertyWrappers in CSSPropertyAnimation need equals()
+ implementations that also do more than pointer equality testing.
+
+ Tests: transitions/clip-path-transitions.html
+ transitions/shape-outside-transitions.html
+
+ * page/animation/CSSPropertyAnimation.cpp:
+ (WebCore::PropertyWrapperClipPath::equals):
+ (WebCore::PropertyWrapperShape::equals):
+ * rendering/ClipPathOperation.h:
+ * rendering/style/BasicShapes.cpp:
+ (WebCore::BasicShapeCircle::operator==):
+ (WebCore::BasicShapeEllipse::operator==):
+ (WebCore::BasicShapePolygon::operator==):
+ (WebCore::BasicShapeInset::operator==):
+ * rendering/style/BasicShapes.h:
+ (WebCore::BasicShapeCenterCoordinate::operator==):
+ (WebCore::BasicShapeRadius::operator==):
+ * rendering/style/ShapeValue.cpp:
+ (WebCore::pointersOrValuesEqual):
+ (WebCore::ShapeValue::operator==):
+ * rendering/style/ShapeValue.h:
+ (WebCore::ShapeValue::operator!=):
+ (WebCore::ShapeValue::operator==): Deleted.
+ (WebCore::ShapeValue::ShapeValue): Deleted.
+ * rendering/style/StyleRareNonInheritedData.cpp:
+ (WebCore::StyleRareNonInheritedData::operator==):
+ (WebCore::StyleRareNonInheritedData::clipPathOperationsEquivalent):
+ (WebCore::StyleRareNonInheritedData::shapeOutsideDataEquivalent):
+ * rendering/style/StyleRareNonInheritedData.h:
+
</ins><span class="cx"> 2015-10-12 Myles C. Maxfield <mmaxfield@apple.com>
</span><span class="cx">
</span><span class="cx"> Test font-variant-* and font-feature-settings on Yosemite and Mavericks
</span></span></pre></div>
<a id="trunkSourceWebCorepageanimationCSSPropertyAnimationcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -477,6 +477,24 @@
</span><span class="cx"> : RefCountedPropertyWrapper<ClipPathOperation>(prop, getter, setter)
</span><span class="cx"> {
</span><span class="cx"> }
</span><ins>+
+ virtual bool equals(const RenderStyle* a, const RenderStyle* b) const
+ {
+ // If the style pointers are the same, don't bother doing the test.
+ // If either is null, return false. If both are null, return true.
+ if (a == b)
+ return true;
+ if (!a || !b)
+ return false;
+
+ ClipPathOperation* clipPathA = (a->*m_getter)();
+ ClipPathOperation* clipPathB = (b->*m_getter)();
+ if (clipPathA == clipPathB)
+ return true;
+ if (!clipPathA || !clipPathB)
+ return false;
+ return *clipPathA == *clipPathB;
+ }
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> #if ENABLE(CSS_SHAPES)
</span><span class="lines">@@ -487,6 +505,24 @@
</span><span class="cx"> : RefCountedPropertyWrapper<ShapeValue>(prop, getter, setter)
</span><span class="cx"> {
</span><span class="cx"> }
</span><ins>+
+ virtual bool equals(const RenderStyle* a, const RenderStyle* b) const
+ {
+ // If the style pointers are the same, don't bother doing the test.
+ // If either is null, return false. If both are null, return true.
+ if (a == b)
+ return true;
+ if (!a || !b)
+ return false;
+
+ ShapeValue* shapeA = (a->*m_getter)();
+ ShapeValue* shapeB = (b->*m_getter)();
+ if (shapeA == shapeB)
+ return true;
+ if (!shapeA || !shapeB)
+ return false;
+ return *shapeA == *shapeB;
+ }
</ins><span class="cx"> };
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingClipPathOperationh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/ClipPathOperation.h (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/ClipPathOperation.h 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/ClipPathOperation.h 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -75,12 +75,12 @@
</span><span class="cx"> const String& fragment() const { return m_fragment; }
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- virtual bool operator==(const ClipPathOperation& o) const override
</del><ins>+ virtual bool operator==(const ClipPathOperation& other) const override
</ins><span class="cx"> {
</span><del>- if (!isSameType(o))
</del><ins>+ if (!isSameType(other))
</ins><span class="cx"> return false;
</span><del>- const ReferenceClipPathOperation* other = static_cast<const ReferenceClipPathOperation*>(&o);
- return m_url == other->m_url;
</del><ins>+ auto& referenceClip = downcast<ReferenceClipPathOperation>(other);
+ return m_url == referenceClip.m_url;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ReferenceClipPathOperation(const String& url, const String& fragment)
</span><span class="lines">@@ -118,8 +118,9 @@
</span><span class="cx"> {
</span><span class="cx"> if (!isSameType(other))
</span><span class="cx"> return false;
</span><del>- const auto& shapeClip = downcast<ShapeClipPathOperation>(other);
- return m_shape.ptr() == shapeClip.m_shape.ptr();
</del><ins>+ auto& shapeClip = downcast<ShapeClipPathOperation>(other);
+ return m_referenceBox == shapeClip.referenceBox()
+ && (m_shape.ptr() == shapeClip.m_shape.ptr() || m_shape.get() == shapeClip.m_shape.get());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> explicit ShapeClipPathOperation(Ref<BasicShape>&& shape)
</span><span class="lines">@@ -149,12 +150,12 @@
</span><span class="cx"> CSSBoxType referenceBox() const { return m_referenceBox; }
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- virtual bool operator==(const ClipPathOperation& o) const override
</del><ins>+ virtual bool operator==(const ClipPathOperation& other) const override
</ins><span class="cx"> {
</span><del>- if (!isSameType(o))
</del><ins>+ if (!isSameType(other))
</ins><span class="cx"> return false;
</span><del>- const BoxClipPathOperation* other = static_cast<const BoxClipPathOperation*>(&o);
- return m_referenceBox == other->m_referenceBox;
</del><ins>+ auto& boxClip = downcast<BoxClipPathOperation>(other);
+ return m_referenceBox == boxClip.m_referenceBox;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> explicit BoxClipPathOperation(CSSBoxType referenceBox)
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleBasicShapescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/BasicShapes.cpp (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/BasicShapes.cpp 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/BasicShapes.cpp 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -88,6 +88,18 @@
</span><span class="cx"> && thisEllipse.radiusY().canBlend(otherEllipse.radiusY()));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+
+bool BasicShapeCircle::operator==(const BasicShape& other) const
+{
+ if (type() != other.type())
+ return false;
+
+ const auto& otherCircle = downcast<BasicShapeCircle>(other);
+ return m_centerX == otherCircle.m_centerX
+ && m_centerY == otherCircle.m_centerY
+ && m_radius == otherCircle.m_radius;
+}
+
</ins><span class="cx"> float BasicShapeCircle::floatValueForRadiusInBox(float boxWidth, float boxHeight) const
</span><span class="cx"> {
</span><span class="cx"> if (m_radius.type() == BasicShapeRadius::Value)
</span><span class="lines">@@ -132,6 +144,18 @@
</span><span class="cx"> return result.releaseNonNull();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool BasicShapeEllipse::operator==(const BasicShape& other) const
+{
+ if (type() != other.type())
+ return false;
+
+ const auto& otherEllipse = downcast<BasicShapeEllipse>(other);
+ return m_centerX == otherEllipse.m_centerX
+ && m_centerY == otherEllipse.m_centerY
+ && m_radiusX == otherEllipse.m_radiusX
+ && m_radiusY == otherEllipse.m_radiusY;
+}
+
</ins><span class="cx"> float BasicShapeEllipse::floatValueForRadiusInBox(const BasicShapeRadius& radius, float center, float boxWidthOrHeight) const
</span><span class="cx"> {
</span><span class="cx"> if (radius.type() == BasicShapeRadius::Value)
</span><span class="lines">@@ -182,6 +206,16 @@
</span><span class="cx"> return result.releaseNonNull();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool BasicShapePolygon::operator==(const BasicShape& other) const
+{
+ if (type() != other.type())
+ return false;
+
+ const auto& otherPolygon = downcast<BasicShapePolygon>(other);
+ return m_windRule == otherPolygon.m_windRule
+ && m_values == otherPolygon.m_values;
+}
+
</ins><span class="cx"> void BasicShapePolygon::path(Path& path, const FloatRect& boundingBox)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(path.isEmpty());
</span><span class="lines">@@ -223,6 +257,22 @@
</span><span class="cx"> return result.releaseNonNull();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool BasicShapeInset::operator==(const BasicShape& other) const
+{
+ if (type() != other.type())
+ return false;
+
+ const auto& otherInset = downcast<BasicShapeInset>(other);
+ return m_right == otherInset.m_right
+ && m_top == otherInset.m_top
+ && m_bottom == otherInset.m_bottom
+ && m_left == otherInset.m_left
+ && m_topLeftRadius == otherInset.m_topLeftRadius
+ && m_topRightRadius == otherInset.m_topRightRadius
+ && m_bottomRightRadius == otherInset.m_bottomRightRadius
+ && m_bottomLeftRadius == otherInset.m_bottomLeftRadius;
+}
+
</ins><span class="cx"> static FloatSize floatSizeForLengthSize(const LengthSize& lengthSize, const FloatRect& boundingBox)
</span><span class="cx"> {
</span><span class="cx"> return FloatSize(floatValueForLength(lengthSize.width(), boundingBox.width()),
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleBasicShapesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/BasicShapes.h (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/BasicShapes.h 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/BasicShapes.h 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -63,6 +63,7 @@
</span><span class="cx"> virtual Ref<BasicShape> blend(const BasicShape&, double) const = 0;
</span><span class="cx">
</span><span class="cx"> virtual Type type() const = 0;
</span><ins>+ virtual bool operator==(const BasicShape&) const = 0;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> class BasicShapeCenterCoordinate {
</span><span class="lines">@@ -101,6 +102,13 @@
</span><span class="cx"> {
</span><span class="cx"> return BasicShapeCenterCoordinate(TopLeft, m_computedLength.blend(other.m_computedLength, progress));
</span><span class="cx"> }
</span><ins>+
+ bool operator==(const BasicShapeCenterCoordinate& other) const
+ {
+ return m_direction == other.m_direction
+ && m_length == other.m_length
+ && m_computedLength == other.m_computedLength;
+ }
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> Direction m_direction;
</span><span class="lines">@@ -138,6 +146,11 @@
</span><span class="cx">
</span><span class="cx"> return BasicShapeRadius(m_value.blend(other.value(), progress));
</span><span class="cx"> }
</span><ins>+
+ bool operator==(const BasicShapeRadius& other) const
+ {
+ return m_value == other.m_value && m_type == other.m_type;
+ }
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> Length m_value;
</span><span class="lines">@@ -162,6 +175,8 @@
</span><span class="cx"> virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
</span><span class="cx">
</span><span class="cx"> virtual Type type() const override { return BasicShapeCircleType; }
</span><ins>+ virtual bool operator==(const BasicShape&) const override;
+
</ins><span class="cx"> private:
</span><span class="cx"> BasicShapeCircle() { }
</span><span class="cx">
</span><span class="lines">@@ -189,6 +204,8 @@
</span><span class="cx"> virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
</span><span class="cx">
</span><span class="cx"> virtual Type type() const override { return BasicShapeEllipseType; }
</span><ins>+ virtual bool operator==(const BasicShape&) const override;
+
</ins><span class="cx"> private:
</span><span class="cx"> BasicShapeEllipse() { }
</span><span class="cx">
</span><span class="lines">@@ -215,6 +232,8 @@
</span><span class="cx"> virtual WindRule windRule() const override { return m_windRule; }
</span><span class="cx">
</span><span class="cx"> virtual Type type() const override { return BasicShapePolygonType; }
</span><ins>+ virtual bool operator==(const BasicShape&) const override;
+
</ins><span class="cx"> private:
</span><span class="cx"> BasicShapePolygon()
</span><span class="cx"> : m_windRule(RULE_NONZERO)
</span><span class="lines">@@ -252,6 +271,8 @@
</span><span class="cx"> virtual Ref<BasicShape> blend(const BasicShape&, double) const override;
</span><span class="cx">
</span><span class="cx"> virtual Type type() const override { return BasicShapeInsetType; }
</span><ins>+ virtual bool operator==(const BasicShape&) const override;
+
</ins><span class="cx"> private:
</span><span class="cx"> BasicShapeInset() { }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleShapeValuecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/ShapeValue.cpp (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/ShapeValue.cpp 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/ShapeValue.cpp 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -39,4 +39,26 @@
</span><span class="cx"> return image()->isGeneratedImage();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+template <typename T>
+bool pointersOrValuesEqual(T p1, T p2)
+{
+ if (p1 == p2)
+ return true;
+
+ if (!p1 || !p2)
+ return false;
+
+ return *p1 == *p2;
+}
+
+bool ShapeValue::operator==(const ShapeValue& other) const
+{
+ if (m_type != other.m_type || m_cssBox != other.m_cssBox)
+ return false;
+
+ return pointersOrValuesEqual(m_shape.get(), other.m_shape.get())
+ && pointersOrValuesEqual(m_image.get(), other.m_image.get());
+}
+
+
</ins><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleShapeValueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/ShapeValue.h (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/ShapeValue.h 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/ShapeValue.h 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -76,7 +76,11 @@
</span><span class="cx"> m_image = image;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- bool operator==(const ShapeValue& other) const { return type() == other.type(); }
</del><ins>+ bool operator==(const ShapeValue&) const;
+ bool operator!=(const ShapeValue& other) const
+ {
+ return !(*this == other);
+ }
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> ShapeValue(PassRefPtr<BasicShape> shape, CSSBoxType cssBox)
</span><span class="lines">@@ -87,13 +91,11 @@
</span><span class="cx"> }
</span><span class="cx"> ShapeValue(Type type)
</span><span class="cx"> : m_type(type)
</span><del>- , m_cssBox(BoxMissing)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> ShapeValue(PassRefPtr<StyleImage> image)
</span><span class="cx"> : m_type(Type::Image)
</span><span class="cx"> , m_image(image)
</span><del>- , m_cssBox(BoxMissing)
</del><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -106,7 +108,7 @@
</span><span class="cx"> Type m_type;
</span><span class="cx"> RefPtr<BasicShape> m_shape;
</span><span class="cx"> RefPtr<StyleImage> m_image;
</span><del>- CSSBoxType m_cssBox;
</del><ins>+ CSSBoxType m_cssBox { BoxMissing };
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleStyleRareNonInheritedDatacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -243,11 +243,11 @@
</span><span class="cx"> && m_maskBoxImage == o.m_maskBoxImage
</span><span class="cx"> && m_pageSize == o.m_pageSize
</span><span class="cx"> #if ENABLE(CSS_SHAPES)
</span><del>- && m_shapeOutside == o.m_shapeOutside
</del><ins>+ && shapeOutsideDataEquivalent(o)
</ins><span class="cx"> && m_shapeMargin == o.m_shapeMargin
</span><span class="cx"> && m_shapeImageThreshold == o.m_shapeImageThreshold
</span><span class="cx"> #endif
</span><del>- && m_clipPath == o.m_clipPath // FIXME: This needs to compare values.
</del><ins>+ && clipPathOperationsEquivalent(o)
</ins><span class="cx"> && m_textDecorationColor == o.m_textDecorationColor
</span><span class="cx"> && m_visitedLinkTextDecorationColor == o.m_visitedLinkTextDecorationColor
</span><span class="cx"> && m_visitedLinkBackgroundColor == o.m_visitedLinkBackgroundColor
</span><span class="lines">@@ -363,6 +363,26 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool StyleRareNonInheritedData::clipPathOperationsEquivalent(const StyleRareNonInheritedData& o) const
+{
+ if ((!m_clipPath && o.m_clipPath) || (m_clipPath && !o.m_clipPath))
+ return false;
+ if (m_clipPath && o.m_clipPath && (*m_clipPath != *o.m_clipPath))
+ return false;
+ return true;
+}
+
+#if ENABLE(CSS_SHAPES)
+bool StyleRareNonInheritedData::shapeOutsideDataEquivalent(const StyleRareNonInheritedData& o) const
+{
+ if ((!m_shapeOutside && o.m_shapeOutside) || (m_shapeOutside && !o.m_shapeOutside))
+ return false;
+ if (m_shapeOutside && o.m_shapeOutside && (*m_shapeOutside != *o.m_shapeOutside))
+ return false;
+ return true;
+}
+#endif
+
</ins><span class="cx"> bool StyleRareNonInheritedData::hasFilters() const
</span><span class="cx"> {
</span><span class="cx"> return m_filter.get() && !m_filter->m_operations.isEmpty();
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingstyleStyleRareNonInheritedDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.h (190878 => 190879)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.h 2015-10-12 19:15:17 UTC (rev 190878)
+++ trunk/Source/WebCore/rendering/style/StyleRareNonInheritedData.h 2015-10-12 19:15:28 UTC (rev 190879)
</span><span class="lines">@@ -98,6 +98,10 @@
</span><span class="cx"> bool reflectionDataEquivalent(const StyleRareNonInheritedData&) const;
</span><span class="cx"> bool animationDataEquivalent(const StyleRareNonInheritedData&) const;
</span><span class="cx"> bool transitionDataEquivalent(const StyleRareNonInheritedData&) const;
</span><ins>+ bool clipPathOperationsEquivalent(const StyleRareNonInheritedData&) const;
+#if ENABLE(CSS_SHAPES)
+ bool shapeOutsideDataEquivalent(const StyleRareNonInheritedData&) const;
+#endif
</ins><span class="cx"> bool hasFilters() const;
</span><span class="cx"> #if ENABLE(FILTERS_LEVEL_2)
</span><span class="cx"> bool hasBackdropFilters() const;
</span></span></pre>
</div>
</div>
</body>
</html>