<!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>[183732] 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/183732">183732</a></dd>
<dt>Author</dt> <dd>simon.fraser@apple.com</dd>
<dt>Date</dt> <dd>2015-05-03 09:10:43 -0700 (Sun, 03 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Assertion failure (!needsLayout()) loading inkedmag.com
https://bugs.webkit.org/show_bug.cgi?id=144528
rdar://problem/20788681
Reviewed by Darin Adler.
Source/WebCore:
When animated GIFs get into catch-up mode, which is common on inkedmag.com,
BitmapImage::advanceAnimation() can synchronously call it's observer's
animationAdvanced(). This could cause RenderImage::repaintOrMarkForLayout()
to repaint or mark itself as needing layout in the middle of painting.
If painting multiple tiles, this could occur when painting the first tile,
and then painting the second tile would assert in RenderView::paint().
It's always wrong to synchronously call the observer when advancing
the animation, since this happens when painting, and you can't repaint
when painting. The long comment and call to startAnimation(DoNotCatchUp)
was required to explain and work around this, but it's simpler to just
advance the animation on a zero-delay timer.
Special handling is required for the case where internalAdvanceAnimation()
is catching up, and reaches the end of a non-repeating image; there, we
have to set a flag and do the notify on a zero-delay timer.
Lots of comment cleanup.
Test: fast/images/set-needs-layout-in-painting.html
* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::BitmapImage):
(WebCore::BitmapImage::startTimer): Utility to create and start the timer.
(WebCore::BitmapImage::repetitionCount):
(WebCore::BitmapImage::startAnimation): Early return in the DoNotCatchUp clause.
If skipping, and internalAdvanceAnimation() returns false (meaning it must have
reached the end), then queue up a notify. Change the normal behavior to just
start the timer.
(WebCore::BitmapImage::stopAnimation):
(WebCore::BitmapImage::internalAdvanceAnimation): Notify if the flag is set.
* platform/graphics/BitmapImage.h:
* rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::paint): Add a SetLayoutNeededForbiddenScope to
detect setNeedsLayouts when painting replaced elements, including images.
LayoutTests:
Test that sleeps for a while to force an image into catchup mode.
* fast/images/resources/spinner.gif: Added.
* fast/images/set-needs-layout-in-painting-expected.txt: Added.
* fast/images/set-needs-layout-in-painting.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsBitmapImagecpp">trunk/Source/WebCore/platform/graphics/BitmapImage.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsBitmapImageh">trunk/Source/WebCore/platform/graphics/BitmapImage.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderReplacedcpp">trunk/Source/WebCore/rendering/RenderReplaced.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastimagesresourcesspinnergif">trunk/LayoutTests/fast/images/resources/spinner.gif</a></li>
<li><a href="#trunkLayoutTestsfastimagessetneedslayoutinpaintingexpectedtxt">trunk/LayoutTests/fast/images/set-needs-layout-in-painting-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastimagessetneedslayoutinpaintinghtml">trunk/LayoutTests/fast/images/set-needs-layout-in-painting.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (183731 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/LayoutTests/ChangeLog 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2015-05-02 Simon Fraser <simon.fraser@apple.com>
+
+ Assertion failure (!needsLayout()) loading inkedmag.com
+ https://bugs.webkit.org/show_bug.cgi?id=144528
+ rdar://problem/20788681
+
+ Reviewed by Darin Adler.
+
+ Test that sleeps for a while to force an image into catchup mode.
+
+ * fast/images/resources/spinner.gif: Added.
+ * fast/images/set-needs-layout-in-painting-expected.txt: Added.
+ * fast/images/set-needs-layout-in-painting.html: Added.
+
</ins><span class="cx"> 2015-05-03 Alexey Proskuryakov <ap@apple.com>
</span><span class="cx">
</span><span class="cx"> Skip fast/frames/flattening/crash-remove-iframe-during-object-beforeload.html
</span></span></pre></div>
<a id="trunkLayoutTestsfastimagesresourcesspinnergif"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/images/resources/spinner.gif (0 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/images/resources/spinner.gif (rev 0)
+++ trunk/LayoutTests/fast/images/resources/spinner.gif 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -0,0 +1,41 @@
</span><ins>+GIF89a \xF3\xFF\xFF\xFF\x99\x99\x99\xE7\xE7\xE7\xCD\xCD\xCD\xE1\xE1\xE1\xD6\xD6֮\xAE\xAE\xBB\xBB\xBB\xEE\xEE\xEE\xF3\xF3\xF3\xE3\xE3㤤\xA4\x9A\x9A\x9A!\xFF NETSCAPE2.0!\xFECreated with ajaxload.info!\xF9
+, \xE7\xC8Iia\xA5\xEA\xCD\xE7bK\x85$\x9DF \xA3RA\x94T\xB2,\xA52S\xE2*05//\xC9m\xA2p!z\x93\xC1\xCC0;$\xC50C\x9C.I*!\xFCHC(A@o\x83!39T5\xBA\\xD18)\xA8+\x87\xA0`\xC1dwxG=Y
+g\x83wHb\x86 vA=\x920 V\\x9C\\x88; \xA4\xA5\x9C\x9F;\xA5\xAA\x9BH\xA8\x8A\xA2\xAB\xAC\x9D\xB3 \x980\xB6\xB5t%\x91Hs\x89\x8BrY<H.\x81ʼn\xB7\x96 \xBE\x92b\xBFZb\xC7OEg:\x98GY].\xC0=\xDAA\xDFOQ\x9Cs\x86\xE6\xEA\b\xC3h.9\xEC=sg\xF5\x9Ec\x8C\xF3e\xE2 \xD8*\x8Fֆf7D!\xF9
+, \xEA\xC8IiY\xA4\xEAͧYF5\x9DF\x90ԢRÔTbG\xBAJ\x85\xBB\xC0\xEC\xACL\xAA\x9Dd\xE1\xF0&\x85Ymx莔\xC3 \ @\x98\x80\xEA\xC1\x9A \xB01\xFC&R\x83\xB1\x92H
+41Q\xB4\xDB|V%zv#j0\x87
+\x8El\x8CGg{0~\x89<\x81< \x84[\xA2[\x87h\x91x\xA5\xA1G\xA9
+y\xA9\xA2\xA9\xA7\xAF\x87\xA3\xB6\x96[\x9E0\x97\xBA\xBCG\xB4\x91\xA8\xA6P\x86z\x9C\xC7hɾ\xA1Ękz\xC2i\x97\xC9y\x8E\xA0\xD1\xCAh|z\xD5h\x92G݄\xE2VŢ\xAF\x81\xE9\xED\xB9\xEB\h\xE7[\xAF\x8E\xEFǤ\x88\x8A\xF5&\x95+\x91\xA0W\x9E7\xB78\xE0\xC8!!\xF9
+, \xEE\xC8I)1\xA4\xEA\xCD\xE71G5d]\x85(\x95\xA1RDz\x94T2\x8C\x94jL\x84{\xC3\xD3< [\xD05\xE0M\xBE\xE0
+0\xD0)\x85
+ L\xB8\xA4I\x86\xF0m\x85\xCDE\xA8\xC7`\xB4p\xA5U
+ \x81^f%\x82^\xB1\xC8\xE4 u ;zz}0\x84X
+\x89S0ew yk<\x9C% \x9FO\xA3\xA3\x89\x93 \x91\xA6z\xA4\xAA{\x92\xAD\xAC\xAA|\xA9\xAA\xA4\xB6\xA3\xA2%\xB9 \x9A\xBB\xBDF\xAFi\x8C10\x88\x89\x80\x87\xA6˼Y\xB4\x9B\x9A\xC38\xC4x\x8A\xBF\x92\x8C z\x9F\xC9@\x89\x89\xD7<ݫ\xE2\x9A\xC7\xC1\xDE\xE3\xE8\xAF\xBE\xEC8\xF1\xE7Y<\xAF\x8C\xEAɥ8\xF3\x87\xA7\\x87P$\xB5\xBB\xA5!\x92\xC1+!\xF9
+, \xE7\xC8I\xA9\xA2\xEA\xCDgEU\x9D\x96 ՠR\x87a\x94TB٤\xE1\xBEp>'\xB6\x95\xA4e\xF5$\x88\x99"\x88\\x87#E1Cn\x80Ď\x83\xC9~\xD7\xD5J,\xDC,Aa\x95\xAA\x9DUw^4I%P\xDD\xDEu Q33{0\x81i1T\x85Ggw y}%\x88%'R\x9C\x9D \xA0\xA1 \x8E\x85\x8C=\xA1\xAA \xA6\xA7\x9D\xAB \xA3\xA5\xA73\x9E\xB6G\x96%\xB9\x94p\xBA\xBD0\xA6
+\x99\xB3JRo\x855Ȇ0IĦmyk\x88\xC3x\xCDT\x88_}\xC8(\x8F\x85\xD7^\xE2\xE2yK\x9D\x8Es\xB5\xEC\x9C\xE9>i_\xA8%\x8E\xD5\xEEn\xFA=\xD9\xE2\xDA\xCAq\xD84e\xCD-M¤D!\xF9
+, \xEE\xC8I)*\xA8\xEA\xCD')E\xA5d]\x95\x90\xA6\xC3PR A\x94:!\xAD\xFBzr\x92\x82\x93\x9Cbw\x93+%6\x80"G\xA4(d$["\x87\x92\xF8J\xB1 \xC0Fh\xAD\x90aQP\x8D`p% /BFP\cU+\xE2+?T\xD0tW/pG&OtDa_ sy lD'M\x98\x99 \x9C\x9Dq \x8Atc\x98\x9D\xA5 \xA1\xA2\x99\xA6 b\xA0\xA22\x9A\xB1D\x93\x90M : \x91\xB5\xB9\xB9 \x8Fd\xA1\x88%\xC0 \xA24%s) \xC0\xBB\x91u\x83\x83E3\xB8 \xA3YU\x80\x8Btږ\xE6\xE6\x91\xC6D\x8A$\xE6JiM\xED<\xE0Y\xE0;\x8Aذ\x80\x98d<\x93 O\x82tX\xF2<q'+B!\xF9
+, \xF3\xC8IiR\xA9\xEAͧ"J% \x9D\x96\x90\xA1\xA6EQZ\xAA\x90\xAE\xD2\xEFLd\x92\x8A\xF7-Y\xAE\xA6
+\xF5h\x82\x96k\xE8Q\xA1|\x80\x845\xE1u\xBE 4Y\xF8I\x83\xAB\xA0N+bW \x87\x8D\x80u\x91\x875\x9B
+\xEE\xC1r\x82\xF6 \xAC%yb>^%o/rv l9'L\x92\x93\x96\x97; \x86\x879\x97\x9F\x9B\x9C\x93\xA0\x99\x85\xA3\x94\xAA9\x80%\x8D \x8Bi9\xB3\xB3\x9D\xA8 C\xB9 "\x86B B\xB9\xB5Ds\x8F \xCE^Xf}$P \xD7 {L\xDE?P\xCA \x94\x9BO4 \xD7\xD0\xC0E\xD3\xF3咛V\xEB$\xB8\xE6\xCAdJ\xD0#)\x85pV\xC2\xC0$!\xF9
+, \xEB\xC8IiR\xA9\xEAͧ"J\x85d]\x95 \xA1R\xC2ZN\x89*P*\xAB\xE1;\xD5$P{*\x94N\x82\xC0\xED\EА\xF2!1UO2\xDDD \x99_r6I\xF6b
+\xA1\xA4\xE5\xD4\xC4H\x97\x9A8 B\x97; \xB2\xAC"'\xAA\x9CZ\xDB\xDAt\xBD\x92b \x80K#C'K\x88\x89\x8C\x8Dw}?\x88\x8D\x94\x91\x92K\x95\x8Fiz6\x8A\xA0:x\x82KAC\xA8\xA8\x9F&}9\xAEtz\ \\xB6\xA8\xAAD5;x \xB9\xA8\xB1Q\x81d( \xD6 \xCB \xB1KW\xD6 \xCA \x8AMB\xE0\xCB\x88I\xB4\xE9ڈM=\xF1ˤs\xF8⸽8Da\x83\xA1J`@LG!\xF9
+, \xEF\xC8IiR\xA9\xEAͧ"J\x85d]\x95 \xA1R\xC2ZN\x89*P*\xAB\xE1;\xD5$P{*\x94N\x82\xC0\xED\EА\xF2!1UO2\xDDD \x99_r6I\xF6b
+\xA1\xA4\xE5\xD4\xC4H\x97\x9A8 B\x97; \xB2\xAC"'\xAA\x9CZ\xDB\xDAt\xBD\x92b \x80K#C'K\x88\x89Gziz6\x88\x8F8}z\x89\x92\x8D\x94~\x8A\x9B%X\x84K9 \x83:\xA2\xA8\x810}\xA4% \xA8tz\ B\xB1\xA4lc L\xA2bQ\x81 \xC7 \xD1 \xC5\x90\x88\xCE \xD1 \xC5lj \xCE\xC2\xDDųK\xCE\xDE\xE8\xB8ň\xE4 \xEC\xE7\xD2\xE9\xC5x \xCE(țP\xE0\x9AX ,\xC8\xE9ւ|/"!\xF9
+, \xF0\xC8IiR\xA9\xEAͧ"J\x85d]\x95 \xA1R\xC2ZN\x89*P*\xAB\xE1;\xD5$P{*\x94N\x82\xC0\xED\EА\xF2!1UO2\xDDD \x99_r6I\xF6b
+\xA1\xA4\xE5\xD4\xC4H\x97\x9A8 B\x97; \xB2\xAC"'\xAA\x9CZ\xDB\xDAt\xBD\x92b \x80K#C'K\x88\x89Gziz6\x88\x8F8}z\x89\x92\x8D\x94~\x8A\x9B%\x85:\x84A/ C} \xA6\xA6\x86u\\xAF \xB3h}b\xA5\xA6 D\xC2\xC3] =\xA6\xA8 \xA8\xD6 \xC8\x81V)\xD3 \xD6
+ڊ\xD3\xD0\xE2\xC89C\xD3\xE3\xEBD\xE6K\xE8 \x90\xF5\xED\xBCK\xA6\x85\xA2u\x8D \xB7\xC7* 00\x90S \xCAtD!\xF9
+, \xEB\xC8IiR\xA9\xEAͧ"J\x85d]\x95 \xA1R\xC2ZN\x89*P*\xAB\xE1;\xD5$P{*\x94N\x82\xC0\xED\EА\xF2!1UO2\xDDD \x99_r6I\xF6b
+\xA1\xA4\xE5\xD4\xC4H\x97\x9A8 B\x97; \xB2\xAC"'\xAA\x9CZ\xDB\xDAt\xBD\x92b \x80K#C'K\x88\x89Gz \x90\x91\x89z5
+\x91\x97 \x93\x94\x8D\x8F\x98\x8A\x9FC\x85: \x84A/ C}\xAA\xAA\x86u\\xB3 \xB7Eh}b\xA9\xAA6\xC5[ =\xAA\xA5\xB8\xA5\xD7Wx&)\xD4\xD7\xD2I9\x88Ԭ\xE1@oC\xD4\xEAT?K\xDE\xC6\xE9\xD8d\xDB\xEF\xF2]\xF8\xC1B7\xA1\xC0\x82\xA06ЫD!\xF9
+, \xE8\xC8IiR\xA9\xEAͧ"J\x85d]\x95 \xA1R\xC2ZN\x89*P*\xAB\xE1;\xD5$P{*\x94N\x82\xC0\xED\EА\xF2!1UO2\xDDD \x99_r6I\xF6 ƀ\xD4\xC4H\x97\x9A03\xAA\xB3\x94hո\x83\x9Ba\xC0\x97j U {CIk mbK#\x87cK\x91\x92\x808 \x84{a\x92\x958\x99n\x9B\x95\x98\x99\x93\xA5\x87\x82V\x90:\x88/q:M\x81
+\xAF\xAFCu\x80~\xB7\xB8\x89Eh\xB3k\xAE\xAF6 \xBD[ _\xAF\xB1\x836P</U\xD9YHF\x92\xE19?M\xC2%
+\xE1G\xCB\xCC\xE9C\xE1k\xF3v\xA8\xD9\xF1>.]\xFA6\xA9\xF0!\x87)V\x88!\xF9
+, \xF0\xC8IiR\xA9\xEAͧ"J\x85d]U \xA1R\xC2ZN \xC3\x94J\xC0j\xF8N2sK6\x8F
+\xB1\x9B d\x8BI\x80\xC8)
+L\xD8H\xB0W\xA1G 6 \xCAKX\xA6\x83젱\x92.6\xA2d\xB0\xA8~z\x93h\xD9\xC2uur/6 X5\x83I;_\x86 t O#E {O\x9B\x9B\x889V\xA2\xA3\x9C\x9E9\xA3\xA84\x9D\x9E\xA1\xA9\x9C\xB1\x9B\x97;V\x96C/
+\x80\xB96\xBBØ~*\xBD'\xC3\x8AMo \xB8\xBA\xBB\x80n\xCE\xC7b X\xC2:~]+V*\xCDm\xE5K_\xE0O\xD1rK\xF1\xB3N@.\xEA\x9B\xD1d\xF9~\xCEqЦ\xE4 \x81 D\xA2B 5D;
</ins><span class="cx">\ No newline at end of file
</span></span></pre></div>
<a id="trunkLayoutTestsfastimagessetneedslayoutinpaintingexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/images/set-needs-layout-in-painting-expected.txt (0 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/images/set-needs-layout-in-painting-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/set-needs-layout-in-painting-expected.txt 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+This test should not assert in debug builds.
+
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastimagessetneedslayoutinpaintinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/images/set-needs-layout-in-painting.html (0 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/images/set-needs-layout-in-painting.html (rev 0)
+++ trunk/LayoutTests/fast/images/set-needs-layout-in-painting.html 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -0,0 +1,53 @@
</span><ins>+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+ .box {
+ height: 100px;
+ width: 100px;
+ background-color: blue;
+ }
+
+ img {
+ height: 90%;
+ width: 90%;
+ }
+ </style>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function sleep(ms) {
+ var endTime = (new Date().getTime()) + ms;
+ while ((new Date().getTime()) < endTime) {
+ }
+ }
+
+ const maxIterations = 20;
+ var iteration = 0;
+ function startTest()
+ {
+ var interval = window.setInterval(function() {
+ // Sleep to get the GIF into catchup mode.
+ sleep(50);
+ if (++iteration == maxIterations) {
+ window.clearInterval(interval);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ }, 20);
+ }
+
+ window.addEventListener('load', startTest);
+ </script>
+</head>
+<body>
+ <p>This test should not assert in debug builds.</p>
+ <div class="box">
+ <img src="resources/spinner.gif" width="100" height="100">
+ </div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (183731 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/Source/WebCore/ChangeLog 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -1,3 +1,47 @@
</span><ins>+2015-05-02 Simon Fraser <simon.fraser@apple.com>
+
+ Assertion failure (!needsLayout()) loading inkedmag.com
+ https://bugs.webkit.org/show_bug.cgi?id=144528
+ rdar://problem/20788681
+
+ Reviewed by Darin Adler.
+
+ When animated GIFs get into catch-up mode, which is common on inkedmag.com,
+ BitmapImage::advanceAnimation() can synchronously call it's observer's
+ animationAdvanced(). This could cause RenderImage::repaintOrMarkForLayout()
+ to repaint or mark itself as needing layout in the middle of painting.
+ If painting multiple tiles, this could occur when painting the first tile,
+ and then painting the second tile would assert in RenderView::paint().
+
+ It's always wrong to synchronously call the observer when advancing
+ the animation, since this happens when painting, and you can't repaint
+ when painting. The long comment and call to startAnimation(DoNotCatchUp)
+ was required to explain and work around this, but it's simpler to just
+ advance the animation on a zero-delay timer.
+
+ Special handling is required for the case where internalAdvanceAnimation()
+ is catching up, and reaches the end of a non-repeating image; there, we
+ have to set a flag and do the notify on a zero-delay timer.
+
+ Lots of comment cleanup.
+
+ Test: fast/images/set-needs-layout-in-painting.html
+
+ * platform/graphics/BitmapImage.cpp:
+ (WebCore::BitmapImage::BitmapImage):
+ (WebCore::BitmapImage::startTimer): Utility to create and start the timer.
+ (WebCore::BitmapImage::repetitionCount):
+ (WebCore::BitmapImage::startAnimation): Early return in the DoNotCatchUp clause.
+ If skipping, and internalAdvanceAnimation() returns false (meaning it must have
+ reached the end), then queue up a notify. Change the normal behavior to just
+ start the timer.
+ (WebCore::BitmapImage::stopAnimation):
+ (WebCore::BitmapImage::internalAdvanceAnimation): Notify if the flag is set.
+ * platform/graphics/BitmapImage.h:
+ * rendering/RenderReplaced.cpp:
+ (WebCore::RenderReplaced::paint): Add a SetLayoutNeededForbiddenScope to
+ detect setNeedsLayouts when painting replaced elements, including images.
+
</ins><span class="cx"> 2015-05-03 Carlos Garcia Campos <cgarcia@igalia.com>
</span><span class="cx">
</span><span class="cx"> [GTK][EFL] Unify platform display handling
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsBitmapImagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (183731 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -73,7 +73,7 @@
</span><span class="cx"> , m_sizeAvailable(false)
</span><span class="cx"> , m_hasUniformFrameSize(true)
</span><span class="cx"> , m_haveFrameCount(false)
</span><del>- , m_cachedImage(0)
</del><ins>+ , m_animationFinishedWhenCatchingUp(false)
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -88,6 +88,13 @@
</span><span class="cx"> m_frameTimer = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void BitmapImage::startTimer(double delay)
+{
+ ASSERT(!m_frameTimer);
+ m_frameTimer = std::make_unique<Timer>(*this, &BitmapImage::advanceAnimation);
+ m_frameTimer->startOneShot(delay);
+}
+
</ins><span class="cx"> #if !USE(CG)
</span><span class="cx"> bool BitmapImage::decodedDataIsPurgeable() const
</span><span class="cx"> {
</span><span class="lines">@@ -473,7 +480,7 @@
</span><span class="cx"> int BitmapImage::repetitionCount(bool imageKnownToBeComplete)
</span><span class="cx"> {
</span><span class="cx"> if ((m_repetitionCountStatus == Unknown) || ((m_repetitionCountStatus == Uncertain) && imageKnownToBeComplete)) {
</span><del>- // Snag the repetition count. If |imageKnownToBeComplete| is false, the
</del><ins>+ // Snag the repetition count. If |imageKnownToBeComplete| is false, the
</ins><span class="cx"> // repetition count may not be accurate yet for GIFs; in this case the
</span><span class="cx"> // decoder will default to cAnimationLoopOnce, and we'll try and read
</span><span class="cx"> // the count again once the whole image is decoded.
</span><span class="lines">@@ -505,13 +512,13 @@
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> // Don't advance past the last frame if we haven't decoded the whole image
</span><del>- // yet and our repetition count is potentially unset. The repetition count
</del><ins>+ // yet and our repetition count is potentially unset. The repetition count
</ins><span class="cx"> // in a GIF can potentially come after all the rest of the image data, so
</span><span class="cx"> // wait on it.
</span><span class="cx"> if (!m_allDataReceived && repetitionCount(false) == cAnimationLoopOnce && m_currentFrame >= (frameCount() - 1))
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- // Determine time for next frame to start. By ignoring paint and timer lag
</del><ins>+ // Determine time for next frame to start. By ignoring paint and timer lag
</ins><span class="cx"> // in this calculation, we make the animation appear to run at its desired
</span><span class="cx"> // rate regardless of how fast it's being repainted.
</span><span class="cx"> const double currentDuration = frameDurationAtIndex(m_currentFrame);
</span><span class="lines">@@ -520,7 +527,7 @@
</span><span class="cx"> #if !PLATFORM(IOS)
</span><span class="cx"> // When an animated image is more than five minutes out of date, the
</span><span class="cx"> // user probably doesn't care about resyncing and we could burn a lot of
</span><del>- // time looping through frames below. Just reset the timings.
</del><ins>+ // time looping through frames below. Just reset the timings.
</ins><span class="cx"> const double cAnimationResyncCutoff = 5 * 60;
</span><span class="cx"> if ((time - m_desiredFrameStartTime) > cAnimationResyncCutoff)
</span><span class="cx"> m_desiredFrameStartTime = time + currentDuration;
</span><span class="lines">@@ -533,7 +540,7 @@
</span><span class="cx"> // The image may load more slowly than it's supposed to animate, so that by
</span><span class="cx"> // the time we reach the end of the first repetition, we're well behind.
</span><span class="cx"> // Clamp the desired frame start time in this case, so that we don't skip
</span><del>- // frames (or whole iterations) trying to "catch up". This is a tradeoff:
</del><ins>+ // frames (or whole iterations) trying to "catch up". This is a tradeoff:
</ins><span class="cx"> // It guarantees users see the whole animation the second time through and
</span><span class="cx"> // don't miss any repetitions, and is closer to what other browsers do; on
</span><span class="cx"> // the other hand, it makes animations "less accurate" for pages that try to
</span><span class="lines">@@ -545,55 +552,42 @@
</span><span class="cx">
</span><span class="cx"> if (catchUpIfNecessary == DoNotCatchUp || time < m_desiredFrameStartTime) {
</span><span class="cx"> // Haven't yet reached time for next frame to start; delay until then.
</span><del>- m_frameTimer = std::make_unique<Timer>(*this, &BitmapImage::advanceAnimation);
- m_frameTimer->startOneShot(std::max(m_desiredFrameStartTime - time, 0.));
- } else {
- // We've already reached or passed the time for the next frame to start.
- // See if we've also passed the time for frames after that to start, in
- // case we need to skip some frames entirely. Remember not to advance
- // to an incomplete frame.
- for (size_t frameAfterNext = (nextFrame + 1) % frameCount(); frameIsCompleteAtIndex(frameAfterNext); frameAfterNext = (nextFrame + 1) % frameCount()) {
- // Should we skip the next frame?
- double frameAfterNextStartTime = m_desiredFrameStartTime + frameDurationAtIndex(nextFrame);
- if (time < frameAfterNextStartTime)
- break;
</del><ins>+ startTimer(std::max<double>(m_desiredFrameStartTime - time, 0));
+ return;
+ }
</ins><span class="cx">
</span><del>- // Yes; skip over it without notifying our observers.
- if (!internalAdvanceAnimation(SkippingFramesToCatchUp))
- return;
- m_desiredFrameStartTime = frameAfterNextStartTime;
- nextFrame = frameAfterNext;
- }
</del><ins>+ ASSERT(!m_frameTimer);
</ins><span class="cx">
</span><del>- // Draw the next frame immediately. Note that m_desiredFrameStartTime
- // may be in the past, meaning the next time through this function we'll
- // kick off the next advancement sooner than this frame's duration would
- // suggest.
- if (internalAdvanceAnimation()) {
- // The image region has been marked dirty, but once we return to our
- // caller, draw() will clear it, and nothing will cause the
- // animation to advance again. We need to start the timer for the
- // next frame running, or the animation can hang. (Compare this
- // with when advanceAnimation() is called, and the region is dirtied
- // while draw() is not in the callstack, meaning draw() gets called
- // to update the region and thus startAnimation() is reached again.)
- // NOTE: For large images with slow or heavily-loaded systems,
- // throwing away data as we go (see destroyDecodedData()) means we
- // can spend so much time re-decoding data above that by the time we
- // reach here we're behind again. If we let startAnimation() run
- // the catch-up code again, we can get long delays without painting
- // as we race the timer, or even infinite recursion. In this
- // situation the best we can do is to simply change frames as fast
- // as possible, so force startAnimation() to set a zero-delay timer
- // and bail out if we're not caught up.
- startAnimation(DoNotCatchUp);
</del><ins>+ // We've already reached or passed the time for the next frame to start.
+ // See if we've also passed the time for frames after that to start, in
+ // case we need to skip some frames entirely. Remember not to advance
+ // to an incomplete frame.
+ for (size_t frameAfterNext = (nextFrame + 1) % frameCount(); frameIsCompleteAtIndex(frameAfterNext); frameAfterNext = (nextFrame + 1) % frameCount()) {
+ // Should we skip the next frame?
+ double frameAfterNextStartTime = m_desiredFrameStartTime + frameDurationAtIndex(nextFrame);
+ if (time < frameAfterNextStartTime)
+ break;
+
+ // Yes; skip over it without notifying our observers. If we hit the end while catching up,
+ // tell the observer asynchronously.
+ if (!internalAdvanceAnimation(SkippingFramesToCatchUp)) {
+ m_animationFinishedWhenCatchingUp = true;
+ startTimer(0);
+ return;
</ins><span class="cx"> }
</span><ins>+ m_desiredFrameStartTime = frameAfterNextStartTime;
+ nextFrame = frameAfterNext;
</ins><span class="cx"> }
</span><ins>+
+ // Draw the next frame as soon as possible. Note that m_desiredFrameStartTime
+ // may be in the past, meaning the next time through this function we'll
+ // kick off the next advancement sooner than this frame's duration would suggest.
+ startTimer(0);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void BitmapImage::stopAnimation()
</span><span class="cx"> {
</span><del>- // This timer is used to animate all occurrences of this image. Don't invalidate
</del><ins>+ // This timer is used to animate all occurrences of this image. Don't invalidate
</ins><span class="cx"> // the timer unless all renderers have stopped drawing.
</span><span class="cx"> clearTimer();
</span><span class="cx"> }
</span><span class="lines">@@ -657,6 +651,12 @@
</span><span class="cx"> bool BitmapImage::internalAdvanceAnimation(AnimationAdvancement advancement)
</span><span class="cx"> {
</span><span class="cx"> clearTimer();
</span><ins>+
+ if (m_animationFinishedWhenCatchingUp) {
+ imageObserver()->animationAdvanced(this);
+ m_animationFinishedWhenCatchingUp = false;
+ return false;
+ }
</ins><span class="cx">
</span><span class="cx"> ++m_currentFrame;
</span><span class="cx"> bool advancedAnimation = true;
</span><span class="lines">@@ -664,7 +664,7 @@
</span><span class="cx"> if (m_currentFrame >= frameCount()) {
</span><span class="cx"> ++m_repetitionsComplete;
</span><span class="cx">
</span><del>- // Get the repetition count again. If we weren't able to get a
</del><ins>+ // Get the repetition count again. If we weren't able to get a
</ins><span class="cx"> // repetition count before, we should have decoded the whole image by
</span><span class="cx"> // now, so it should now be available.
</span><span class="cx"> // Note that we don't need to special-case cAnimationLoopOnce here
</span><span class="lines">@@ -683,7 +683,7 @@
</span><span class="cx">
</span><span class="cx"> // We need to draw this frame if we advanced to it while not skipping, or if
</span><span class="cx"> // while trying to skip frames we hit the last frame and thus had to stop.
</span><del>- if ((advancement == Normal && advancedAnimation) || (advancement == SkippingFramesToCatchUp && !advancedAnimation))
</del><ins>+ if (advancement == Normal && advancedAnimation)
</ins><span class="cx"> imageObserver()->animationAdvanced(this);
</span><span class="cx">
</span><span class="cx"> return advancedAnimation;
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsBitmapImageh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.h (183731 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/BitmapImage.h 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.h 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -136,8 +136,8 @@
</span><span class="cx"> virtual bool dataChanged(bool allDataReceived) override;
</span><span class="cx"> virtual String filenameExtension() const override;
</span><span class="cx">
</span><del>- // It may look unusual that there is no start animation call as public API. This is because
- // we start and stop animating lazily. Animation begins whenever someone draws the image. It will
</del><ins>+ // It may look unusual that there is no start animation call as public API. This is because
+ // we start and stop animating lazily. Animation begins whenever someone draws the image. It will
</ins><span class="cx"> // automatically pause once all observers no longer want to render the image anywhere.
</span><span class="cx"> virtual void stopAnimation() override;
</span><span class="cx"> virtual void resetAnimation() override;
</span><span class="lines">@@ -228,10 +228,10 @@
</span><span class="cx"> // Called before accessing m_frames[index] for info without decoding. Returns false on index out of bounds.
</span><span class="cx"> bool ensureFrameIsCached(size_t index, ImageFrameCaching = CacheMetadataAndFrame);
</span><span class="cx">
</span><del>- // Called to invalidate cached data. When |destroyAll| is true, we wipe out
</del><ins>+ // Called to invalidate cached data. When |destroyAll| is true, we wipe out
</ins><span class="cx"> // the entire frame buffer cache and tell the image source to destroy
</span><span class="cx"> // everything; this is used when e.g. we want to free some room in the image
</span><del>- // cache. If |destroyAll| is false, we only delete frames up to the current
</del><ins>+ // cache. If |destroyAll| is false, we only delete frames up to the current
</ins><span class="cx"> // one; this is used while animating large images to keep memory footprint
</span><span class="cx"> // low without redecoding the whole image on every frame.
</span><span class="cx"> virtual void destroyDecodedData(bool destroyAll = true) override;
</span><span class="lines">@@ -249,7 +249,7 @@
</span><span class="cx"> bool isSizeAvailable();
</span><span class="cx">
</span><span class="cx"> // Called after asking the source for any information that may require
</span><del>- // decoding part of the image (e.g., the image size). We need to report
</del><ins>+ // decoding part of the image (e.g., the image size). We need to report
</ins><span class="cx"> // the partially decoded data to our observer so it has an accurate
</span><span class="cx"> // account of the BitmapImage's memory usage.
</span><span class="cx"> void didDecodeProperties() const;
</span><span class="lines">@@ -260,7 +260,7 @@
</span><span class="cx"> virtual void startAnimation(CatchUpAnimation = CatchUp) override;
</span><span class="cx"> void advanceAnimation();
</span><span class="cx">
</span><del>- // Function that does the real work of advancing the animation. When
</del><ins>+ // Function that does the real work of advancing the animation. When
</ins><span class="cx"> // skippingFrames is true, we're in the middle of a loop trying to skip over
</span><span class="cx"> // a bunch of animation frames, so we should not do things like decode each
</span><span class="cx"> // one or notify our observers.
</span><span class="lines">@@ -271,7 +271,7 @@
</span><span class="cx"> // Handle platform-specific data
</span><span class="cx"> void invalidatePlatformData();
</span><span class="cx">
</span><del>- // Checks to see if the image is a 1x1 solid color. We optimize these images and just do a fill rect instead.
</del><ins>+ // Checks to see if the image is a 1x1 solid color. We optimize these images and just do a fill rect instead.
</ins><span class="cx"> // This check should happen regardless whether m_checkedForSolidColor is already set, as the frame may have
</span><span class="cx"> // changed.
</span><span class="cx"> void checkForSolidColor();
</span><span class="lines">@@ -286,6 +286,7 @@
</span><span class="cx"> private:
</span><span class="cx"> virtual bool decodedDataIsPurgeable() const override;
</span><span class="cx"> void clearTimer();
</span><ins>+ void startTimer(double delay);
</ins><span class="cx">
</span><span class="cx"> ImageSource m_source;
</span><span class="cx"> mutable IntSize m_size; // The size to use for the overall image (will just be the size of the first image).
</span><span class="lines">@@ -300,7 +301,7 @@
</span><span class="cx"> Vector<FrameData, 1> m_frames; // An array of the cached frames of the animation. We have to ref frames to pin them in the cache.
</span><span class="cx">
</span><span class="cx"> std::unique_ptr<Timer> m_frameTimer;
</span><del>- int m_repetitionCount; // How many total animation loops we should do. This will be cAnimationNone if this image type is incapable of animation.
</del><ins>+ int m_repetitionCount; // How many total animation loops we should do. This will be cAnimationNone if this image type is incapable of animation.
</ins><span class="cx"> RepetitionCountStatus m_repetitionCountStatus;
</span><span class="cx"> int m_repetitionsComplete; // How many repetitions we've finished.
</span><span class="cx"> double m_desiredFrameStartTime; // The system time at which we hope to see the next call to startAnimation().
</span><span class="lines">@@ -309,7 +310,7 @@
</span><span class="cx"> mutable RetainPtr<NSImage> m_nsImage; // A cached NSImage of frame 0. Only built lazily if someone actually queries for one.
</span><span class="cx"> #endif
</span><span class="cx"> #if USE(CG)
</span><del>- mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for frame 0. Only built lazily if someone queries for one.
</del><ins>+ mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for frame 0. Only built lazily if someone queries for one.
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> Color m_solidColor; // If we're a 1x1 solid color, this is the color to use to fill.
</span><span class="lines">@@ -335,6 +336,7 @@
</span><span class="cx"> bool m_sizeAvailable : 1; // Whether or not we can obtain the size of the first image frame yet from ImageIO.
</span><span class="cx"> mutable bool m_hasUniformFrameSize : 1;
</span><span class="cx"> mutable bool m_haveFrameCount : 1;
</span><ins>+ bool m_animationFinishedWhenCatchingUp : 1;
</ins><span class="cx">
</span><span class="cx"> RefPtr<Image> m_cachedImage;
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderReplacedcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (183731 => 183732)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderReplaced.cpp 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp 2015-05-03 16:10:43 UTC (rev 183732)
</span><span class="lines">@@ -139,7 +139,10 @@
</span><span class="cx"> {
</span><span class="cx"> if (!shouldPaint(paintInfo, paintOffset))
</span><span class="cx"> return;
</span><del>-
</del><ins>+
+#ifndef NDEBUG
+ SetLayoutNeededForbiddenScope scope(this);
+#endif
</ins><span class="cx"> LayoutPoint adjustedPaintOffset = paintOffset + location();
</span><span class="cx">
</span><span class="cx"> if (hasBoxDecorations() && paintInfo.phase == PaintPhaseForeground)
</span></span></pre>
</div>
</div>
</body>
</html>