Unify iOS Frame::setTimersPaused() logic and Frame::{suspend, resume}ActiveDOMObjects...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Dec 2015 02:51:11 +0000 (02:51 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Dec 2015 02:51:11 +0000 (02:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152006

Reviewed by Simon Fraser.

Currently we have almost identical logic to suspend and resume a web page for iOS and non-iOS ports.
We should unify this logic instead of duplicating it.

* dom/ActiveDOMObject.h: Remove iOS-specific enumeration DocumentWillBePaused and standardize on
enumerator PageWillBeSuspended.
* dom/Document.cpp:
(WebCore::Document::didBecomeCurrentDocumentInFrame): Unify iOS and non-iOS-specific code.
(WebCore::Document::suspendScheduledTasks): Ignore subsequent calls to this function so long as the reason for
the first invocation was ActiveDOMObject::PageWillBeSuspended. Such a subsequent call may occur as part of
handling a scroll or zoom gesture.
* dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::suspendActiveDOMObjects): Ignore subsequent calls to this function
so long as the reason for the first invocation was ActiveDOMObject::PageWillBeSuspended. Such a subsequent
call may occur as part of the process of a page being added to the page cache.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::suspend): Remove case for ActiveDOMObject::DocumentWillBePaused as this
enumerator is being removed.
* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Write logic that used Frame::timersPaused() terms of
ScriptExecutionContext::activeDOMObjectsAreSuspended() as we are removing Frame::timersPaused().
(WebCore::DOMTimer::fired): Remove iOS-specific assertion with respect to Frame::timersPaused().
This function already asserts the equivalent condition that ScriptExecutionContext::activeDOMObjectsAreSuspended()
evaluates to false. Clean up iOS-specific code that depends on the ScriptExecutionContext being a
Document object by taking advantage of the fact that this assumption is true when shouldBeginObservingChanges
evaluates to true.
* page/Frame.cpp:
(WebCore::Frame::Frame): Remove instance variable m_timersPausedCount and unify the iOS and non-iOS logic.
(WebCore::Frame::suspendActiveDOMObjectsAndAnimations): Standardize on the iOS logic for suspending
DOM objects and animations because it is more comprehensive on what it suspends and works with the deferred
loading machinery (Page::setDefersLoading() - see remarks in Frame::resumeActiveDOMObjectsAndAnimations() for
more details). Specifically, make use of Frame::clearTimers() to suspend non-scripted animations (i.e. non-requestAnimationFrame()
animations), auto-scroll timer, and pending relayouts. And use Document::suspendScheduledTasks() to suspend
all other tasks, including WebSQL database callbacks, active DOM objects, scripted animations and execution of
<script async>/<script defer> JavaScript scripts.
(WebCore::Frame::resumeActiveDOMObjectsAndAnimations): Standardize on the iOS logic for resuming
DOM objects and animations for symmetry and because it works with the deferred loading machinery. We call
Document::resumeScheduledTasks() (which calls Document::resumeActiveDOMObjects()) instead of calling
Document::resumeActiveDOMObjects() directly because the former will ultimately process the queue of pending
tasks (Document::m_pendingTasks).
* page/Frame.h: Remove instance variable m_timersPausedCount.
(WebCore::Frame::timersPaused): Deleted.
* page/ios/FrameIOS.mm:
(WebCore::Frame::setTimersPaused): Write this function in terms of Page::{suspend, resume}ActiveDOMObjectsAndAnimations().
We need to keep this function for Legacy WebKit on iOS.
(WebCore::Frame::setTimersPausedInternal): Deleted.
* rendering/RenderElement.cpp:
(WebCore::shouldRepaintForImageAnimation): Remove iOS-specific code to early return when Frame::timersPaused()
evaluates to true. This function already has the equivalent code to early return when Document::activeDOMObjectsAreSuspended()
evaluates to true.

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

Source/WebCore/ChangeLog
Source/WebCore/dom/ActiveDOMObject.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/ScriptExecutionContext.cpp
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/page/DOMTimer.cpp
Source/WebCore/page/Frame.cpp
Source/WebCore/page/Frame.h
Source/WebCore/page/ios/FrameIOS.mm
Source/WebCore/rendering/RenderElement.cpp

index 4dbf3d53c9df4b835d3c00c6234cc1a34feb353f..1f6e3800eaa532797d7a6440be7e8caf52b00073 100644 (file)
@@ -1,3 +1,60 @@
+2015-12-09  Daniel Bates  <dabates@apple.com>
+
+        Unify iOS Frame::setTimersPaused() logic and Frame::{suspend, resume}ActiveDOMObjectsAndAnimations()
+        https://bugs.webkit.org/show_bug.cgi?id=152006
+
+        Reviewed by Simon Fraser.
+
+        Currently we have almost identical logic to suspend and resume a web page for iOS and non-iOS ports.
+        We should unify this logic instead of duplicating it.
+
+        * dom/ActiveDOMObject.h: Remove iOS-specific enumeration DocumentWillBePaused and standardize on
+        enumerator PageWillBeSuspended.
+        * dom/Document.cpp:
+        (WebCore::Document::didBecomeCurrentDocumentInFrame): Unify iOS and non-iOS-specific code.
+        (WebCore::Document::suspendScheduledTasks): Ignore subsequent calls to this function so long as the reason for
+        the first invocation was ActiveDOMObject::PageWillBeSuspended. Such a subsequent call may occur as part of
+        handling a scroll or zoom gesture.
+        * dom/ScriptExecutionContext.cpp:
+        (WebCore::ScriptExecutionContext::suspendActiveDOMObjects): Ignore subsequent calls to this function
+        so long as the reason for the first invocation was ActiveDOMObject::PageWillBeSuspended. Such a subsequent
+        call may occur as part of the process of a page being added to the page cache.
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::suspend): Remove case for ActiveDOMObject::DocumentWillBePaused as this
+        enumerator is being removed.
+        * page/DOMTimer.cpp:
+        (WebCore::DOMTimer::install): Write logic that used Frame::timersPaused() terms of
+        ScriptExecutionContext::activeDOMObjectsAreSuspended() as we are removing Frame::timersPaused().
+        (WebCore::DOMTimer::fired): Remove iOS-specific assertion with respect to Frame::timersPaused().
+        This function already asserts the equivalent condition that ScriptExecutionContext::activeDOMObjectsAreSuspended()
+        evaluates to false. Clean up iOS-specific code that depends on the ScriptExecutionContext being a
+        Document object by taking advantage of the fact that this assumption is true when shouldBeginObservingChanges
+        evaluates to true.
+        * page/Frame.cpp:
+        (WebCore::Frame::Frame): Remove instance variable m_timersPausedCount and unify the iOS and non-iOS logic.
+        (WebCore::Frame::suspendActiveDOMObjectsAndAnimations): Standardize on the iOS logic for suspending
+        DOM objects and animations because it is more comprehensive on what it suspends and works with the deferred
+        loading machinery (Page::setDefersLoading() - see remarks in Frame::resumeActiveDOMObjectsAndAnimations() for
+        more details). Specifically, make use of Frame::clearTimers() to suspend non-scripted animations (i.e. non-requestAnimationFrame()
+        animations), auto-scroll timer, and pending relayouts. And use Document::suspendScheduledTasks() to suspend
+        all other tasks, including WebSQL database callbacks, active DOM objects, scripted animations and execution of
+        <script async>/<script defer> JavaScript scripts.
+        (WebCore::Frame::resumeActiveDOMObjectsAndAnimations): Standardize on the iOS logic for resuming
+        DOM objects and animations for symmetry and because it works with the deferred loading machinery. We call
+        Document::resumeScheduledTasks() (which calls Document::resumeActiveDOMObjects()) instead of calling
+        Document::resumeActiveDOMObjects() directly because the former will ultimately process the queue of pending
+        tasks (Document::m_pendingTasks).
+        * page/Frame.h: Remove instance variable m_timersPausedCount.
+        (WebCore::Frame::timersPaused): Deleted.
+        * page/ios/FrameIOS.mm:
+        (WebCore::Frame::setTimersPaused): Write this function in terms of Page::{suspend, resume}ActiveDOMObjectsAndAnimations().
+        We need to keep this function for Legacy WebKit on iOS.
+        (WebCore::Frame::setTimersPausedInternal): Deleted.
+        * rendering/RenderElement.cpp:
+        (WebCore::shouldRepaintForImageAnimation): Remove iOS-specific code to early return when Frame::timersPaused()
+        evaluates to true. This function already has the equivalent code to early return when Document::activeDOMObjectsAreSuspended()
+        evaluates to true.
+
 2015-12-09  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: storage/indexeddb/metadata.html fails
index fb1c04eecc84469168f70cee3cc476881871a1a7..3dcc4cd5e7d5d8cd52c70805ec7af73922167dbb 100644 (file)
@@ -55,7 +55,6 @@ public:
         WillDeferLoading,
         PageCache,
         PageWillBeSuspended,
-        DocumentWillBePaused
     };
 
     virtual const char* activeDOMObjectName() const = 0;
index 89a410b4dc21f3af794fa5b39c693c2012961a4f..2ee54f77f7b5b2ec7846704f9da76fd0fe3124cb 100644 (file)
@@ -2207,21 +2207,15 @@ void Document::didBecomeCurrentDocumentInFrame()
         page()->chrome().client().needTouchEvents(true);
 #endif
 
-#if PLATFORM(IOS)
-    // Ensure that document scheduled task state matches frame timer state. It can be out of sync
-    // if timers state changed while the document was not in the frame (possibly in page cache,
-    // or simply newly created).
-    // FIXME: How does this interact with cross-platform code below?
-    if (m_frame->timersPaused())
-        suspendScheduledTasks(ActiveDOMObject::DocumentWillBePaused);
-    else
-        resumeScheduledTasks(ActiveDOMObject::DocumentWillBePaused);
-#endif
-
+    // Ensure that the scheduled task state of the document matches the DOM suspension state of the frame. It can
+    // be out of sync if the DOM suspension state changed while the document was not in the frame (possibly in the
+    // page cache, or simply newly created).
     if (m_frame->activeDOMObjectsAndAnimationsSuspended()) {
-        suspendScriptedAnimationControllerCallbacks();
         m_frame->animation().suspendAnimationsForDocument(this);
-        suspendActiveDOMObjects(ActiveDOMObject::PageWillBeSuspended);
+        suspendScheduledTasks(ActiveDOMObject::PageWillBeSuspended);
+    } else {
+        resumeScheduledTasks(ActiveDOMObject::PageWillBeSuspended);
+        m_frame->animation().resumeAnimationsForDocument(this);
     }
 }
 
@@ -5267,14 +5261,13 @@ void Document::pendingTasksTimerFired()
 
 void Document::suspendScheduledTasks(ActiveDOMObject::ReasonForSuspension reason)
 {
-#if PLATFORM(IOS)
     if (m_scheduledTasksAreSuspended) {
-        ASSERT(reasonForSuspendingActiveDOMObjects() == ActiveDOMObject::DocumentWillBePaused);
+        // A page may subsequently suspend DOM objects, say as part of handling a scroll or zoom gesture, after the
+        // embedding client requested the page be suspended. We ignore such requests so long as the embedding client
+        // requested the suspension first. See <rdar://problem/13754896> for more details.
+        ASSERT(reasonForSuspendingActiveDOMObjects() == ActiveDOMObject::PageWillBeSuspended);
         return;
     }
-#endif
-
-    ASSERT(!m_scheduledTasksAreSuspended);
 
     suspendScriptedAnimationControllerCallbacks();
     suspendActiveDOMObjects(reason);
index 366e16e9fdac03eb531d8254c0a778aa5e77f9a2..a8aa0bfabda747a9919af5d0ed17c5e91dd139b5 100644 (file)
@@ -214,12 +214,13 @@ void ScriptExecutionContext::suspendActiveDOMObjects(ActiveDOMObject::ReasonForS
 {
     checkConsistency();
 
-#if PLATFORM(IOS)
     if (m_activeDOMObjectsAreSuspended) {
-        ASSERT(m_reasonForSuspendingActiveDOMObjects == ActiveDOMObject::DocumentWillBePaused);
+        // A page may subsequently suspend DOM objects, say as part of entering the page cache, after the embedding
+        // client requested the page be suspended. We ignore such requests so long as the embedding client requested
+        // the suspension first. See <rdar://problem/13754896> for more details.
+        ASSERT(m_reasonForSuspendingActiveDOMObjects == ActiveDOMObject::PageWillBeSuspended);
         return;
     }
-#endif
 
     m_activeDOMObjectAdditionForbidden = true;
 #if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
index 146f4bea4eb604405f14c7a919fbb750d7842d79..577ab0c16aee89b3c032e4a104026943cb7a69b2 100644 (file)
@@ -5049,7 +5049,6 @@ void HTMLMediaElement::suspend(ReasonForSuspension why)
             setShouldBufferData(false);
             m_mediaSession->addBehaviorRestriction(MediaElementSession::RequirePageConsentToResumeMedia);
             break;
-        case DocumentWillBePaused:
         case JavaScriptDebuggerPaused:
         case PageWillBeSuspended:
         case WillDeferLoading:
index 585aa7268a9c31d5bbf3c6e3a4f4b1c00a6239d4..dfe038bd717dc08477f626edd9746488af6e2f98 100644 (file)
@@ -202,8 +202,7 @@ int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<Scheduled
     DOMTimer* timer = new DOMTimer(context, WTF::move(action), timeout, singleShot);
 #if PLATFORM(IOS)
     if (is<Document>(context)) {
-        Document& document = downcast<Document>(context);
-        bool didDeferTimeout = document.frame() && document.frame()->timersPaused();
+        bool didDeferTimeout = context.activeDOMObjectsAreSuspended();
         if (!didDeferTimeout && timeout <= 100 && singleShot) {
             WKSetObservedContentChange(WKContentIndeterminateChange);
             WebThreadAddObservedContentModifier(timer); // Will only take affect if not already visibility change.
@@ -297,13 +296,6 @@ void DOMTimer::fired()
 
     DOMTimerFireState fireState(context);
 
-#if PLATFORM(IOS)
-    Document* document = nullptr;
-    if (is<Document>(context)) {
-        document = &downcast<Document>(context);
-        ASSERT(!document->frame()->timersPaused());
-    }
-#endif
     context.setTimerNestingLevel(std::min(m_nestingLevel + 1, maxTimerNestingLevel));
 
     ASSERT(!isSuspended());
@@ -334,7 +326,7 @@ void DOMTimer::fired()
 #if PLATFORM(IOS)
     bool shouldReportLackOfChanges;
     bool shouldBeginObservingChanges;
-    if (document) {
+    if (is<Document>(context)) {
         shouldReportLackOfChanges = WebThreadCountOfObservedContentModifiers() == 1;
         shouldBeginObservingChanges = WebThreadContainsObservedContentModifier(this);
     } else {
@@ -359,9 +351,11 @@ void DOMTimer::fired()
     if (shouldBeginObservingChanges) {
         WKStopObservingContentChanges();
 
-        if (WKObservedContentChange() == WKContentVisibilityChange || shouldReportLackOfChanges)
-            if (document && document->page())
-                document->page()->chrome().client().observedContentChange(document->frame());
+        if (WKObservedContentChange() == WKContentVisibilityChange || shouldReportLackOfChanges) {
+            Document& document = downcast<Document>(context);
+            if (Page* page = document.page())
+                page->chrome().client().observedContentChange(document.frame());
+        }
     }
 #endif
 
index db670110bbb861e3a1bd3d1de7b3341c098943a1..ea5d92e4009462fa8e6bf59cfb33f65d0b41f803 100644 (file)
@@ -165,7 +165,6 @@ Frame::Frame(Page& page, HTMLFrameOwnerElement* ownerElement, FrameLoaderClient&
 #if PLATFORM(IOS)
     , m_overflowAutoScrollTimer(*this, &Frame::overflowAutoScrollTimerFired)
     , m_selectionChangeCallbacksDisabled(false)
-    , m_timersPausedCount(0)
 #endif
     , m_pageZoomFactor(parentPageZoomFactor(this))
     , m_textZoomFactor(parentTextZoomFactor(this))
@@ -192,17 +191,10 @@ Frame::Frame(Page& page, HTMLFrameOwnerElement* ownerElement, FrameLoaderClient&
     frameCounter.increment();
 #endif
 
-    // FIXME: We should reconcile the iOS and OpenSource code below.
-    Frame* parent = parentFromOwnerElement(ownerElement);
-#if PLATFORM(IOS)
-    // Pause future timers if this frame is created when page is in pending state.
-    if (parent && parent->timersPaused())
-        setTimersPaused(true);
-#else
     // Pause future ActiveDOMObjects if this frame is being created while the page is in a paused state.
+    Frame* parent = parentFromOwnerElement(ownerElement);
     if (parent && parent->activeDOMObjectsAndAnimationsSuspended())
         suspendActiveDOMObjectsAndAnimations();
-#endif
 }
 
 Ref<Frame> Frame::create(Page* page, HTMLFrameOwnerElement* ownerElement, FrameLoaderClient* client)
@@ -1012,27 +1004,31 @@ void Frame::suspendActiveDOMObjectsAndAnimations()
         return;
 
     // FIXME: Suspend/resume calls will not match if the frame is navigated, and gets a new document.
-    if (document()) {
-        document()->suspendScriptedAnimationControllerCallbacks();
-        animation().suspendAnimationsForDocument(document());
-        document()->suspendActiveDOMObjects(ActiveDOMObject::PageWillBeSuspended);
-    }
+    clearTimers(); // Suspends animations and pending relayouts.
+    if (m_doc)
+        m_doc->suspendScheduledTasks(ActiveDOMObject::PageWillBeSuspended);
 }
 
 void Frame::resumeActiveDOMObjectsAndAnimations()
 {
-    ASSERT(activeDOMObjectsAndAnimationsSuspended());
+    if (!activeDOMObjectsAndAnimationsSuspended())
+        return;
 
     m_activeDOMObjectsAndAnimationsSuspendedCount--;
 
     if (activeDOMObjectsAndAnimationsSuspended())
         return;
 
-    if (document()) {
-        document()->resumeActiveDOMObjects(ActiveDOMObject::PageWillBeSuspended);
-        animation().resumeAnimationsForDocument(document());
-        document()->resumeScriptedAnimationControllerCallbacks();
-    }
+    if (!m_doc)
+        return;
+
+    // FIXME: Suspend/resume calls will not match if the frame is navigated, and gets a new document.
+    m_doc->resumeScheduledTasks(ActiveDOMObject::PageWillBeSuspended);
+
+    // Frame::clearTimers() suspended animations and pending relayouts.
+    animation().resumeAnimationsForDocument(m_doc.get());
+    if (m_view)
+        m_view->scheduleRelayout();
 }
 
 void Frame::deviceOrPageScaleFactorChanged()
index 04cc95b0decdabafdc6a2c24cbc77c6d5f687761..535346d46bc0b7f40fbf10bbcabfd92d50524207 100644 (file)
@@ -242,8 +242,10 @@ namespace WebCore {
         WEBCORE_EXPORT NSRect rectForScrollToVisible() const;
         WEBCORE_EXPORT DOMCSSStyleDeclaration* styleAtSelectionStart() const;
         WEBCORE_EXPORT unsigned formElementsCharacterCount() const;
+
+        // This function is used by Legacy WebKit.
         WEBCORE_EXPORT void setTimersPaused(bool);
-        bool timersPaused() const { return m_timersPausedCount; }
+
         WEBCORE_EXPORT void dispatchPageHideEventBeforePause();
         WEBCORE_EXPORT void dispatchPageShowEventBeforeResume();
         WEBCORE_EXPORT void setRangedSelectionBaseToCurrentSelection();
@@ -306,7 +308,6 @@ namespace WebCore {
         IntPoint m_overflowAutoScrollPos;
         ViewportArguments m_viewportArguments;
         bool m_selectionChangeCallbacksDisabled;
-        int m_timersPausedCount;
         VisibleSelection m_rangedSelectionBase;
         VisibleSelection m_rangedSelectionInitialExtent;
 #endif
index 3565b364d77ab55b428f6433f45cf9251c5b2fed..34d9e0d937ae66fbbaf57a1fe23c9e0e1ad99edf 100644 (file)
@@ -623,37 +623,13 @@ unsigned Frame::formElementsCharacterCount() const
 
 void Frame::setTimersPaused(bool paused)
 {
+    if (!m_page)
+        return;
     JSLockHolder lock(JSDOMWindowBase::commonVM());
-    setTimersPausedInternal(paused);
-}
-
-void Frame::setTimersPausedInternal(bool paused)
-{
-    if (paused) {
-        ++m_timersPausedCount;
-        if (m_timersPausedCount == 1) {
-            clearTimers();
-            if (document())
-                document()->suspendScheduledTasks(ActiveDOMObject::DocumentWillBePaused);
-        }
-    } else {
-        --m_timersPausedCount;
-        ASSERT(m_timersPausedCount >= 0);
-        if (m_timersPausedCount == 0) {
-            if (document())
-                document()->resumeScheduledTasks(ActiveDOMObject::DocumentWillBePaused);
-
-            // clearTimers() suspended animations and pending relayouts, reschedule if needed.
-            animation().resumeAnimationsForDocument(document());
-
-            if (view())
-                view()->scheduleRelayout();
-        }
-    }
-
-    // We need to make sure all subframes' states are up to date.
-    for (Frame* frame = tree().firstChild(); frame; frame = frame->tree().nextSibling())
-        frame->setTimersPausedInternal(paused);
+    if (paused)
+        m_page->suspendActiveDOMObjectsAndAnimations();
+    else
+        m_page->resumeActiveDOMObjectsAndAnimations();
 }
 
 void Frame::dispatchPageHideEventBeforePause()
index 5d882b81f4c2568984133b388fcc30f48d578183..a26efb63e71d4be00885519a80ffcb649cc52d73 100644 (file)
@@ -1435,10 +1435,6 @@ static bool shouldRepaintForImageAnimation(const RenderElement& renderer, const
     const Document& document = renderer.document();
     if (document.inPageCache())
         return false;
-#if PLATFORM(IOS)
-    if (document.frame()->timersPaused())
-        return false;
-#endif
     if (document.activeDOMObjectsAreSuspended())
         return false;
     if (renderer.style().visibility() != VISIBLE)