Title: [193884] trunk/Source/WebCore
Revision
193884
Author
dba...@webkit.org
Date
2015-12-09 18:51:11 -0800 (Wed, 09 Dec 2015)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (193883 => 193884)


--- trunk/Source/WebCore/ChangeLog	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/ChangeLog	2015-12-10 02:51:11 UTC (rev 193884)
@@ -1,3 +1,60 @@
+2015-12-09  Daniel Bates  <daba...@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  <beid...@apple.com>
 
         Modern IDB: storage/indexeddb/metadata.html fails

Modified: trunk/Source/WebCore/dom/ActiveDOMObject.h (193883 => 193884)


--- trunk/Source/WebCore/dom/ActiveDOMObject.h	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/dom/ActiveDOMObject.h	2015-12-10 02:51:11 UTC (rev 193884)
@@ -55,7 +55,6 @@
         WillDeferLoading,
         PageCache,
         PageWillBeSuspended,
-        DocumentWillBePaused
     };
 
     virtual const char* activeDOMObjectName() const = 0;

Modified: trunk/Source/WebCore/dom/Document.cpp (193883 => 193884)


--- trunk/Source/WebCore/dom/Document.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/dom/Document.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -2207,21 +2207,15 @@
         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,15 +5261,14 @@
 
 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);
     scriptRunner()->suspend();

Modified: trunk/Source/WebCore/dom/ScriptExecutionContext.cpp (193883 => 193884)


--- trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/dom/ScriptExecutionContext.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -214,12 +214,13 @@
 {
     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)

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (193883 => 193884)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -5049,7 +5049,6 @@
             setShouldBufferData(false);
             m_mediaSession->addBehaviorRestriction(MediaElementSession::RequirePageConsentToResumeMedia);
             break;
-        case DocumentWillBePaused:
         case _javascript_DebuggerPaused:
         case PageWillBeSuspended:
         case WillDeferLoading:

Modified: trunk/Source/WebCore/page/DOMTimer.cpp (193883 => 193884)


--- trunk/Source/WebCore/page/DOMTimer.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/page/DOMTimer.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -202,8 +202,7 @@
     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 @@
 
     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 @@
 #if PLATFORM(IOS)
     bool shouldReportLackOfChanges;
     bool shouldBeginObservingChanges;
-    if (document) {
+    if (is<Document>(context)) {
         shouldReportLackOfChanges = WebThreadCountOfObservedContentModifiers() == 1;
         shouldBeginObservingChanges = WebThreadContainsObservedContentModifier(this);
     } else {
@@ -359,9 +351,11 @@
     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
 

Modified: trunk/Source/WebCore/page/Frame.cpp (193883 => 193884)


--- trunk/Source/WebCore/page/Frame.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/page/Frame.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -165,7 +165,6 @@
 #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 @@
     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 @@
         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()

Modified: trunk/Source/WebCore/page/Frame.h (193883 => 193884)


--- trunk/Source/WebCore/page/Frame.h	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/page/Frame.h	2015-12-10 02:51:11 UTC (rev 193884)
@@ -242,8 +242,10 @@
         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 @@
         IntPoint m_overflowAutoScrollPos;
         ViewportArguments m_viewportArguments;
         bool m_selectionChangeCallbacksDisabled;
-        int m_timersPausedCount;
         VisibleSelection m_rangedSelectionBase;
         VisibleSelection m_rangedSelectionInitialExtent;
 #endif

Modified: trunk/Source/WebCore/page/ios/FrameIOS.mm (193883 => 193884)


--- trunk/Source/WebCore/page/ios/FrameIOS.mm	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/page/ios/FrameIOS.mm	2015-12-10 02:51:11 UTC (rev 193884)
@@ -623,39 +623,15 @@
 
 void Frame::setTimersPaused(bool paused)
 {
+    if (!m_page)
+        return;
     JSLockHolder lock(JSDOMWindowBase::commonVM());
-    setTimersPausedInternal(paused);
+    if (paused)
+        m_page->suspendActiveDOMObjectsAndAnimations();
+    else
+        m_page->resumeActiveDOMObjectsAndAnimations();
 }
 
-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);
-}
-
 void Frame::dispatchPageHideEventBeforePause()
 {
     ASSERT(isMainFrame());

Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (193883 => 193884)


--- trunk/Source/WebCore/rendering/RenderElement.cpp	2015-12-10 02:50:31 UTC (rev 193883)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp	2015-12-10 02:51:11 UTC (rev 193884)
@@ -1435,10 +1435,6 @@
     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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to