Changeset 195790 in webkit

Timestamp:

Jan 28, 2016 3:30:45 PM (8 years ago)

Author:

Antti Koivisto

Message:

Tab suspension code hits asserts

Reviewed by Chris Dumez.

Enabling tab suspension and navigating around in a few tabs hits an assert in
ScriptExecutionContext::suspendActiveDOMObject. This is because suspend/resume reasons don't pair properly

• dom/Document.cpp:

(WebCore::Document::documentWillBecomeInactive):
(WebCore::Document::suspend):
(WebCore::Document::resume):

Provide the reason as argument.

• dom/Document.h:
• history/CachedFrame.cpp:

(WebCore::CachedFrameBase::restore):

No need to call resumeActiveDOMObjects/resumeScriptedAnimationControllerCallbacks explicitly as Document::resume does that.

(WebCore::CachedFrame::CachedFrame):

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::commitProvisionalLoad):

• page/Page.cpp:

(WebCore::Page::canTabSuspend):
(WebCore::Page::setIsTabSuspended):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (4 diffs)
• dom/Document.h (modified) (1 diff)
• history/CachedFrame.cpp (modified) (3 diffs)
• loader/FrameLoader.cpp (modified) (1 diff)
• page/Page.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-01-28  Antti Koivisto  <antti@apple.com>
+
+        Tab suspension code hits asserts
+             
+
+        Reviewed by Chris Dumez.
+
+        Enabling tab suspension and navigating around in a few tabs hits an assert in
+        ScriptExecutionContext::suspendActiveDOMObject. This is because suspend/resume reasons don't pair properly
+
+        * dom/Document.cpp:
+        (WebCore::Document::documentWillBecomeInactive):
+        (WebCore::Document::suspend):
+        (WebCore::Document::resume):
+
+            Provide the reason as argument.
+
+        * dom/Document.h:
+        * history/CachedFrame.cpp:
+        (WebCore::CachedFrameBase::restore):
+
+            No need to call resumeActiveDOMObjects/resumeScriptedAnimationControllerCallbacks explicitly as Document::resume does that.
+
+        (WebCore::CachedFrame::CachedFrame):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::commitProvisionalLoad):
+        * page/Page.cpp:
+        (WebCore::Page::canTabSuspend):
+        (WebCore::Page::setIsTabSuspended):
+
 2016-01-28  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -4656 +4656 @@
 }
 
-void Document::suspend()
+void Document::suspend(ActiveDOMObject::ReasonForSuspension reason)
 {
     if (m_isSuspended)
@@ -4681 +4681 @@
 
     suspendScriptedAnimationControllerCallbacks();
-    suspendActiveDOMObjects(ActiveDOMObject::PageCache);
+    suspendActiveDOMObjects(reason);
 
     ASSERT(m_frame);
@@ -4692 +4692 @@
 }
 
-void Document::resume()
+void Document::resume(ActiveDOMObject::ReasonForSuspension reason)
 {
     if (!m_isSuspended)
@@ -4712 +4712 @@
     m_frame->animation().resumeAnimationsForDocument(this);
 
-    resumeActiveDOMObjects(ActiveDOMObject::PageWillBeSuspended);
+    resumeActiveDOMObjects(reason);
     resumeScriptedAnimationControllerCallbacks();
 

trunk/Source/WebCore/dom/Document.h

@@ -1012 +1012 @@
 
     void documentWillBecomeInactive();
-    void suspend();
-    void resume();
+    void suspend(ActiveDOMObject::ReasonForSuspension);
+    void resume(ActiveDOMObject::ReasonForSuspension);
 
     void registerForMediaVolumeCallbacks(Element*);

trunk/Source/WebCore/history/CachedFrame.cpp

@@ -93 +93 @@
 
     frame.animation().resumeAnimationsForDocument(m_document.get());
-    m_document->resumeActiveDOMObjects(ActiveDOMObject::PageCache);
-    m_document->resumeScriptedAnimationControllerCallbacks();
+
+    m_document->resume(ActiveDOMObject::PageCache);
 
     // It is necessary to update any platform script objects after restoring the
@@ -136 +136 @@
 #endif
 
-    m_document->resume();
 }
 
@@ -162 +161 @@
 
     // Active DOM objects must be suspended before we cache the frame script data.
-    m_document->suspend();
+    m_document->suspend(ActiveDOMObject::PageCache);
 
     m_cachedFrameScriptData = std::make_unique<ScriptCachedFrameData>(frame);

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1846 +1846 @@
             m_frame.page()->chrome().didReceiveDocType(&m_frame);
 #endif
-        m_frame.document()->resume();
+        m_frame.document()->resume(ActiveDOMObject::PageCache);
 
         // Force a layout to update view size and thereby update scrollbars.

trunk/Source/WebCore/page/Page.cpp

@@ -1864 +1864 @@
 bool Page::canTabSuspend()
 {
-    return s_tabSuspensionIsEnabled && !m_isPrerender && (m_pageThrottler.activityState() == PageActivityState::NoFlags) && PageCache::singleton().canCache(*this);
+    if (!s_tabSuspensionIsEnabled)
+        return false;
+    if (m_isPrerender)
+        return false;
+    if (m_pageThrottler.activityState() != PageActivityState::NoFlags)
+        return false;
+    // FIXME: PageCache::canCache does a bunch of checks that are not needed for the tab suspension case. There should be a specific check.
+    if (!PageCache::singleton().canCache(*this))
+        return false;
+
+    return true;
 }
 
@@ -1872 +1882 @@
         if (auto* document = frame->document()) {
             if (shouldSuspend)
-                document->suspend();
+                document->suspend(ActiveDOMObject::PageWillBeSuspended);
             else
-                document->resume();
+                document->resume(ActiveDOMObject::PageWillBeSuspended);
         }
     }