2011-06-30 Sreeram Ramachandran <sreeram@chromium.org>
Reviewed by Adam Barth.
Suppress modal JavaScript/HTML dialogs during unload events
https://bugs.webkit.org/show_bug.cgi?id=56397
Adds Chromium-specific test expectations that show alerts in unload
handlers being replaced by console messages.
* fast/loader/page-dismissal-modal-dialogs-expected.txt: Added.
* fast/loader/page-dismissal-modal-dialogs.html: Added.
* fast/loader/resources/page-dismissal-modal-dialogs-iframe.html: Added.
* platform/chromium-mac/fast/history/timed-refresh-in-cached-frame-expected.txt:
* platform/chromium-win/fast/history/timed-refresh-in-cached-frame-expected.txt:
* platform/chromium/fast/dom/Geolocation/notimer-after-unload-expected.txt: Added.
* platform/chromium/fast/events/onbeforeunload-focused-iframe-expected.txt: Added.
* platform/chromium/fast/events/onunload-clears-onbeforeunload-expected.txt: Added.
* platform/chromium/fast/events/onunload-expected.txt: Added.
* platform/chromium/fast/events/onunload-not-on-body-expected.txt: Added.
* platform/chromium/fast/events/onunload-window-property-expected.txt: Added.
* platform/chromium/fast/events/pageshow-pagehide-on-back-uncached-expected.txt: Added.
* platform/chromium/fast/history/timed-refresh-in-cached-frame-expected.txt: Added.
* platform/chromium/fast/loader/frames-with-unload-handlers-in-page-cache-expected.txt: Added.
* platform/chromium/fast/loader/page-dismissal-modal-dialogs-expected.txt: Added.
* platform/chromium/fast/loader/recursive-before-unload-crash-expected.txt: Added.
2011-06-30 Sreeram Ramachandran <sreeram@chromium.org>
Reviewed by Adam Barth.
Suppress modal JavaScript/HTML dialogs during unload events
https://bugs.webkit.org/show_bug.cgi?id=56397
Allows clients to specify whether to show alerts during unload handlers.
When checking for whether a page dismissal event is being dispatched,
it's important to check all frames on the page (otherwise it becomes a
loophole easily exploited).
Test: fast/loader/page-dismissal-modal-dialogs.html
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader):
(WebCore::FrameLoader::stopLoading):
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::stopAllLoaders):
(WebCore::FrameLoader::fireBeforeUnloadEvent):
* loader/FrameLoader.h:
(WebCore::FrameLoader::pageDismissalEventBeingDispatched):
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestImage):
* page/Chrome.cpp:
(WebCore::canRunModalIfDuringPageDismissal):
(WebCore::Chrome::canRunModalNow):
(WebCore::Chrome::runJavaScriptAlert):
(WebCore::Chrome::runJavaScriptConfirm):
(WebCore::Chrome::runJavaScriptPrompt):
* page/Chrome.h:
* page/ChromeClient.h:
(WebCore::ChromeClient::shouldRunModalDialogDuringPageDismissal):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::showModalDialog):
2011-06-30 Sreeram Ramachandran <sreeram@chromium.org>
Reviewed by Adam Barth.
Suppress modal JavaScript/HTML dialogs during unload events
https://bugs.webkit.org/show_bug.cgi?id=56397
Implementation of the new shouldRunModalDialogDuringPageDismissal() API
to block alerts during unload handlers. Logs such events to the console
and updates histograms.
* src/ChromeClientImpl.cpp:
(WebKit::ChromeClientImpl::shouldRunModalDialogDuringPageDismissal):
* src/ChromeClientImpl.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@90164 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index a47d6fc..147508a 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,40 @@
+2011-06-30 Sreeram Ramachandran <sreeram@chromium.org>
+
+ Reviewed by Adam Barth.
+
+ Suppress modal JavaScript/HTML dialogs during unload events
+ https://bugs.webkit.org/show_bug.cgi?id=56397
+
+ Allows clients to specify whether to show alerts during unload handlers.
+ When checking for whether a page dismissal event is being dispatched,
+ it's important to check all frames on the page (otherwise it becomes a
+ loophole easily exploited).
+
+ Test: fast/loader/page-dismissal-modal-dialogs.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::FrameLoader):
+ (WebCore::FrameLoader::stopLoading):
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::stopAllLoaders):
+ (WebCore::FrameLoader::fireBeforeUnloadEvent):
+ * loader/FrameLoader.h:
+ (WebCore::FrameLoader::pageDismissalEventBeingDispatched):
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::requestImage):
+ * page/Chrome.cpp:
+ (WebCore::canRunModalIfDuringPageDismissal):
+ (WebCore::Chrome::canRunModalNow):
+ (WebCore::Chrome::runJavaScriptAlert):
+ (WebCore::Chrome::runJavaScriptConfirm):
+ (WebCore::Chrome::runJavaScriptPrompt):
+ * page/Chrome.h:
+ * page/ChromeClient.h:
+ (WebCore::ChromeClient::shouldRunModalDialogDuringPageDismissal):
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::showModalDialog):
+
2011-06-30 Julien Chaffraix <jchaffraix@webkit.org>
Reviewed by Nikolas Zimmermann.
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index 0a62b06..4a1447a 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -183,7 +183,7 @@
, m_isExecutingJavaScriptFormAction(false)
, m_didCallImplicitClose(false)
, m_wasUnloadEventEmitted(false)
- , m_pageDismissalEventBeingDispatched(false)
+ , m_pageDismissalEventBeingDispatched(NoDismissal)
, m_isComplete(false)
, m_isLoadingMainResource(false)
, m_hasReceivedFirstData(false)
@@ -367,16 +367,18 @@
Node* currentFocusedNode = m_frame->document()->focusedNode();
if (currentFocusedNode)
currentFocusedNode->aboutToUnload();
- m_pageDismissalEventBeingDispatched = true;
if (m_frame->domWindow()) {
- if (unloadEventPolicy == UnloadEventPolicyUnloadAndPageHide)
+ if (unloadEventPolicy == UnloadEventPolicyUnloadAndPageHide) {
+ m_pageDismissalEventBeingDispatched = PageHideDismissal;
m_frame->domWindow()->dispatchEvent(PageTransitionEvent::create(eventNames().pagehideEvent, m_frame->document()->inPageCache()), m_frame->document());
+ }
if (!m_frame->document()->inPageCache()) {
RefPtr<Event> unloadEvent(Event::create(eventNames().unloadEvent, false, false));
// The DocumentLoader (and thus its DocumentLoadTiming) might get destroyed
// while dispatching the event, so protect it to prevent writing the end
// time into freed memory.
RefPtr<DocumentLoader> documentLoader = m_provisionalDocumentLoader;
+ m_pageDismissalEventBeingDispatched = UnloadDismissal;
if (documentLoader && !documentLoader->timing()->unloadEventStart && !documentLoader->timing()->unloadEventEnd) {
DocumentLoadTiming* timing = documentLoader->timing();
ASSERT(timing->navigationStart);
@@ -385,7 +387,7 @@
m_frame->domWindow()->dispatchEvent(unloadEvent, m_frame->domWindow()->document());
}
}
- m_pageDismissalEventBeingDispatched = false;
+ m_pageDismissalEventBeingDispatched = NoDismissal;
if (m_frame->document())
m_frame->document()->updateStyleIfNeeded();
m_wasUnloadEventEmitted = true;
@@ -1182,7 +1184,7 @@
return;
}
- if (m_pageDismissalEventBeingDispatched)
+ if (m_pageDismissalEventBeingDispatched != NoDismissal)
return;
NavigationAction action(newURL, newLoadType, isFormSubmission, event);
@@ -1317,7 +1319,7 @@
ASSERT(m_frame->view());
- if (m_pageDismissalEventBeingDispatched)
+ if (m_pageDismissalEventBeingDispatched != NoDismissal)
return;
if (m_frame->document())
@@ -1552,7 +1554,7 @@
void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy)
{
ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());
- if (m_pageDismissalEventBeingDispatched)
+ if (m_pageDismissalEventBeingDispatched != NoDismissal)
return;
// If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
@@ -2721,9 +2723,9 @@
return true;
RefPtr<BeforeUnloadEvent> beforeUnloadEvent = BeforeUnloadEvent::create();
- m_pageDismissalEventBeingDispatched = true;
+ m_pageDismissalEventBeingDispatched = BeforeUnloadDismissal;
domWindow->dispatchEvent(beforeUnloadEvent.get(), domWindow->document());
- m_pageDismissalEventBeingDispatched = false;
+ m_pageDismissalEventBeingDispatched = NoDismissal;
if (!beforeUnloadEvent->defaultPrevented())
document->defaultEventHandler(beforeUnloadEvent.get());
diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
index d73b5aa..fddaa08 100644
--- a/Source/WebCore/loader/FrameLoader.h
+++ b/Source/WebCore/loader/FrameLoader.h
@@ -272,7 +272,13 @@
void started();
- bool pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
+ enum PageDismissalType {
+ NoDismissal = 0,
+ BeforeUnloadDismissal = 1,
+ PageHideDismissal = 2,
+ UnloadDismissal = 3
+ };
+ PageDismissalType pageDismissalEventBeingDispatched() const { return m_pageDismissalEventBeingDispatched; }
NetworkingContext* networkingContext() const;
@@ -395,7 +401,7 @@
bool m_didCallImplicitClose;
bool m_wasUnloadEventEmitted;
- bool m_pageDismissalEventBeingDispatched;
+ PageDismissalType m_pageDismissalEventBeingDispatched;
bool m_isComplete;
bool m_isLoadingMainResource;
diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
index deda239..f613156 100644
--- a/Source/WebCore/loader/cache/CachedResourceLoader.cpp
+++ b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -129,7 +129,7 @@
if (!f->loader()->client()->allowImages(!settings || settings->areImagesEnabled()))
return 0;
- if (f->loader()->pageDismissalEventBeingDispatched()) {
+ if (f->loader()->pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal) {
KURL requestURL = request.url();
if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL))
PingLoader::loadImage(f, requestURL);
diff --git a/Source/WebCore/page/Chrome.cpp b/Source/WebCore/page/Chrome.cpp
index 394d5e5..7ede62c 100644
--- a/Source/WebCore/page/Chrome.cpp
+++ b/Source/WebCore/page/Chrome.cpp
@@ -206,11 +206,22 @@
return m_client->canRunModal();
}
+static bool canRunModalIfDuringPageDismissal(Page* page, ChromeClient::DialogType dialog, const String& message)
+{
+ for (Frame* frame = page->mainFrame(); frame; frame = frame->tree()->traverseNext()) {
+ FrameLoader::PageDismissalType dismissal = frame->loader()->pageDismissalEventBeingDispatched();
+ if (dismissal != FrameLoader::NoDismissal)
+ return page->chrome()->client()->shouldRunModalDialogDuringPageDismissal(dialog, message, dismissal);
+ }
+ return true;
+}
+
bool Chrome::canRunModalNow() const
{
// If loads are blocked, we can't run modal because the contents
// of the modal dialog will never show up!
- return canRunModal() && !ResourceHandle::loadsBlocked();
+ return canRunModal() && !ResourceHandle::loadsBlocked()
+ && canRunModalIfDuringPageDismissal(m_page, ChromeClient::HTMLDialog, String());
}
void Chrome::runModal() const
@@ -287,15 +298,10 @@
m_client->closeWindowSoon();
}
-static inline void willRunModalDialog(const Frame* frame, const ChromeClient::DialogType& dialogType, const ChromeClient* client)
-{
- if (frame->loader()->pageDismissalEventBeingDispatched())
- client->willRunModalDialogDuringPageDismissal(dialogType);
-}
-
void Chrome::runJavaScriptAlert(Frame* frame, const String& message)
{
- willRunModalDialog(frame, ChromeClient::AlertDialog, m_client);
+ if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::AlertDialog, message))
+ return;
// Defer loads in case the client method runs a new event loop that would
// otherwise cause the load to continue while we're in the middle of executing JavaScript.
@@ -307,7 +313,8 @@
bool Chrome::runJavaScriptConfirm(Frame* frame, const String& message)
{
- willRunModalDialog(frame, ChromeClient::ConfirmDialog, m_client);
+ if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::ConfirmDialog, message))
+ return false;
// Defer loads in case the client method runs a new event loop that would
// otherwise cause the load to continue while we're in the middle of executing JavaScript.
@@ -319,7 +326,8 @@
bool Chrome::runJavaScriptPrompt(Frame* frame, const String& prompt, const String& defaultValue, String& result)
{
- willRunModalDialog(frame, ChromeClient::PromptDialog, m_client);
+ if (!canRunModalIfDuringPageDismissal(m_page, ChromeClient::PromptDialog, prompt))
+ return false;
// Defer loads in case the client method runs a new event loop that would
// otherwise cause the load to continue while we're in the middle of executing JavaScript.
@@ -574,9 +582,4 @@
return m_client->requiresFullscreenForVideoPlayback();
}
-void Chrome::willRunModalHTMLDialog(const Frame* frame) const
-{
- willRunModalDialog(frame, ChromeClient::HTMLDialog, m_client);
-}
-
} // namespace WebCore
diff --git a/Source/WebCore/page/Chrome.h b/Source/WebCore/page/Chrome.h
index 69389ff..d9ae0de 100644
--- a/Source/WebCore/page/Chrome.h
+++ b/Source/WebCore/page/Chrome.h
@@ -179,8 +179,6 @@
void showContextMenu();
#endif
- void willRunModalHTMLDialog(const Frame*) const;
-
private:
Page* m_page;
ChromeClient* m_client;
diff --git a/Source/WebCore/page/ChromeClient.h b/Source/WebCore/page/ChromeClient.h
index 9c59c23..a27c766 100644
--- a/Source/WebCore/page/ChromeClient.h
+++ b/Source/WebCore/page/ChromeClient.h
@@ -25,6 +25,7 @@
#include "ConsoleTypes.h"
#include "Cursor.h"
#include "FocusDirection.h"
+#include "FrameLoader.h"
#include "GraphicsContext.h"
#include "HostWindow.h"
#include "PopupMenu.h"
@@ -34,6 +35,7 @@
#include "WebCoreKeyboardUIMode.h"
#include <wtf/Forward.h>
#include <wtf/PassOwnPtr.h>
+#include <wtf/UnusedParam.h>
#include <wtf/Vector.h>
#ifndef __OBJC__
@@ -323,10 +325,9 @@
AlertDialog = 0,
ConfirmDialog = 1,
PromptDialog = 2,
- HTMLDialog = 3,
- NumDialogTypes = 4
+ HTMLDialog = 3
};
- virtual void willRunModalDialogDuringPageDismissal(const DialogType&) const { }
+ virtual bool shouldRunModalDialogDuringPageDismissal(const DialogType&, const String& dialogMessage, FrameLoader::PageDismissalType) const { UNUSED_PARAM(dialogMessage); return true; }
virtual void numWheelEventHandlersChanged(unsigned) = 0;
diff --git a/Source/WebCore/page/DOMWindow.cpp b/Source/WebCore/page/DOMWindow.cpp
index d7f2ae4..da78596 100644
--- a/Source/WebCore/page/DOMWindow.cpp
+++ b/Source/WebCore/page/DOMWindow.cpp
@@ -1851,9 +1851,6 @@
if (!firstFrame)
return;
- if (m_frame->page())
- m_frame->page()->chrome()->willRunModalHTMLDialog(m_frame);
-
if (!canShowModalDialogNow(m_frame) || !firstWindow->allowPopUp())
return;
diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
index 5be0b60..45eafc3 100644
--- a/Source/WebKit/chromium/ChangeLog
+++ b/Source/WebKit/chromium/ChangeLog
@@ -1,3 +1,18 @@
+2011-06-30 Sreeram Ramachandran <sreeram@chromium.org>
+
+ Reviewed by Adam Barth.
+
+ Suppress modal JavaScript/HTML dialogs during unload events
+ https://bugs.webkit.org/show_bug.cgi?id=56397
+
+ Implementation of the new shouldRunModalDialogDuringPageDismissal() API
+ to block alerts during unload handlers. Logs such events to the console
+ and updates histograms.
+
+ * src/ChromeClientImpl.cpp:
+ (WebKit::ChromeClientImpl::shouldRunModalDialogDuringPageDismissal):
+ * src/ChromeClientImpl.h:
+
2011-06-30 Anders Carlsson <andersca@apple.com>
Reviewed by Dan Bernstein.
diff --git a/Source/WebKit/chromium/src/ChromeClientImpl.cpp b/Source/WebKit/chromium/src/ChromeClientImpl.cpp
index 5a180bd..bbb340c 100644
--- a/Source/WebKit/chromium/src/ChromeClientImpl.cpp
+++ b/Source/WebKit/chromium/src/ChromeClientImpl.cpp
@@ -90,6 +90,7 @@
#include "WebWindowFeatures.h"
#include "WindowFeatures.h"
#include "WrappedResourceRequest.h"
+#include <wtf/text/StringConcatenate.h>
#include <wtf/unicode/CharacterNames.h>
using namespace WebCore;
@@ -945,9 +946,21 @@
return adoptRef(new SearchPopupMenuChromium(client));
}
-void ChromeClientImpl::willRunModalDialogDuringPageDismissal(const DialogType& dialogType) const
+bool ChromeClientImpl::shouldRunModalDialogDuringPageDismissal(const DialogType& dialogType, const String& dialogMessage, FrameLoader::PageDismissalType dismissalType) const
{
- PlatformBridge::histogramEnumeration("Renderer.ModalDialogsDuringPageDismissal", static_cast<int>(dialogType), static_cast<int>(NumDialogTypes));
+ const char* kDialogs[] = {"alert", "confirm", "prompt", "showModalDialog"};
+ int dialog = static_cast<int>(dialogType);
+ ASSERT(0 <= dialog && dialog < static_cast<int>(arraysize(kDialogs)));
+
+ const char* kDismissals[] = {"beforeunload", "pagehide", "unload"};
+ int dismissal = static_cast<int>(dismissalType) - 1; // Exclude NoDismissal.
+ ASSERT(0 <= dismissal && dismissal < static_cast<int>(arraysize(kDismissals)));
+
+ PlatformBridge::histogramEnumeration("Renderer.ModalDialogsDuringPageDismissal", dismissal * arraysize(kDialogs) + dialog, arraysize(kDialogs) * arraysize(kDismissals));
+
+ m_webView->mainFrame()->addMessageToConsole(WebConsoleMessage(WebConsoleMessage::LevelError, makeString("Blocked ", kDialogs[dialog], "('", dialogMessage, "') during ", kDismissals[dismissal], ".")));
+
+ return false;
}
} // namespace WebKit
diff --git a/Source/WebKit/chromium/src/ChromeClientImpl.h b/Source/WebKit/chromium/src/ChromeClientImpl.h
index a09d4c5..2f79c05 100644
--- a/Source/WebKit/chromium/src/ChromeClientImpl.h
+++ b/Source/WebKit/chromium/src/ChromeClientImpl.h
@@ -195,7 +195,7 @@
virtual void showContextMenu() { }
#endif
- virtual void willRunModalDialogDuringPageDismissal(const DialogType&) const;
+ virtual bool shouldRunModalDialogDuringPageDismissal(const DialogType&, const String& dialogMessage, WebCore::FrameLoader::PageDismissalType) const;
virtual bool shouldRubberBandInDirection(WebCore::ScrollDirection) const { return true; }
virtual void numWheelEventHandlersChanged(unsigned) { }
