- Revision
- 120326
- Author
- commit-qu...@webkit.org
- Date
- 2012-06-14 07:57:10 -0700 (Thu, 14 Jun 2012)
Log Message
[chromium] Fix race condition where animations start, finish and are deleted on the composite thread, all before the start even arrives on the main thread.
https://bugs.webkit.org/show_bug.cgi?id=88439
Patch by Eric Penner <epen...@google.com> on 2012-06-14
Reviewed by James Robinson.
Scheduling behavior covered by existing tests.
* platform/graphics/chromium/cc/CCActiveAnimation.cpp:
(WebCore::CCActiveAnimation::isFinishedAt):
* platform/graphics/chromium/cc/CCActiveAnimation.h:
(WebCore::CCActiveAnimation::isFinished):
* platform/graphics/chromium/cc/CCLayerAnimationController.cpp:
(WebCore::CCLayerAnimationController::suspendAnimations):
(WebCore::CCLayerAnimationController::pushAnimationUpdatesTo):
(WebCore::CCLayerAnimationController::animate):
(WebCore::CCLayerAnimationController::hasActiveAnimation):
(WebCore):
(WebCore::CCLayerAnimationController::markAnimationsForDeletion):
(WebCore::CCLayerAnimationController::purgeAnimationsMarkedForDeletion):
* platform/graphics/chromium/cc/CCLayerAnimationController.h:
(CCLayerAnimationController):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (120325 => 120326)
--- trunk/Source/WebCore/ChangeLog 2012-06-14 14:44:37 UTC (rev 120325)
+++ trunk/Source/WebCore/ChangeLog 2012-06-14 14:57:10 UTC (rev 120326)
@@ -1,3 +1,27 @@
+2012-06-14 Eric Penner <epen...@google.com>
+
+ [chromium] Fix race condition where animations start, finish and are deleted on the composite thread, all before the start even arrives on the main thread.
+ https://bugs.webkit.org/show_bug.cgi?id=88439
+
+ Reviewed by James Robinson.
+
+ Scheduling behavior covered by existing tests.
+
+ * platform/graphics/chromium/cc/CCActiveAnimation.cpp:
+ (WebCore::CCActiveAnimation::isFinishedAt):
+ * platform/graphics/chromium/cc/CCActiveAnimation.h:
+ (WebCore::CCActiveAnimation::isFinished):
+ * platform/graphics/chromium/cc/CCLayerAnimationController.cpp:
+ (WebCore::CCLayerAnimationController::suspendAnimations):
+ (WebCore::CCLayerAnimationController::pushAnimationUpdatesTo):
+ (WebCore::CCLayerAnimationController::animate):
+ (WebCore::CCLayerAnimationController::hasActiveAnimation):
+ (WebCore):
+ (WebCore::CCLayerAnimationController::markAnimationsForDeletion):
+ (WebCore::CCLayerAnimationController::purgeAnimationsMarkedForDeletion):
+ * platform/graphics/chromium/cc/CCLayerAnimationController.h:
+ (CCLayerAnimationController):
+
2012-06-14 Vsevolod Vlasov <vse...@chromium.org>
Web Inspector: ConsoleView.evaluateUsingTextPrompt should evaluate without command line API.
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.cpp (120325 => 120326)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.cpp 2012-06-14 14:44:37 UTC (rev 120325)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.cpp 2012-06-14 14:57:10 UTC (rev 120326)
@@ -84,7 +84,7 @@
bool CCActiveAnimation::isFinishedAt(double monotonicTime) const
{
- if (m_runState == Finished || m_runState == Aborted)
+ if (isFinished())
return true;
if (m_needsSynchronizedStartTime)
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h (120325 => 120326)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h 2012-06-14 14:44:37 UTC (rev 120325)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCActiveAnimation.h 2012-06-14 14:57:10 UTC (rev 120326)
@@ -52,6 +52,7 @@
WaitingForNextTick = 1,
WaitingForTargetAvailability,
WaitingForStartTime,
+ WaitingForDeletion,
Running,
Paused,
Finished,
@@ -95,7 +96,9 @@
void setAlternatesDirection(bool alternates) { m_alternatesDirection = alternates; }
bool isFinishedAt(double monotonicTime) const;
- bool isFinished() const { return m_runState == Finished || m_runState == Aborted; }
+ bool isFinished() const { return m_runState == Finished
+ || m_runState == Aborted
+ || m_runState == WaitingForDeletion; }
CCAnimationCurve* curve() { return m_curve.get(); }
const CCAnimationCurve* curve() const { return m_curve.get(); }
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp (120325 => 120326)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp 2012-06-14 14:44:37 UTC (rev 120325)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp 2012-06-14 14:57:10 UTC (rev 120326)
@@ -84,7 +84,7 @@
void CCLayerAnimationController::suspendAnimations(double monotonicTime)
{
for (size_t i = 0; i < m_activeAnimations.size(); ++i) {
- if (m_activeAnimations[i]->runState() != CCActiveAnimation::Finished && m_activeAnimations[i]->runState() != CCActiveAnimation::Aborted)
+ if (!m_activeAnimations[i]->isFinished())
m_activeAnimations[i]->setRunState(CCActiveAnimation::Paused, monotonicTime);
}
}
@@ -106,8 +106,14 @@
replaceImplThreadAnimations(controllerImpl);
m_forceSync = false;
} else {
+ purgeAnimationsMarkedForDeletion();
pushNewAnimationsToImplThread(controllerImpl);
+
+ // Remove finished impl side animations only after pushing,
+ // and only after the animations are deleted on the main thread
+ // this insures we will never push an animation twice.
removeAnimationsCompletedOnMainThread(controllerImpl);
+
pushPropertiesToImplThread(controllerImpl);
}
}
@@ -119,7 +125,7 @@
startAnimationsWaitingForTargetAvailability(monotonicTime, events);
resolveConflicts(monotonicTime);
tickAnimations(monotonicTime);
- purgeFinishedAnimations(monotonicTime, events);
+ markAnimationsForDeletion(monotonicTime, events);
startAnimationsWaitingForTargetAvailability(monotonicTime, events);
}
@@ -149,7 +155,7 @@
bool CCLayerAnimationController::hasActiveAnimation() const
{
for (size_t i = 0; i < m_activeAnimations.size(); ++i) {
- if (m_activeAnimations[i]->runState() != CCActiveAnimation::Finished && m_activeAnimations[i]->runState() != CCActiveAnimation::Aborted)
+ if (!m_activeAnimations[i]->isFinished())
return true;
}
return false;
@@ -319,14 +325,14 @@
}
}
-void CCLayerAnimationController::purgeFinishedAnimations(double monotonicTime, CCAnimationEventsVector* events)
+
+void CCLayerAnimationController::markAnimationsForDeletion(double monotonicTime, CCAnimationEventsVector* events)
{
- // Each iteration, m_activeAnimations.size() decreases or i increments,
- // guaranteeing progress towards loop termination.
- size_t i = 0;
- while (i < m_activeAnimations.size()) {
+ for (size_t i = 0; i < m_activeAnimations.size(); i++) {
int groupId = m_activeAnimations[i]->group();
bool allAnimsWithSameIdAreFinished = false;
+ // If an animation is finished, and not already marked for deletion,
+ // Find out if all other animations in the same group are also finished.
if (m_activeAnimations[i]->isFinished()) {
allAnimsWithSameIdAreFinished = true;
for (size_t j = 0; j < m_activeAnimations.size(); ++j) {
@@ -339,19 +345,23 @@
if (allAnimsWithSameIdAreFinished) {
// We now need to remove all animations with the same group id as groupId
// (and send along animation finished notifications, if necessary).
- // Each iteration, m_activeAnimations.size() decreases or j increments,
- // guaranteeing progress towards loop termination. Also, we are guaranteed
- // to remove at least one active animation.
- for (size_t j = i; j < m_activeAnimations.size();) {
- if (groupId != m_activeAnimations[j]->group())
- j++;
- else {
+ for (size_t j = i; j < m_activeAnimations.size(); j++) {
+ if (groupId == m_activeAnimations[j]->group()) {
if (events)
events->append(CCAnimationEvent(CCAnimationEvent::Finished, m_client->id(), m_activeAnimations[j]->group(), m_activeAnimations[j]->targetProperty(), monotonicTime));
- m_activeAnimations.remove(j);
+ m_activeAnimations[j]->setRunState(CCActiveAnimation::WaitingForDeletion, monotonicTime);
}
}
- } else
+ }
+ }
+}
+
+void CCLayerAnimationController::purgeAnimationsMarkedForDeletion()
+{
+ for (size_t i = 0; i < m_activeAnimations.size();) {
+ if (m_activeAnimations[i]->runState() == CCActiveAnimation::WaitingForDeletion)
+ m_activeAnimations.remove(i);
+ else
i++;
}
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.h (120325 => 120326)
--- trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.h 2012-06-14 14:44:37 UTC (rev 120325)
+++ trunk/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.h 2012-06-14 14:57:10 UTC (rev 120326)
@@ -117,7 +117,8 @@
void startAnimationsWaitingForStartTime(double monotonicTime, CCAnimationEventsVector*);
void startAnimationsWaitingForTargetAvailability(double monotonicTime, CCAnimationEventsVector*);
void resolveConflicts(double monotonicTime);
- void purgeFinishedAnimations(double monotonicTime, CCAnimationEventsVector*);
+ void markAnimationsForDeletion(double monotonicTime, CCAnimationEventsVector*);
+ void purgeAnimationsMarkedForDeletion();
void tickAnimations(double monotonicTime);
