- Revision
- 214747
- Author
- carlo...@webkit.org
- Date
- 2017-04-03 00:46:26 -0700 (Mon, 03 Apr 2017)
Log Message
Merge r214392 - media/restore-from-page-cache.html causes NoEventDispatchAssertion::isEventAllowedInMainThread() assertion failure
https://bugs.webkit.org/show_bug.cgi?id=170087
<rdar://problem/31254822>
Reviewed by Simon Fraser.
Reduce the scope of code that should never dispatch DOM events so as to allow updating contents size
after restoring a page from the page cache.
In r214014 we instantiate a NoEventDispatchAssertion in FrameLoader::commitProvisionalLoad()
around the call to CachedPage::restore() to assert when a DOM event is dispatched during
page restoration as such events can cause re-entrancy into the page cache. As it turns out
it is sufficient to ensure that no DOM events are dispatched after restoring all cached frames
as opposed to after CachedPage::restore() returns.
Also rename Document::enqueue{Pageshow, Popstate}Event() to dispatch{Pageshow, Popstate}Event(),
respectively, since they synchronously dispatch events :(. We hope in the future to make them
asynchronously dispatch events.
* dom/Document.cpp:
(WebCore::Document::implicitClose): Update for renaming.
(WebCore::Document::statePopped): Ditto.
(WebCore::Document::dispatchPageshowEvent): Renamed; formerly named enqueuePageshowEvent().
(WebCore::Document::dispatchPopstateEvent): Renamed; formerly named enqueuePopstateEvent().
(WebCore::Document::enqueuePageshowEvent): Deleted.
(WebCore::Document::enqueuePopstateEvent): Deleted.
* dom/Document.h:
* history/CachedPage.cpp:
(WebCore::firePageShowAndPopStateEvents): Moved logic from FrameLoader::didRestoreFromCachedPage() to here.
(WebCore::CachedPage::restore): Modified to call firePageShowAndPopStateEvents().
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::commitProvisionalLoad): Removed use of NoEventDispatchAssertion RAII object. We
will instantiate it in CachedPage::restore() with a smaller scope.
(WebCore::FrameLoader::didRestoreFromCachedPage): Deleted; moved logic from here to WebCore::firePageShowAndPopStateEvents().
* loader/FrameLoader.h:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-04-03 07:46:26 UTC (rev 214747)
@@ -1,3 +1,41 @@
+2017-03-24 Daniel Bates <daba...@apple.com>
+
+ media/restore-from-page-cache.html causes NoEventDispatchAssertion::isEventAllowedInMainThread() assertion failure
+ https://bugs.webkit.org/show_bug.cgi?id=170087
+ <rdar://problem/31254822>
+
+ Reviewed by Simon Fraser.
+
+ Reduce the scope of code that should never dispatch DOM events so as to allow updating contents size
+ after restoring a page from the page cache.
+
+ In r214014 we instantiate a NoEventDispatchAssertion in FrameLoader::commitProvisionalLoad()
+ around the call to CachedPage::restore() to assert when a DOM event is dispatched during
+ page restoration as such events can cause re-entrancy into the page cache. As it turns out
+ it is sufficient to ensure that no DOM events are dispatched after restoring all cached frames
+ as opposed to after CachedPage::restore() returns.
+
+ Also rename Document::enqueue{Pageshow, Popstate}Event() to dispatch{Pageshow, Popstate}Event(),
+ respectively, since they synchronously dispatch events :(. We hope in the future to make them
+ asynchronously dispatch events.
+
+ * dom/Document.cpp:
+ (WebCore::Document::implicitClose): Update for renaming.
+ (WebCore::Document::statePopped): Ditto.
+ (WebCore::Document::dispatchPageshowEvent): Renamed; formerly named enqueuePageshowEvent().
+ (WebCore::Document::dispatchPopstateEvent): Renamed; formerly named enqueuePopstateEvent().
+ (WebCore::Document::enqueuePageshowEvent): Deleted.
+ (WebCore::Document::enqueuePopstateEvent): Deleted.
+ * dom/Document.h:
+ * history/CachedPage.cpp:
+ (WebCore::firePageShowAndPopStateEvents): Moved logic from FrameLoader::didRestoreFromCachedPage() to here.
+ (WebCore::CachedPage::restore): Modified to call firePageShowAndPopStateEvents().
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::commitProvisionalLoad): Removed use of NoEventDispatchAssertion RAII object. We
+ will instantiate it in CachedPage::restore() with a smaller scope.
+ (WebCore::FrameLoader::didRestoreFromCachedPage): Deleted; moved logic from here to WebCore::firePageShowAndPopStateEvents().
+ * loader/FrameLoader.h:
+
2017-03-15 Daniel Bates <daba...@apple.com>
Iteratively dispatch DOM events after restoring a cached page
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.cpp 2017-04-03 07:46:26 UTC (rev 214747)
@@ -2683,9 +2683,9 @@
accessSVGExtensions().dispatchSVGLoadEventToOutermostSVGElements();
dispatchWindowLoadEvent();
- enqueuePageshowEvent(PageshowEventNotPersisted);
+ dispatchPageshowEvent(PageshowEventNotPersisted);
if (m_pendingStateObject)
- enqueuePopstateEvent(WTFMove(m_pendingStateObject));
+ dispatchPopstateEvent(WTFMove(m_pendingStateObject));
if (f)
f->loader().dispatchOnloadEvents();
@@ -5272,7 +5272,7 @@
// Per step 11 of section 6.5.9 (history traversal) of the HTML5 spec, we
// defer firing of popstate until we're in the complete state.
if (m_readyState == Complete)
- enqueuePopstateEvent(WTFMove(stateObject));
+ dispatchPopstateEvent(WTFMove(stateObject));
else
m_pendingStateObject = WTFMove(stateObject);
}
@@ -5503,7 +5503,7 @@
return String { string }.replace('\\', m_decoder->encoding().backslashAsCurrencySymbol());
}
-void Document::enqueuePageshowEvent(PageshowEventPersistence persisted)
+void Document::dispatchPageshowEvent(PageshowEventPersistence persisted)
{
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=36334 Pageshow event needs to fire asynchronously.
dispatchWindowEvent(PageTransitionEvent::create(eventNames().pageshowEvent, persisted), this);
@@ -5514,7 +5514,7 @@
enqueueWindowEvent(HashChangeEvent::create(oldURL, newURL));
}
-void Document::enqueuePopstateEvent(RefPtr<SerializedScriptValue>&& stateObject)
+void Document::dispatchPopstateEvent(RefPtr<SerializedScriptValue>&& stateObject)
{
dispatchWindowEvent(PopStateEvent::create(WTFMove(stateObject), m_domWindow ? m_domWindow->history() : nullptr));
}
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/Document.h 2017-04-03 07:46:26 UTC (rev 214747)
@@ -1059,9 +1059,9 @@
void enqueueWindowEvent(Ref<Event>&&);
void enqueueDocumentEvent(Ref<Event>&&);
void enqueueOverflowEvent(Ref<Event>&&);
- void enqueuePageshowEvent(PageshowEventPersistence);
+ void dispatchPageshowEvent(PageshowEventPersistence);
void enqueueHashchangeEvent(const String& oldURL, const String& newURL);
- void enqueuePopstateEvent(RefPtr<SerializedScriptValue>&& stateObject);
+ void dispatchPopstateEvent(RefPtr<SerializedScriptValue>&& stateObject);
DocumentEventQueue& eventQueue() const final { return m_eventQueue; }
WEBCORE_EXPORT void addMediaCanStartListener(MediaCanStartListener*);
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/history/CachedPage.cpp (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/history/CachedPage.cpp 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/history/CachedPage.cpp 2017-04-03 07:46:26 UTC (rev 214747)
@@ -30,9 +30,13 @@
#include "Element.h"
#include "FocusController.h"
#include "FrameView.h"
+#include "HistoryController.h"
+#include "HistoryItem.h"
#include "MainFrame.h"
+#include "NoEventDispatchAssertion.h"
#include "Node.h"
#include "Page.h"
+#include "PageTransitionEvent.h"
#include "Settings.h"
#include "VisitedLinkState.h"
#include <wtf/CurrentTime.h>
@@ -69,6 +73,31 @@
m_cachedMainFrame->destroy();
}
+static void firePageShowAndPopStateEvents(Page& page)
+{
+ // Dispatching _javascript_ events can cause frame destruction.
+ auto& mainFrame = 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->dispatchPageshowEvent(PageshowEventPersisted);
+
+ auto* historyItem = child->loader().history().currentItem();
+ if (historyItem && historyItem->stateObject())
+ document->dispatchPopstateEvent(historyItem->stateObject());
+ }
+}
+
void CachedPage::restore(Page& page)
{
ASSERT(m_cachedMainFrame);
@@ -75,7 +104,13 @@
ASSERT(m_cachedMainFrame->view()->frame().isMainFrame());
ASSERT(!page.subframeCount());
- m_cachedMainFrame->open();
+ {
+ // 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 noEventDispatchAssertion;
+
+ m_cachedMainFrame->open();
+ }
// Restore the focus appearance for the focused element.
// FIXME: Right now we don't support pages w/ frames in the b/f cache. This may need to be tweaked when we add support for that.
@@ -116,6 +151,8 @@
frameView->updateContentsSize();
}
+ firePageShowAndPopStateEvents(page);
+
clear();
}
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-04-03 07:46:26 UTC (rev 214747)
@@ -86,7 +86,6 @@
#include "MainFrame.h"
#include "MemoryCache.h"
#include "MemoryRelease.h"
-#include "NoEventDispatchAssertion.h"
#include "Page.h"
#include "PageCache.h"
#include "PageTransitionEvent.h"
@@ -1826,19 +1825,11 @@
std::optional<HasInsecureContent> hasInsecureContent = cachedPage->cachedMainFrame()->hasInsecureContent();
- {
- // 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());
- // 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());
@@ -2076,31 +2067,6 @@
}
}
-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;
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.h (214746 => 214747)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.h 2017-04-03 07:46:13 UTC (rev 214746)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.h 2017-04-03 07:46:26 UTC (rev 214747)
@@ -346,7 +346,6 @@
void closeOldDataSources();
void willRestoreFromCachedPage();
- void didRestoreFromCachedPage();
bool shouldReloadToHandleUnreachableURL(DocumentLoader*);
