Prevent new loads while in PageCache (or being added to PageCache)
https://bugs.webkit.org/show_bug.cgi?id=146299
<rdar://problem/21523788>
Reviewed by Darin Adler.
Generalize the change in r185337 to prevent new loads while in the
PageCache (or being added to the PageCache), instead of merely
preventing new loads in pagehide event handlers. We should never
have any pages that are still loading inside the PageCache.
The fix in r185337 was apparently insufficient to address the
problem so generalizing the check / policy will hopefully catch
more cases where content is able to start loads while being added
to the PageCache. This patch also removes some of the complexity
added in r185337 as it is no longer needed.
No new tests, already covered by:
http/tests/navigation/image-load-in-pagehide-handler.html
http/tests/navigation/subframe-pagehide-handler-starts-load.html
http/tests/navigation/subframe-pagehide-handler-starts-load2.html
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::stopLoading):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::stopAllLoaders):
(WebCore::FrameLoader::handleBeforeUnloadEvent):
(WebCore::FrameLoader::FrameLoader): Deleted.
* loader/FrameLoader.h:
(WebCore::FrameLoader::pageDismissalEventBeingDispatched):
* loader/ImageLoader.cpp:
(WebCore::pageIsBeingDismissed):
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::load):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestImage):
* page/Page.cpp:
(WebCore::Page::inPageCache):
* page/Page.h:
(WebCore::Page::group): Deleted.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@186005 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 7294723..9b994c2 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,47 @@
+2015-06-26 Chris Dumez <cdumez@apple.com>
+
+ Prevent new loads while in PageCache (or being added to PageCache)
+ https://bugs.webkit.org/show_bug.cgi?id=146299
+ <rdar://problem/21523788>
+
+ Reviewed by Darin Adler.
+
+ Generalize the change in r185337 to prevent new loads while in the
+ PageCache (or being added to the PageCache), instead of merely
+ preventing new loads in pagehide event handlers. We should never
+ have any pages that are still loading inside the PageCache.
+
+ The fix in r185337 was apparently insufficient to address the
+ problem so generalizing the check / policy will hopefully catch
+ more cases where content is able to start loads while being added
+ to the PageCache. This patch also removes some of the complexity
+ added in r185337 as it is no longer needed.
+
+ No new tests, already covered by:
+ http/tests/navigation/image-load-in-pagehide-handler.html
+ http/tests/navigation/subframe-pagehide-handler-starts-load.html
+ http/tests/navigation/subframe-pagehide-handler-starts-load2.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::stopLoading):
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::stopAllLoaders):
+ (WebCore::FrameLoader::handleBeforeUnloadEvent):
+ (WebCore::FrameLoader::FrameLoader): Deleted.
+ * loader/FrameLoader.h:
+ (WebCore::FrameLoader::pageDismissalEventBeingDispatched):
+ * loader/ImageLoader.cpp:
+ (WebCore::pageIsBeingDismissed):
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::load):
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::requestImage):
+ * page/Page.cpp:
+ (WebCore::Page::inPageCache):
+ * page/Page.h:
+ (WebCore::Page::group): Deleted.
+
2015-06-26 Simon Fraser <simon.fraser@apple.com>
[OS X] Change the layer tiling threshold from 2000 to 2048 pixels
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 2ef9500..1e49557 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -225,7 +225,6 @@
, m_isExecutingJavaScriptFormAction(false)
, m_didCallImplicitClose(true)
, m_wasUnloadEventEmitted(false)
- , m_pageDismissalEventBeingDispatched(frame)
, m_isComplete(false)
, m_needsClear(false)
, m_checkTimer(*this, &FrameLoader::checkTimerFired)
@@ -424,9 +423,9 @@
Element* currentFocusedElement = m_frame.document()->focusedElement();
if (currentFocusedElement && currentFocusedElement->toInputElement())
currentFocusedElement->toInputElement()->endEditing();
- if (m_pageDismissalEventBeingDispatched == Page::DismissalType::None) {
+ if (m_pageDismissalEventBeingDispatched == PageDismissalType::None) {
if (unloadEventPolicy == UnloadEventPolicyUnloadAndPageHide) {
- m_pageDismissalEventBeingDispatched = Page::DismissalType::PageHide;
+ m_pageDismissalEventBeingDispatched = PageDismissalType::PageHide;
m_frame.document()->domWindow()->dispatchEvent(PageTransitionEvent::create(eventNames().pagehideEvent, m_frame.document()->inPageCache()), m_frame.document());
}
@@ -439,7 +438,7 @@
// while dispatching the event, so protect it to prevent writing the end
// time into freed memory.
RefPtr<DocumentLoader> documentLoader = m_provisionalDocumentLoader;
- m_pageDismissalEventBeingDispatched = Page::DismissalType::Unload;
+ m_pageDismissalEventBeingDispatched = PageDismissalType::Unload;
if (documentLoader && !documentLoader->timing().unloadEventStart() && !documentLoader->timing().unloadEventEnd()) {
DocumentLoadTiming& timing = documentLoader->timing();
ASSERT(timing.navigationStart());
@@ -450,7 +449,7 @@
m_frame.document()->domWindow()->dispatchEvent(unloadEvent, m_frame.document());
}
}
- m_pageDismissalEventBeingDispatched = Page::DismissalType::None;
+ m_pageDismissalEventBeingDispatched = PageDismissalType::None;
if (m_frame.document())
m_frame.document()->updateStyleIfNeeded();
m_wasUnloadEventEmitted = true;
@@ -1226,7 +1225,7 @@
return;
}
- if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
+ if (m_pageDismissalEventBeingDispatched != PageDismissalType::None)
return;
NavigationAction action(request, newLoadType, isFormSubmission, event, frameLoadRequest.shouldOpenExternalURLsPolicy());
@@ -1417,7 +1416,7 @@
ASSERT(m_frame.view());
- if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
+ if (m_pageDismissalEventBeingDispatched != PageDismissalType::None)
return;
if (m_frame.document())
@@ -1591,7 +1590,7 @@
void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy)
{
ASSERT(!m_frame.document() || !m_frame.document()->inPageCache());
- if (m_pageDismissalEventBeingDispatched != Page::DismissalType::None)
+ if (m_pageDismissalEventBeingDispatched != PageDismissalType::None)
return;
// If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
@@ -2847,7 +2846,7 @@
return true;
RefPtr<BeforeUnloadEvent> beforeUnloadEvent = BeforeUnloadEvent::create();
- m_pageDismissalEventBeingDispatched = Page::DismissalType::BeforeUnload;
+ m_pageDismissalEventBeingDispatched = PageDismissalType::BeforeUnload;
// We store the frame's page in a local variable because the frame might get detached inside dispatchEvent.
Page* page = m_frame.page();
@@ -2855,7 +2854,7 @@
domWindow->dispatchEvent(beforeUnloadEvent.get(), domWindow->document());
page->decrementFrameHandlingBeforeUnloadEventCount();
- m_pageDismissalEventBeingDispatched = Page::DismissalType::None;
+ m_pageDismissalEventBeingDispatched = PageDismissalType::None;
if (!beforeUnloadEvent->defaultPrevented())
document->defaultEventHandler(beforeUnloadEvent.get());
@@ -3553,14 +3552,4 @@
return WTF::move(frame);
}
-auto FrameLoader::PageDismissalEventType::operator=(Page::DismissalType dismissalType) -> PageDismissalEventType&
-{
- m_dismissalEventBeingDispatched = dismissalType;
-
- if (auto* page = m_frame.page())
- page->setDismissalEventBeingDispatched(dismissalType);
-
- return *this;
-}
-
} // namespace WebCore
diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
index e7bfe17..81b7497b 100644
--- a/Source/WebCore/loader/FrameLoader.h
+++ b/Source/WebCore/loader/FrameLoader.h
@@ -272,7 +272,8 @@
void started();
- Page::DismissalType pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
+ enum class PageDismissalType { None, BeforeUnload, PageHide, Unload };
+ PageDismissalType pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
WEBCORE_EXPORT NetworkingContext* networkingContext() const;
@@ -413,21 +414,7 @@
bool m_didCallImplicitClose;
bool m_wasUnloadEventEmitted;
- class PageDismissalEventType {
- public:
- PageDismissalEventType(Frame& frame)
- : m_frame(frame)
- { }
-
- PageDismissalEventType& operator=(Page::DismissalType);
- operator Page::DismissalType() const { return m_dismissalEventBeingDispatched; }
-
- private:
- Frame& m_frame;
- Page::DismissalType m_dismissalEventBeingDispatched { Page::DismissalType::None };
- };
-
- PageDismissalEventType m_pageDismissalEventBeingDispatched;
+ PageDismissalType m_pageDismissalEventBeingDispatched { PageDismissalType::None };
bool m_isComplete;
RefPtr<SerializedScriptValue> m_pendingStateObject;
diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
index 20394d1..82808b6 100644
--- a/Source/WebCore/loader/ImageLoader.cpp
+++ b/Source/WebCore/loader/ImageLoader.cpp
@@ -84,7 +84,7 @@
static inline bool pageIsBeingDismissed(Document& document)
{
Frame* frame = document.frame();
- return frame && frame->loader().pageDismissalEventBeingDispatched() != Page::DismissalType::None;
+ return frame && frame->loader().pageDismissalEventBeingDispatched() != FrameLoader::PageDismissalType::None;
}
ImageLoader::ImageLoader(Element& element)
diff --git a/Source/WebCore/loader/cache/CachedResource.cpp b/Source/WebCore/loader/cache/CachedResource.cpp
index 5378d9b..f4431bf 100644
--- a/Source/WebCore/loader/cache/CachedResource.cpp
+++ b/Source/WebCore/loader/cache/CachedResource.cpp
@@ -214,8 +214,8 @@
return;
}
- // Prevent 'pagehide' event handlers from starting new loads as we are in the PageCache.
- if (cachedResourceLoader.frame()->page() && cachedResourceLoader.frame()->page()->dismissalEventBeingDispatched() == Page::DismissalType::PageHide) {
+ // Prevent new loads if we are in the PageCache or being added to the PageCache.
+ if (cachedResourceLoader.frame()->page() && cachedResourceLoader.frame()->page()->inPageCache()) {
failBeforeStarting();
return;
}
diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
index 6e085cc..6c30d74 100644
--- a/Source/WebCore/loader/cache/CachedResourceLoader.cpp
+++ b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -176,7 +176,7 @@
CachedResourceHandle<CachedImage> CachedResourceLoader::requestImage(CachedResourceRequest& request)
{
if (Frame* frame = this->frame()) {
- if (frame->loader().pageDismissalEventBeingDispatched() != Page::DismissalType::None) {
+ if (frame->loader().pageDismissalEventBeingDispatched() != FrameLoader::PageDismissalType::None) {
URL requestURL = request.resourceRequest().url();
if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
PingLoader::loadImage(*frame, requestURL);
diff --git a/Source/WebCore/page/Page.cpp b/Source/WebCore/page/Page.cpp
index fcb71bb..1d3dcc2 100644
--- a/Source/WebCore/page/Page.cpp
+++ b/Source/WebCore/page/Page.cpp
@@ -562,6 +562,12 @@
}
}
+bool Page::inPageCache() const
+{
+ auto* document = mainFrame().document();
+ return document && document->inPageCache();
+}
+
static Frame* incrementFrame(Frame* curr, bool forward, bool wrapFlag)
{
return forward
diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
index 14b57df..264b8d4 100644
--- a/Source/WebCore/page/Page.h
+++ b/Source/WebCore/page/Page.h
@@ -156,14 +156,7 @@
MainFrame& mainFrame() { ASSERT(m_mainFrame); return *m_mainFrame; }
const MainFrame& mainFrame() const { ASSERT(m_mainFrame); return *m_mainFrame; }
- enum class DismissalType {
- None,
- BeforeUnload,
- PageHide,
- Unload
- };
- DismissalType dismissalEventBeingDispatched() const { return m_dismissalEventBeingDispatched; }
- void setDismissalEventBeingDispatched(DismissalType dismissalType) { m_dismissalEventBeingDispatched = dismissalType; }
+ bool inPageCache() const;
bool openedByDOM() const;
void setOpenedByDOM();
@@ -627,7 +620,6 @@
bool m_isClosing;
MediaProducer::MediaStateFlags m_mediaState { MediaProducer::IsNotPlaying };
- DismissalType m_dismissalEventBeingDispatched { DismissalType::None };
bool m_userContentExtensionsEnabled { true };
};
