- Revision
- 113381
- Author
- [email protected]
- Date
- 2012-04-05 14:29:38 -0700 (Thu, 05 Apr 2012)
Log Message
[mac] requestAnimationFrame sometimes stuck when page loads in a background tab https://bugs.webkit.org/show_bug.cgi?id=76105
Reviewed by Simon Fraser.
Source/WebCore:
Fix three issues with requestAnimationFrame:
- It's possible for the call to rAF to come to the document before there
is a page associated. Added a guard for this.
- A page may try to suspend the scripted animations before the
ScriptedAnimationController exists, in which case we need to
suspend it immediately after it is created. Do this by keeping
track of the state in Page. Otherwise rAF would be busy looping
on hidden pages until they are brought to the front and hidden again.
- A page created in the background (from WebKit1) does not get
informed it is not visible. This can mean that resume() is called
more times than suspend() and we get into a state where the number
of suspensions becomes -1, and thus fails truthiness tests. Clamp it
to values >= 0.
No new tests, since this is not automatically testable. The most reliable test
is to open a page with rAF in a background tab within Safari.
* dom/Document.cpp:
(WebCore::Document::webkitRequestAnimationFrame):
* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::resume):
* page/Page.cpp:
(WebCore::Page::Page):
(WebCore::Page::suspendScriptedAnimations):
(WebCore::Page::resumeScriptedAnimations):
* page/Page.h:
(WebCore::Page::scriptedAnimationsSuspended):
(Page):
Source/WebKit2:
When we are resuming painting only start the scripted animations
if we're a visible window. This can happen when tabs are opened
in the background.
* WebProcess/WebPage/DrawingAreaImpl.cpp:
(WebKit::DrawingAreaImpl::resumePainting):
LayoutTests:
Add fast/animation/request-animation-frame-during-modal.html
to the skipped list. This patch will ensure that non-visible windows
do not get a requestAnimationFrame callback, so the test will timeout.
See https://bugs.webkit.org/show_bug.cgi?id=83239 to re-enable.
* platform/mac/Skipped:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (113380 => 113381)
--- trunk/LayoutTests/ChangeLog 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/LayoutTests/ChangeLog 2012-04-05 21:29:38 UTC (rev 113381)
@@ -1,3 +1,17 @@
+2012-04-05 Dean Jackson <[email protected]>
+
+ [mac] requestAnimationFrame sometimes stuck when page loads in a background tab
+ https://bugs.webkit.org/show_bug.cgi?id=76105
+
+ Add fast/animation/request-animation-frame-during-modal.html
+ to the skipped list. This patch will ensure that non-visible windows
+ do not get a requestAnimationFrame callback, so the test will timeout.
+ See https://bugs.webkit.org/show_bug.cgi?id=83239 to re-enable.
+
+ Reviewed by Simon Fraser.
+
+ * platform/mac/Skipped:
+
2012-04-05 Tony Chang <[email protected]>
[chromium] Unreviewed, moving linux result from 32-bit directory to 64 and 32 bit directory.
Modified: trunk/LayoutTests/platform/mac/Skipped (113380 => 113381)
--- trunk/LayoutTests/platform/mac/Skipped 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/LayoutTests/platform/mac/Skipped 2012-04-05 21:29:38 UTC (rev 113381)
@@ -772,3 +772,6 @@
# https://bugs.webkit.org/show_bug.cgi?id=82886
inspector/styles/override-screen-size.html
+
+# https://bugs.webkit.org/show_bug.cgi?id=83239
+fast/animation/request-animation-frame-during-modal.html
Modified: trunk/Source/WebCore/ChangeLog (113380 => 113381)
--- trunk/Source/WebCore/ChangeLog 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebCore/ChangeLog 2012-04-05 21:29:38 UTC (rev 113381)
@@ -1,3 +1,39 @@
+2012-04-05 Dean Jackson <[email protected]>
+
+ [mac] requestAnimationFrame sometimes stuck when page loads in a background tab
+ https://bugs.webkit.org/show_bug.cgi?id=76105
+
+ Reviewed by Simon Fraser.
+
+ Fix three issues with requestAnimationFrame:
+ - It's possible for the call to rAF to come to the document before there
+ is a page associated. Added a guard for this.
+ - A page may try to suspend the scripted animations before the
+ ScriptedAnimationController exists, in which case we need to
+ suspend it immediately after it is created. Do this by keeping
+ track of the state in Page. Otherwise rAF would be busy looping
+ on hidden pages until they are brought to the front and hidden again.
+ - A page created in the background (from WebKit1) does not get
+ informed it is not visible. This can mean that resume() is called
+ more times than suspend() and we get into a state where the number
+ of suspensions becomes -1, and thus fails truthiness tests. Clamp it
+ to values >= 0.
+
+ No new tests, since this is not automatically testable. The most reliable test
+ is to open a page with rAF in a background tab within Safari.
+
+ * dom/Document.cpp:
+ (WebCore::Document::webkitRequestAnimationFrame):
+ * dom/ScriptedAnimationController.cpp:
+ (WebCore::ScriptedAnimationController::resume):
+ * page/Page.cpp:
+ (WebCore::Page::Page):
+ (WebCore::Page::suspendScriptedAnimations):
+ (WebCore::Page::resumeScriptedAnimations):
+ * page/Page.h:
+ (WebCore::Page::scriptedAnimationsSuspended):
+ (Page):
+
2012-04-05 Brady Eidson <[email protected]>
<rdar://problem/9359029> and https://bugs.webkit.org/show_bug.cgi?id=83311
Modified: trunk/Source/WebCore/dom/Document.cpp (113380 => 113381)
--- trunk/Source/WebCore/dom/Document.cpp 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-04-05 21:29:38 UTC (rev 113381)
@@ -5638,10 +5638,15 @@
{
if (!m_scriptedAnimationController) {
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- m_scriptedAnimationController = ScriptedAnimationController::create(this, page()->displayID());
+ m_scriptedAnimationController = ScriptedAnimationController::create(this, page() ? page()->displayID() : 0);
#else
m_scriptedAnimationController = ScriptedAnimationController::create(this, 0);
#endif
+ // It's possible that the Page may have suspended scripted animations before
+ // we were created. We need to make sure that we don't start up the animation
+ // controller on a background tab, for example.
+ if (!page() || page()->scriptedAnimationsSuspended())
+ m_scriptedAnimationController->suspend();
}
return m_scriptedAnimationController->registerCallback(callback, animationElement);
Modified: trunk/Source/WebCore/dom/ScriptedAnimationController.cpp (113380 => 113381)
--- trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2012-04-05 21:29:38 UTC (rev 113381)
@@ -72,7 +72,11 @@
void ScriptedAnimationController::resume()
{
- --m_suspendCount;
+ // It would be nice to put an ASSERT(m_suspendCount > 0) here, but in WK1 resume() can be called
+ // even when suspend hasn't (if a tab was created in the background).
+ if (m_suspendCount > 0)
+ --m_suspendCount;
+
if (!m_suspendCount && m_callbacks.size())
scheduleAnimation();
}
Modified: trunk/Source/WebCore/page/Page.cpp (113380 => 113381)
--- trunk/Source/WebCore/page/Page.cpp 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebCore/page/Page.cpp 2012-04-05 21:29:38 UTC (rev 113381)
@@ -160,6 +160,7 @@
, m_isPainting(false)
#endif
, m_alternativeTextClient(pageClients.alternativeTextClient)
+ , m_scriptedAnimationsSuspended(false)
{
if (!allPages) {
allPages = new HashSet<Page*>;
@@ -717,6 +718,7 @@
void Page::suspendScriptedAnimations()
{
+ m_scriptedAnimationsSuspended = true;
for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext()) {
if (frame->document())
frame->document()->suspendScriptedAnimationControllerCallbacks();
@@ -725,6 +727,7 @@
void Page::resumeScriptedAnimations()
{
+ m_scriptedAnimationsSuspended = false;
for (Frame* frame = mainFrame(); frame; frame = frame->tree()->traverseNext()) {
if (frame->document())
frame->document()->resumeScriptedAnimationControllerCallbacks();
Modified: trunk/Source/WebCore/page/Page.h (113380 => 113381)
--- trunk/Source/WebCore/page/Page.h 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebCore/page/Page.h 2012-04-05 21:29:38 UTC (rev 113381)
@@ -273,6 +273,7 @@
void suspendScriptedAnimations();
void resumeScriptedAnimations();
+ bool scriptedAnimationsSuspended() const { return m_scriptedAnimationsSuspended; }
void userStyleSheetLocationChanged();
const String& userStyleSheet() const;
@@ -438,6 +439,8 @@
bool m_isPainting;
#endif
AlternativeTextClient* m_alternativeTextClient;
+
+ bool m_scriptedAnimationsSuspended;
};
} // namespace WebCore
Modified: trunk/Source/WebKit2/ChangeLog (113380 => 113381)
--- trunk/Source/WebKit2/ChangeLog 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebKit2/ChangeLog 2012-04-05 21:29:38 UTC (rev 113381)
@@ -1,3 +1,17 @@
+2012-04-05 Dean Jackson <[email protected]>
+
+ [mac] requestAnimationFrame sometimes stuck when page loads in a background tab
+ https://bugs.webkit.org/show_bug.cgi?id=76105
+
+ Reviewed by Simon Fraser.
+
+ When we are resuming painting only start the scripted animations
+ if we're a visible window. This can happen when tabs are opened
+ in the background.
+
+ * WebProcess/WebPage/DrawingAreaImpl.cpp:
+ (WebKit::DrawingAreaImpl::resumePainting):
+
2012-04-05 Patrick Gansterer <[email protected]>
[Qt] Correct <wtf/*.h> include paths.
Modified: trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp (113380 => 113381)
--- trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp 2012-04-05 21:18:22 UTC (rev 113380)
+++ trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp 2012-04-05 21:29:38 UTC (rev 113381)
@@ -436,7 +436,8 @@
// FIXME: We shouldn't always repaint everything here.
setNeedsDisplay(m_webPage->bounds());
- m_webPage->corePage()->resumeScriptedAnimations();
+ if (m_webPage->windowIsVisible())
+ m_webPage->corePage()->resumeScriptedAnimations();
}
void DrawingAreaImpl::enterAcceleratedCompositingMode(GraphicsLayer* graphicsLayer)
