[chromium] assertion being hit in CCLayerAnimationController

https://bugs.webkit.org/show_bug.cgi?id=82237

Patch by Ian Vollick <vollick@chromium.org> on 2012-03-30
Reviewed by James Robinson.

Source/WebCore:

Animations are no longer pushed to the impl thread if they have already completed.

Tested in CCLayerAnimationControllerTest.doNotSyncFinishedAnimation

* platform/graphics/chromium/cc/CCLayerAnimationController.cpp:
(WebCore::CCLayerAnimationController::pushNewAnimationsToImplThread):

Source/WebKit/chromium:

* tests/CCLayerAnimationControllerTest.cpp:
(WebKitTests::TEST):
(WebKitTests):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@112701 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index aea34b3..480c4cf 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2012-03-30  Ian Vollick  <vollick@chromium.org>
+
+        [chromium] assertion being hit in CCLayerAnimationController
+        https://bugs.webkit.org/show_bug.cgi?id=82237
+
+        Reviewed by James Robinson.
+
+        Animations are no longer pushed to the impl thread if they have already completed.
+
+        Tested in CCLayerAnimationControllerTest.doNotSyncFinishedAnimation
+
+        * platform/graphics/chromium/cc/CCLayerAnimationController.cpp:
+        (WebCore::CCLayerAnimationController::pushNewAnimationsToImplThread):
+
 2012-03-30  Ryosuke Niwa  <rniwa@webkit.org>
 
         Build fix after r112699.

diff --git a/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp b/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp
index 98ef254..7f5a928 100644 (file)
--- a/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp
+++ b/Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp
@@ -239,14 +239,22 @@ void CCLayerAnimationController::pushNewAnimationsToImplThread(CCLayerAnimationC
 {
     // Any new animations owned by the main thread's controller are cloned and adde to the impl thread's controller.
     for (size_t i = 0; i < m_activeAnimations.size(); ++i) {
-        if (!controllerImpl->getActiveAnimation(m_activeAnimations[i]->group(), m_activeAnimations[i]->targetProperty())) {
-            OwnPtr<CCActiveAnimation> toAdd(m_activeAnimations[i]->cloneForImplThread());
-            ASSERT(m_activeAnimations[i]->needsSynchronizedStartTime());
-            ASSERT(!toAdd->needsSynchronizedStartTime());
-            // The new animation should be set to run as soon as possible.
-            toAdd->setRunState(CCActiveAnimation::WaitingForTargetAvailability, 0);
-            controllerImpl->add(toAdd.release());
-        }
+        // If the animation is already running on the impl thread, there is no need to copy it over.
+        if (controllerImpl->getActiveAnimation(m_activeAnimations[i]->group(), m_activeAnimations[i]->targetProperty()))
+            continue;
+
+        // If the animation is not running on the impl thread, it does not necessarily mean that it needs
+        // to be copied over and started; it may have already finished. In this case, the impl thread animation
+        // will have already notified that it has started and the main thread animation will no longer need
+        // a synchronized start time.
+        if (!m_activeAnimations[i]->needsSynchronizedStartTime())
+            continue;
+
+        OwnPtr<CCActiveAnimation> toAdd(m_activeAnimations[i]->cloneForImplThread());
+        ASSERT(!toAdd->needsSynchronizedStartTime());
+        // The new animation should be set to run as soon as possible.
+        toAdd->setRunState(CCActiveAnimation::WaitingForTargetAvailability, 0);
+        controllerImpl->add(toAdd.release());
     }
 }
 

diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index 32a00bb..f8a5adb 100644 (file)
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,14 @@
+2012-03-30  Ian Vollick  <vollick@chromium.org>
+
+        [chromium] assertion being hit in CCLayerAnimationController
+        https://bugs.webkit.org/show_bug.cgi?id=82237
+
+        Reviewed by James Robinson.
+
+        * tests/CCLayerAnimationControllerTest.cpp:
+        (WebKitTests::TEST):
+        (WebKitTests):
+
 2012-03-30  Alexander Pavlov  <apavlov@chromium.org>
 
         [Chromium] Unreviewed Chromium Mac build fix.

diff --git a/Source/WebKit/chromium/tests/CCLayerAnimationControllerTest.cpp b/Source/WebKit/chromium/tests/CCLayerAnimationControllerTest.cpp
index 25199b3..113b639 100644 (file)
--- a/Source/WebKit/chromium/tests/CCLayerAnimationControllerTest.cpp
+++ b/Source/WebKit/chromium/tests/CCLayerAnimationControllerTest.cpp
@@ -141,6 +141,36 @@ TEST(CCLayerAnimationControllerTest, syncNewAnimation)
     EXPECT_EQ(CCActiveAnimation::WaitingForTargetAvailability, controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity)->runState());
 }
 
+TEST(CCLayerAnimationControllerTest, doNotSyncFinishedAnimation)
+{
+    FakeLayerAnimationControllerClient dummyImpl;
+    OwnPtr<CCLayerAnimationController> controllerImpl(CCLayerAnimationController::create(&dummyImpl));
+    FakeLayerAnimationControllerClient dummy;
+    OwnPtr<CCLayerAnimationController> controller(CCLayerAnimationController::create(&dummy));
+
+    EXPECT_FALSE(controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity));
+
+    addOpacityTransitionToController(*controller, 1, 0, 1, false);
+
+    controller->pushAnimationUpdatesTo(controllerImpl.get());
+
+    EXPECT_TRUE(controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity));
+    EXPECT_EQ(CCActiveAnimation::WaitingForTargetAvailability, controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity)->runState());
+
+    // Notify main thread controller that the animation has started.
+    controller->notifyAnimationStarted(CCAnimationStartedEvent(0, 0, CCActiveAnimation::Opacity, 0));
+
+    // Force animation to complete on impl thread.
+    controllerImpl->removeAnimation(0);
+
+    EXPECT_FALSE(controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity));
+
+    controller->pushAnimationUpdatesTo(controllerImpl.get());
+
+    // Even though the main thread has a 'new' animation, it should not be pushed because the animation has already completed on the impl thread.
+    EXPECT_FALSE(controllerImpl->getActiveAnimation(0, CCActiveAnimation::Opacity));
+}
+
 // Tests that transitioning opacity from 0 to 1 works as expected.
 TEST(CCLayerAnimationControllerTest, TrivialTransition)
 {