Allow new transitions to run even when controller is suspended
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jul 2013 00:57:31 +0000 (00:57 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jul 2013 00:57:31 +0000 (00:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=119171
<rdar://problem/14511404>

Reviewed by Simon Fraser.

Source/WebCore:

Expose a new property on AnimationController that allows newly created
animations to run even if the controller says it is suspended. See WebKit
ChangeLog for more details.

Test: transitions/created-while-suspended.html

* WebCore.exp.in: Export the new methods so WebView can use them.
* page/animation/AnimationController.cpp:
(WebCore::AnimationControllerPrivate::AnimationControllerPrivate): Initialize new flag to false.
(WebCore::AnimationControllerPrivate::startAnimationsIfNotSuspended): Check new flag is not true.
(WebCore::AnimationControllerPrivate::setAllowsNewAnimationsWhileSuspended): Expose setter.
(WebCore::AnimationController::allowsNewAnimationsWhileSuspended): "Public" getter.
(WebCore::AnimationController::setAllowsNewAnimationsWhileSuspended): "Public" setter.
* page/animation/AnimationController.h:
* page/animation/AnimationControllerPrivate.h:
(WebCore::AnimationControllerPrivate::allowsNewAnimationsWhileSuspended):
* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::CompositeAnimation): Only suspend if new flag is false. Everything else
relies on the m_suspended flag, so the real code change is this one line.

Source/WebKit/mac:

Expose a new SPI on WebView that triggers the (buggy) old behaviour
for animations, such that any newly created animations will start even
when the document is supposedly suspended. It turns out that client content was
unknowingly relying on this behaviour - e.g. suspending a view, loading a
bunch of new content, bringing the view on screen and then unsuspending. In this
situation, we were not running CSS transitions, because the page was suspended.
However, JS was still triggering them, and content was expecting a transitionEnd event.

* WebView/WebView.mm:
(-[WebView allowsNewCSSAnimationsWhileSuspended]): Calls into AnimationController.
(-[WebView setAllowsNewCSSAnimationsWhileSuspended:]): Ditto.
* WebView/WebViewPrivate.h: New methods listed above.

LayoutTests:

This is actually a test to make sure this fix didn't break anything. There is no
way to trigger the new behaviour from the test system (or from Safari).

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/transitions/created-while-suspended-expected.txt [new file with mode: 0644]
LayoutTests/transitions/created-while-suspended.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/page/animation/AnimationController.cpp
Source/WebCore/page/animation/AnimationController.h
Source/WebCore/page/animation/AnimationControllerPrivate.h
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebView.mm
Source/WebKit/mac/WebView/WebViewPrivate.h

index 4ac9195..1811078 100644 (file)
@@ -1,3 +1,14 @@
+2013-07-26  Dean Jackson  <dino@apple.com>
+
+        Allow new transitions to run even when controller is suspended
+        https://bugs.webkit.org/show_bug.cgi?id=119171
+        <rdar://problem/14511404>
+
+        Reviewed by Simon Fraser.
+
+        This is actually a test to make sure this fix didn't break anything. There is no
+        way to trigger the new behaviour from the test system (or from Safari).
+
 2013-07-26  Bem Jones-Bey  <bjonesbe@adobe.com>
 
         [CSS Shapes] New positioning model: support for polygon shape-outside
diff --git a/LayoutTests/transitions/created-while-suspended-expected.txt b/LayoutTests/transitions/created-while-suspended-expected.txt
new file mode 100644 (file)
index 0000000..87689a5
--- /dev/null
@@ -0,0 +1,12 @@
+This test creates a box, adds a transition, then sets the left property. It will only have reproducible output when run in the test system
+
+*** Starting test.
+Transitions should not be suspended: PASS
+*** Suspending Animations/Transitions
+Transitions should be suspended: PASS
+*** Creating the box.
+*** Adding transition property and setting left to 100px. We should NOT see transition events.
+*** Resuming Animations/Transitions
+Transitions should not be suspended: PASS
+*** Test finished
+
diff --git a/LayoutTests/transitions/created-while-suspended.html b/LayoutTests/transitions/created-while-suspended.html
new file mode 100644 (file)
index 0000000..4ebff9f
--- /dev/null
@@ -0,0 +1,92 @@
+<title>Test that newly created transitions do not run while we are suspended</title>
+<style>
+#box {
+    position: relative;
+    left: 0px;
+    height: 50px;
+    width: 50px;
+    background-color: blue;
+}
+</style>
+<script>
+var box;
+
+function suspend()
+{
+    if (window.internals)
+        internals.suspendAnimations(document);
+}
+
+function resume()
+{
+    if (window.internals)
+        internals.resumeAnimations(document);
+}
+
+function transitionEnded(event)
+{
+    log("#### Transition ended on element with id: " + event.target.id);
+}
+
+function suspendAndCreate()
+{
+    log("*** Suspending Animations/Transitions");
+    suspend();
+    setTimeout(function() {
+        if (window.internals)
+            log("Transitions should be suspended: " + (window.internals.animationsAreSuspended(document) ? "PASS" : "FAIL"));
+
+        log("*** Creating the box.")
+        box = document.createElement("div");
+        box.id = "box";
+        document.addEventListener("webkitTransitionEnd", transitionEnded, false);
+        document.body.insertBefore(box, document.querySelector("#results"));
+        setTimeout(function() {
+            log("*** Adding transition property and setting left to 100px. We should NOT see transition events.")
+            box.style.webkitTransitionDuration = "100ms";
+            box.style.left = "100px";
+            setTimeout(endTest, 200);
+        }, 100);
+    }, 100);
+}
+
+function endTest()
+{
+    log("*** Resuming Animations/Transitions");
+    resume();
+    if (window.internals)
+        log("Transitions should not be suspended: " + (window.internals.animationsAreSuspended(document) ? "FAIL" : "PASS"));
+
+    resume(); // Just in case.
+    log("*** Test finished");
+    if (window.testRunner)
+        setTimeout(function () { testRunner.notifyDone();}, 8000);
+}
+
+function startTest()
+{
+    log("*** Starting test.")
+
+    if (window.internals)
+        log("Transitions should not be suspended: " + (window.internals.animationsAreSuspended(document) ? "FAIL" : "PASS"));
+
+    suspendAndCreate();
+}
+
+function log(message)
+{
+    var results = document.getElementById("results");
+    results.innerHTML = results.innerHTML + message + "<br>";
+}
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+window.addEventListener("load", startTest, false);
+
+</script>
+<p>This test creates a box, adds a transition, then sets the left property. It will only have reproducible output when run in the test system</p>
+<div id="results">
+</div>
index 425eb5e..5d99fec 100644 (file)
@@ -1,3 +1,31 @@
+2013-07-26  Dean Jackson  <dino@apple.com>
+
+        Allow new transitions to run even when controller is suspended
+        https://bugs.webkit.org/show_bug.cgi?id=119171
+        <rdar://problem/14511404>
+
+        Reviewed by Simon Fraser.
+
+        Expose a new property on AnimationController that allows newly created
+        animations to run even if the controller says it is suspended. See WebKit
+        ChangeLog for more details.
+
+        Test: transitions/created-while-suspended.html
+
+        * WebCore.exp.in: Export the new methods so WebView can use them.
+        * page/animation/AnimationController.cpp:
+        (WebCore::AnimationControllerPrivate::AnimationControllerPrivate): Initialize new flag to false.
+        (WebCore::AnimationControllerPrivate::startAnimationsIfNotSuspended): Check new flag is not true.
+        (WebCore::AnimationControllerPrivate::setAllowsNewAnimationsWhileSuspended): Expose setter.
+        (WebCore::AnimationController::allowsNewAnimationsWhileSuspended): "Public" getter.
+        (WebCore::AnimationController::setAllowsNewAnimationsWhileSuspended): "Public" setter.
+        * page/animation/AnimationController.h:
+        * page/animation/AnimationControllerPrivate.h:
+        (WebCore::AnimationControllerPrivate::allowsNewAnimationsWhileSuspended):
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::CompositeAnimation): Only suspend if new flag is false. Everything else
+        relies on the m_suspended flag, so the real code change is this one line.
+
 2013-07-26  Brent Fulgham  <bfulgham@apple.com>
 
         [Windows] Unreviewed build fix.
index 89f5ebf..57fbc35 100644 (file)
@@ -622,6 +622,8 @@ __ZN7WebCore19AnimationController16resumeAnimationsEv
 __ZN7WebCore19AnimationController17suspendAnimationsEv
 __ZN7WebCore19AnimationController20pauseAnimationAtTimeEPNS_12RenderObjectERKN3WTF12AtomicStringEd
 __ZN7WebCore19AnimationController21pauseTransitionAtTimeEPNS_12RenderObjectERKN3WTF6StringEd
+__ZN7WebCore19AnimationController36setAllowsNewAnimationsWhileSuspendedEb
+__ZNK7WebCore19AnimationController33allowsNewAnimationsWhileSuspendedEv
 __ZN7WebCore19BackForwardListImpl10removeItemEPNS_11HistoryItemE
 __ZN7WebCore19BackForwardListImpl10setEnabledEb
 __ZN7WebCore19BackForwardListImpl11currentItemEv
index 4cca653..e3d0715 100644 (file)
@@ -59,6 +59,7 @@ AnimationControllerPrivate::AnimationControllerPrivate(Frame* frame)
     , m_animationsWaitingForStartTimeResponse()
     , m_waitingForAsyncStartNotification(false)
     , m_isSuspended(false)
+    , m_allowsNewAnimationsWhileSuspended(false)
 {
 }
 
@@ -322,10 +323,15 @@ void AnimationControllerPrivate::resumeAnimationsForDocument(Document* document)
 
 void AnimationControllerPrivate::startAnimationsIfNotSuspended(Document* document)
 {
-    if (!isSuspended())
+    if (!isSuspended() || allowsNewAnimationsWhileSuspended())
         resumeAnimationsForDocument(document);
 }
 
+void AnimationControllerPrivate::setAllowsNewAnimationsWhileSuspended(bool allowed)
+{
+    m_allowsNewAnimationsWhileSuspended = allowed;
+}
+
 bool AnimationControllerPrivate::pauseAnimationAtTime(RenderObject* renderer, const AtomicString& name, double t)
 {
     if (!renderer)
@@ -603,6 +609,16 @@ void AnimationController::resumeAnimations()
     m_data->resumeAnimations();
 }
 
+bool AnimationController::allowsNewAnimationsWhileSuspended() const
+{
+    return m_data->allowsNewAnimationsWhileSuspended();
+}
+
+void AnimationController::setAllowsNewAnimationsWhileSuspended(bool allowed)
+{
+    m_data->setAllowsNewAnimationsWhileSuspended(allowed);
+}
+
 #if ENABLE(REQUEST_ANIMATION_FRAME)
 void AnimationController::serviceAnimations()
 {
index 9a267d4..23b1573 100644 (file)
@@ -76,6 +76,9 @@ public:
 
     void beginAnimationUpdate();
     void endAnimationUpdate();
+
+    bool allowsNewAnimationsWhileSuspended() const;
+    void setAllowsNewAnimationsWhileSuspended(bool);
     
     static bool supportsAcceleratedAnimationOfProperty(CSSPropertyID);
 
index 3f46328..823bfcc 100644 (file)
@@ -109,7 +109,10 @@ public:
     void animationWillBeRemoved(AnimationBase*);
 
     void updateAnimationTimerForRenderer(RenderObject*);
-    
+
+    bool allowsNewAnimationsWhileSuspended() const { return m_allowsNewAnimationsWhileSuspended; }
+    void setAllowsNewAnimationsWhileSuspended(bool);
+
 private:
     void animationTimerFired(Timer<AnimationControllerPrivate>*);
 
@@ -142,6 +145,11 @@ private:
     WaitingAnimationsSet m_animationsWaitingForStartTimeResponse;
     bool m_waitingForAsyncStartNotification;
     bool m_isSuspended;
+
+    // Used to flag whether we should revert to previous buggy
+    // behavior of allowing new transitions and animations to
+    // run even when this object is suspended.
+    bool m_allowsNewAnimationsWhileSuspended;
 };
 
 } // namespace WebCore
index e7d1837..a3641ea 100644 (file)
@@ -44,7 +44,7 @@ namespace WebCore {
 CompositeAnimation::CompositeAnimation(AnimationControllerPrivate* animationController)
     : m_animationController(animationController)
 {
-    m_suspended = animationController->isSuspended();
+    m_suspended = animationController->isSuspended() && !animationController->allowsNewAnimationsWhileSuspended();
 }
 
 CompositeAnimation::~CompositeAnimation()
index 89fcb51..e6e40a8 100644 (file)
@@ -1,3 +1,24 @@
+2013-07-26  Dean Jackson  <dino@apple.com>
+
+        Allow new transitions to run even when controller is suspended
+        https://bugs.webkit.org/show_bug.cgi?id=119171
+        <rdar://problem/14511404>
+
+        Reviewed by Simon Fraser.
+
+        Expose a new SPI on WebView that triggers the (buggy) old behaviour
+        for animations, such that any newly created animations will start even
+        when the document is supposedly suspended. It turns out that client content was
+        unknowingly relying on this behaviour - e.g. suspending a view, loading a
+        bunch of new content, bringing the view on screen and then unsuspending. In this
+        situation, we were not running CSS transitions, because the page was suspended.
+        However, JS was still triggering them, and content was expecting a transitionEnd event.
+
+        * WebView/WebView.mm:
+        (-[WebView allowsNewCSSAnimationsWhileSuspended]): Calls into AnimationController.
+        (-[WebView setAllowsNewCSSAnimationsWhileSuspended:]): Ditto.
+        * WebView/WebViewPrivate.h: New methods listed above.
+
 2013-07-26  Anders Carlsson  <andersca@apple.com>
 
         Add another method that we need to set aside subviews for
index baf235a..2f02074 100644 (file)
@@ -2790,6 +2790,22 @@ static Vector<String> toStringVector(NSArray* patterns)
     pageGroup->removeAllUserContent();
 }
 
+- (BOOL)allowsNewCSSAnimationsWhileSuspended
+{
+    Frame* frame = core([self mainFrame]);
+    if (frame)
+        return frame->animation()->allowsNewAnimationsWhileSuspended();
+
+    return false;
+}
+
+- (void)setAllowsNewCSSAnimationsWhileSuspended:(BOOL)allowed
+{
+    Frame* frame = core([self mainFrame]);
+    if (frame)
+        frame->animation()->setAllowsNewAnimationsWhileSuspended(allowed);
+}
+
 - (BOOL)cssAnimationsSuspended
 {
     // should ask the page!
index 4d71b16..0eb2159 100644 (file)
@@ -564,6 +564,14 @@ Could be worth adding to the API.
 */
 - (void)setCSSAnimationsSuspended:(BOOL)suspended;
 
+/*
+    SPI to revert back to buggy behavior that would allow new transitions
+    and animations to run even when the view is suspended (e.g. loading a
+    new document).
+*/
+- (BOOL)allowsNewCSSAnimationsWhileSuspended;
+- (void)setAllowsNewCSSAnimationsWhileSuspended:(BOOL)allowed;
+
 + (void)_setDomainRelaxationForbidden:(BOOL)forbidden forURLScheme:(NSString *)scheme;
 + (void)_registerURLSchemeAsSecure:(NSString *)scheme;
 + (void)_registerURLSchemeAsAllowingLocalStorageAccessInPrivateBrowsing:(NSString *)scheme;