Iteratively dispatch DOM events after restoring a cached page
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Mar 2017 22:44:59 +0000 (22:44 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 15 Mar 2017 22:44:59 +0000 (22:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169703
<rdar://problem/31075903>

Reviewed by Brady Eidson.

Make dispatching of DOM events when restoring a page from the page cache symmetric with
dispatching of events when saving a page to the page cache.

* history/CachedFrame.cpp:
(WebCore::CachedFrameBase::restore): Move code to dispatch events from here to FrameLoader::didRestoreFromCachedPage().
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::commitProvisionalLoad): Ensure that no DOM events are dispatched during
restoration of a cached page. Call didRestoreFromCachedPage() after restoring the page to
dispatch DOM events on the restored frames.
(WebCore::FrameLoader::willRestoreFromCachedPage): Renamed; formerly named prepareForCachedPageRestore().
(WebCore::FrameLoader::didRestoreFromCachedPage): Added.
(WebCore::FrameLoader::prepareForCachedPageRestore): Renamed to willRestoreFromCachedPage().
* loader/FrameLoader.h:
* page/FrameTree.cpp:
(WebCore::FrameTree::traverseNextInPostOrderWithWrap): Returns the next Frame* in a post-order
traversal of the frame tree optionally wrapping around to the deepest first child in the tree.
(WebCore::FrameTree::deepFirstChild): Added.
* page/FrameTree.h:

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

Source/WebCore/ChangeLog
Source/WebCore/history/CachedFrame.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/page/FrameTree.cpp
Source/WebCore/page/FrameTree.h

index 949751c..583c32b 100644 (file)
@@ -1,3 +1,30 @@
+2017-03-15  Daniel Bates  <dabates@apple.com>
+
+        Iteratively dispatch DOM events after restoring a cached page
+        https://bugs.webkit.org/show_bug.cgi?id=169703
+        <rdar://problem/31075903>
+
+        Reviewed by Brady Eidson.
+
+        Make dispatching of DOM events when restoring a page from the page cache symmetric with
+        dispatching of events when saving a page to the page cache.
+
+        * history/CachedFrame.cpp:
+        (WebCore::CachedFrameBase::restore): Move code to dispatch events from here to FrameLoader::didRestoreFromCachedPage().
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::commitProvisionalLoad): Ensure that no DOM events are dispatched during
+        restoration of a cached page. Call didRestoreFromCachedPage() after restoring the page to
+        dispatch DOM events on the restored frames.
+        (WebCore::FrameLoader::willRestoreFromCachedPage): Renamed; formerly named prepareForCachedPageRestore().
+        (WebCore::FrameLoader::didRestoreFromCachedPage): Added.
+        (WebCore::FrameLoader::prepareForCachedPageRestore): Renamed to willRestoreFromCachedPage().
+        * loader/FrameLoader.h:
+        * page/FrameTree.cpp:
+        (WebCore::FrameTree::traverseNextInPostOrderWithWrap): Returns the next Frame* in a post-order
+        traversal of the frame tree optionally wrapping around to the deepest first child in the tree.
+        (WebCore::FrameTree::deepFirstChild): Added.
+        * page/FrameTree.h:
+
 2017-03-15  Dave Hyatt  <hyatt@apple.com>
 
         Positioned SVG not sized correctly
index 22245fc..2102852 100644 (file)
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
 #include "FrameView.h"
-#include "HistoryController.h"
-#include "HistoryItem.h"
 #include "Logging.h"
 #include "MainFrame.h"
 #include "Page.h"
 #include "PageCache.h"
-#include "PageTransitionEvent.h"
 #include "SVGDocumentExtensions.h"
 #include "ScriptController.h"
 #include "SerializedScriptValue.h"
@@ -116,6 +113,7 @@ void CachedFrameBase::restore()
         ASSERT(childFrame->view()->frame().page());
         frame.tree().appendChild(childFrame->view()->frame());
         childFrame->open();
+        ASSERT_WITH_SECURITY_IMPLICATION(m_document == frame.document());
     }
 
 #if PLATFORM(IOS)
@@ -131,14 +129,6 @@ void CachedFrameBase::restore()
     }
 #endif
 
-    // FIXME: update Page Visibility state here.
-    // https://bugs.webkit.org/show_bug.cgi?id=116770
-    m_document->enqueuePageshowEvent(PageshowEventPersisted);
-
-    HistoryItem* historyItem = frame.loader().history().currentItem();
-    if (historyItem && historyItem->stateObject())
-        m_document->enqueuePopstateEvent(historyItem->stateObject());
-
     frame.view()->didRestoreFromPageCache();
 }
 
index 2345c55..e2ed03d 100644 (file)
@@ -86,6 +86,7 @@
 #include "MainFrame.h"
 #include "MemoryCache.h"
 #include "MemoryRelease.h"
+#include "NoEventDispatchAssertion.h"
 #include "Page.h"
 #include "PageCache.h"
 #include "PageTransitionEvent.h"
@@ -1813,7 +1814,7 @@ void FrameLoader::commitProvisionalLoad()
         // commit to happen before any changes to viewport arguments and dealing with this there is difficult.
         m_frame.page()->chrome().setDispatchViewportDataDidChangeSuppressed(true);
 #endif
-        prepareForCachedPageRestore();
+        willRestoreFromCachedPage();
 
         // Start request for the main resource and dispatch didReceiveResponse before the load is committed for
         // consistency with all other loads. See https://bugs.webkit.org/show_bug.cgi?id=150927.
@@ -1825,10 +1826,19 @@ void FrameLoader::commitProvisionalLoad()
 
         std::optional<HasInsecureContent> hasInsecureContent = cachedPage->cachedMainFrame()->hasInsecureContent();
 
-        // FIXME: This API should be turned around so that we ground CachedPage into the Page.
-        cachedPage->restore(*m_frame.page());
+        {
+            // Do not dispatch DOM events as their JavaScript listeners could cause the page to be put
+            // into the page cache before we have finished restoring it from the page cache.
+            NoEventDispatchAssertion assertNoEventDispatch;
+
+            // FIXME: This API should be turned around so that we ground CachedPage into the Page.
+            cachedPage->restore(*m_frame.page());
+        }
 
         dispatchDidCommitLoad(hasInsecureContent);
+
+        didRestoreFromCachedPage();
+
 #if PLATFORM(IOS)
         m_frame.page()->chrome().setDispatchViewportDataDidChangeSuppressed(false);
         m_frame.page()->chrome().dispatchViewportPropertiesDidChange(m_frame.page()->viewportArguments());
@@ -2047,7 +2057,7 @@ void FrameLoader::closeOldDataSources()
     m_client.setMainFrameDocumentReady(false); // stop giving out the actual DOMDocument to observers
 }
 
-void FrameLoader::prepareForCachedPageRestore()
+void FrameLoader::willRestoreFromCachedPage()
 {
     ASSERT(!m_frame.tree().parent());
     ASSERT(m_frame.page());
@@ -2066,6 +2076,31 @@ void FrameLoader::prepareForCachedPageRestore()
     }
 }
 
+void FrameLoader::didRestoreFromCachedPage()
+{
+    // Dispatching JavaScript events can cause frame destruction.
+    auto& mainFrame = m_frame.page()->mainFrame();
+    Vector<Ref<Frame>> childFrames;
+    for (auto* child = mainFrame.tree().traverseNextInPostOrderWithWrap(true); child; child = child->tree().traverseNextInPostOrderWithWrap(false))
+        childFrames.append(*child);
+
+    for (auto& child : childFrames) {
+        if (!child->tree().isDescendantOf(&mainFrame))
+            continue;
+        auto* document = child->document();
+        if (!document)
+            continue;
+
+        // FIXME: Update Page Visibility state here.
+        // https://bugs.webkit.org/show_bug.cgi?id=116770
+        document->enqueuePageshowEvent(PageshowEventPersisted);
+
+        auto* historyItem = child->loader().history().currentItem();
+        if (historyItem && historyItem->stateObject())
+            document->enqueuePopstateEvent(historyItem->stateObject());
+    }
+}
+
 void FrameLoader::open(CachedFrameBase& cachedFrame)
 {
     m_isComplete = false;
index 6ad12c8..d1b954a 100644 (file)
@@ -345,7 +345,8 @@ private:
     void setState(FrameState);
 
     void closeOldDataSources();
-    void prepareForCachedPageRestore();
+    void willRestoreFromCachedPage();
+    void didRestoreFromCachedPage();
 
     bool shouldReloadToHandleUnreachableURL(DocumentLoader*);
 
index fdcf356..4e1bc29 100644 (file)
@@ -431,6 +431,25 @@ Frame* FrameTree::traversePreviousWithWrap(bool wrap) const
     return nullptr;
 }
 
+Frame* FrameTree::traverseNextInPostOrderWithWrap(bool wrap) const
+{
+    if (m_nextSibling)
+        return m_nextSibling->tree().deepFirstChild();
+    if (m_parent)
+        return m_parent;
+    if (wrap)
+        return deepFirstChild();
+    return nullptr;
+}
+
+Frame* FrameTree::deepFirstChild() const
+{
+    Frame* result = &m_thisFrame;
+    while (auto* next = result->tree().firstChild())
+        result = next;
+    return result;
+}
+
 Frame* FrameTree::deepLastChild() const
 {
     Frame* result = &m_thisFrame;
index 3f87c27..052f360 100644 (file)
@@ -64,7 +64,9 @@ public:
     WEBCORE_EXPORT Frame* traverseNextRendered(const Frame* stayWithin = nullptr) const;
     WEBCORE_EXPORT Frame* traverseNextWithWrap(bool) const;
     WEBCORE_EXPORT Frame* traversePreviousWithWrap(bool) const;
-    
+
+    Frame* traverseNextInPostOrderWithWrap(bool) const;
+
     WEBCORE_EXPORT void appendChild(Frame&);
     void detachFromParent() { m_parent = nullptr; }
     void removeChild(Frame&);
@@ -85,6 +87,7 @@ public:
     unsigned indexInParent() const;
 
 private:
+    Frame* deepFirstChild() const;
     Frame* deepLastChild() const;
 
     bool scopedBy(TreeScope*) const;