- Revision
- 183732
- Author
- [email protected]
- Date
- 2015-05-03 09:10:43 -0700 (Sun, 03 May 2015)
Log Message
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (183731 => 183732)
--- trunk/LayoutTests/ChangeLog 2015-05-03 15:20:34 UTC (rev 183731)
+++ trunk/LayoutTests/ChangeLog 2015-05-03 16:10:43 UTC (rev 183732)
@@ -1,3 +1,17 @@
+2015-05-02 Simon Fraser <[email protected]>
+
+ 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.
+
2015-05-03 Alexey Proskuryakov <[email protected]>
Skip fast/frames/flattening/crash-remove-iframe-during-object-beforeload.html
Added: trunk/LayoutTests/fast/images/resources/spinner.gif (0 => 183732)
--- 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)
@@ -0,0 +1,41 @@
+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;
\ No newline at end of file
Added: trunk/LayoutTests/fast/images/set-needs-layout-in-painting-expected.txt (0 => 183732)
--- 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)
@@ -0,0 +1,3 @@
+This test should not assert in debug builds.
+
+
Added: trunk/LayoutTests/fast/images/set-needs-layout-in-painting.html (0 => 183732)
--- 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)
@@ -0,0 +1,53 @@
+<!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="" width="100" height="100">
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (183731 => 183732)
--- 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)
@@ -1,3 +1,47 @@
+2015-05-02 Simon Fraser <[email protected]>
+
+ 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.
+
2015-05-03 Carlos Garcia Campos <[email protected]>
[GTK][EFL] Unify platform display handling
Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (183731 => 183732)
--- 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)
@@ -73,7 +73,7 @@
, m_sizeAvailable(false)
, m_hasUniformFrameSize(true)
, m_haveFrameCount(false)
- , m_cachedImage(0)
+ , m_animationFinishedWhenCatchingUp(false)
{
}
@@ -88,6 +88,13 @@
m_frameTimer = nullptr;
}
+void BitmapImage::startTimer(double delay)
+{
+ ASSERT(!m_frameTimer);
+ m_frameTimer = std::make_unique<Timer>(*this, &BitmapImage::advanceAnimation);
+ m_frameTimer->startOneShot(delay);
+}
+
#if !USE(CG)
bool BitmapImage::decodedDataIsPurgeable() const
{
@@ -473,7 +480,7 @@
int BitmapImage::repetitionCount(bool imageKnownToBeComplete)
{
if ((m_repetitionCountStatus == Unknown) || ((m_repetitionCountStatus == Uncertain) && imageKnownToBeComplete)) {
- // Snag the repetition count. If |imageKnownToBeComplete| is false, the
+ // Snag the repetition count. If |imageKnownToBeComplete| is false, the
// repetition count may not be accurate yet for GIFs; in this case the
// decoder will default to cAnimationLoopOnce, and we'll try and read
// the count again once the whole image is decoded.
@@ -505,13 +512,13 @@
return;
// Don't advance past the last frame if we haven't decoded the whole image
- // yet and our repetition count is potentially unset. The repetition count
+ // yet and our repetition count is potentially unset. The repetition count
// in a GIF can potentially come after all the rest of the image data, so
// wait on it.
if (!m_allDataReceived && repetitionCount(false) == cAnimationLoopOnce && m_currentFrame >= (frameCount() - 1))
return;
- // Determine time for next frame to start. By ignoring paint and timer lag
+ // Determine time for next frame to start. By ignoring paint and timer lag
// in this calculation, we make the animation appear to run at its desired
// rate regardless of how fast it's being repainted.
const double currentDuration = frameDurationAtIndex(m_currentFrame);
@@ -520,7 +527,7 @@
#if !PLATFORM(IOS)
// When an animated image is more than five minutes out of date, the
// user probably doesn't care about resyncing and we could burn a lot of
- // time looping through frames below. Just reset the timings.
+ // time looping through frames below. Just reset the timings.
const double cAnimationResyncCutoff = 5 * 60;
if ((time - m_desiredFrameStartTime) > cAnimationResyncCutoff)
m_desiredFrameStartTime = time + currentDuration;
@@ -533,7 +540,7 @@
// The image may load more slowly than it's supposed to animate, so that by
// the time we reach the end of the first repetition, we're well behind.
// Clamp the desired frame start time in this case, so that we don't skip
- // frames (or whole iterations) trying to "catch up". This is a tradeoff:
+ // frames (or whole iterations) trying to "catch up". This is a tradeoff:
// It guarantees users see the whole animation the second time through and
// don't miss any repetitions, and is closer to what other browsers do; on
// the other hand, it makes animations "less accurate" for pages that try to
@@ -545,55 +552,42 @@
if (catchUpIfNecessary == DoNotCatchUp || time < m_desiredFrameStartTime) {
// Haven't yet reached time for next frame to start; delay until then.
- 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;
+ startTimer(std::max<double>(m_desiredFrameStartTime - time, 0));
+ return;
+ }
- // Yes; skip over it without notifying our observers.
- if (!internalAdvanceAnimation(SkippingFramesToCatchUp))
- return;
- m_desiredFrameStartTime = frameAfterNextStartTime;
- nextFrame = frameAfterNext;
- }
+ ASSERT(!m_frameTimer);
- // 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);
+ // 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;
}
+ m_desiredFrameStartTime = frameAfterNextStartTime;
+ nextFrame = frameAfterNext;
}
+
+ // 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);
}
void BitmapImage::stopAnimation()
{
- // This timer is used to animate all occurrences of this image. Don't invalidate
+ // This timer is used to animate all occurrences of this image. Don't invalidate
// the timer unless all renderers have stopped drawing.
clearTimer();
}
@@ -657,6 +651,12 @@
bool BitmapImage::internalAdvanceAnimation(AnimationAdvancement advancement)
{
clearTimer();
+
+ if (m_animationFinishedWhenCatchingUp) {
+ imageObserver()->animationAdvanced(this);
+ m_animationFinishedWhenCatchingUp = false;
+ return false;
+ }
++m_currentFrame;
bool advancedAnimation = true;
@@ -664,7 +664,7 @@
if (m_currentFrame >= frameCount()) {
++m_repetitionsComplete;
- // Get the repetition count again. If we weren't able to get a
+ // Get the repetition count again. If we weren't able to get a
// repetition count before, we should have decoded the whole image by
// now, so it should now be available.
// Note that we don't need to special-case cAnimationLoopOnce here
@@ -683,7 +683,7 @@
// We need to draw this frame if we advanced to it while not skipping, or if
// while trying to skip frames we hit the last frame and thus had to stop.
- if ((advancement == Normal && advancedAnimation) || (advancement == SkippingFramesToCatchUp && !advancedAnimation))
+ if (advancement == Normal && advancedAnimation)
imageObserver()->animationAdvanced(this);
return advancedAnimation;
Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.h (183731 => 183732)
--- 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)
@@ -136,8 +136,8 @@
virtual bool dataChanged(bool allDataReceived) override;
virtual String filenameExtension() const override;
- // 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
+ // 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
// automatically pause once all observers no longer want to render the image anywhere.
virtual void stopAnimation() override;
virtual void resetAnimation() override;
@@ -228,10 +228,10 @@
// Called before accessing m_frames[index] for info without decoding. Returns false on index out of bounds.
bool ensureFrameIsCached(size_t index, ImageFrameCaching = CacheMetadataAndFrame);
- // Called to invalidate cached data. When |destroyAll| is true, we wipe out
+ // Called to invalidate cached data. When |destroyAll| is true, we wipe out
// the entire frame buffer cache and tell the image source to destroy
// everything; this is used when e.g. we want to free some room in the image
- // cache. If |destroyAll| is false, we only delete frames up to the current
+ // cache. If |destroyAll| is false, we only delete frames up to the current
// one; this is used while animating large images to keep memory footprint
// low without redecoding the whole image on every frame.
virtual void destroyDecodedData(bool destroyAll = true) override;
@@ -249,7 +249,7 @@
bool isSizeAvailable();
// Called after asking the source for any information that may require
- // decoding part of the image (e.g., the image size). We need to report
+ // decoding part of the image (e.g., the image size). We need to report
// the partially decoded data to our observer so it has an accurate
// account of the BitmapImage's memory usage.
void didDecodeProperties() const;
@@ -260,7 +260,7 @@
virtual void startAnimation(CatchUpAnimation = CatchUp) override;
void advanceAnimation();
- // Function that does the real work of advancing the animation. When
+ // Function that does the real work of advancing the animation. When
// skippingFrames is true, we're in the middle of a loop trying to skip over
// a bunch of animation frames, so we should not do things like decode each
// one or notify our observers.
@@ -271,7 +271,7 @@
// Handle platform-specific data
void invalidatePlatformData();
- // Checks to see if the image is a 1x1 solid color. We optimize these images and just do a fill rect instead.
+ // Checks to see if the image is a 1x1 solid color. We optimize these images and just do a fill rect instead.
// This check should happen regardless whether m_checkedForSolidColor is already set, as the frame may have
// changed.
void checkForSolidColor();
@@ -286,6 +286,7 @@
private:
virtual bool decodedDataIsPurgeable() const override;
void clearTimer();
+ void startTimer(double delay);
ImageSource m_source;
mutable IntSize m_size; // The size to use for the overall image (will just be the size of the first image).
@@ -300,7 +301,7 @@
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.
std::unique_ptr<Timer> m_frameTimer;
- int m_repetitionCount; // How many total animation loops we should do. This will be cAnimationNone if this image type is incapable of animation.
+ int m_repetitionCount; // How many total animation loops we should do. This will be cAnimationNone if this image type is incapable of animation.
RepetitionCountStatus m_repetitionCountStatus;
int m_repetitionsComplete; // How many repetitions we've finished.
double m_desiredFrameStartTime; // The system time at which we hope to see the next call to startAnimation().
@@ -309,7 +310,7 @@
mutable RetainPtr<NSImage> m_nsImage; // A cached NSImage of frame 0. Only built lazily if someone actually queries for one.
#endif
#if USE(CG)
- mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for frame 0. Only built lazily if someone queries for one.
+ mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for frame 0. Only built lazily if someone queries for one.
#endif
Color m_solidColor; // If we're a 1x1 solid color, this is the color to use to fill.
@@ -335,6 +336,7 @@
bool m_sizeAvailable : 1; // Whether or not we can obtain the size of the first image frame yet from ImageIO.
mutable bool m_hasUniformFrameSize : 1;
mutable bool m_haveFrameCount : 1;
+ bool m_animationFinishedWhenCatchingUp : 1;
RefPtr<Image> m_cachedImage;
};
Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (183731 => 183732)
--- 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)
@@ -139,7 +139,10 @@
{
if (!shouldPaint(paintInfo, paintOffset))
return;
-
+
+#ifndef NDEBUG
+ SetLayoutNeededForbiddenScope scope(this);
+#endif
LayoutPoint adjustedPaintOffset = paintOffset + location();
if (hasBoxDecorations() && paintInfo.phase == PaintPhaseForeground)
