Changeset 75305 in webkit


Ignore:
Timestamp:
Jan 7, 2011 6:15:10 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

2011-01-06 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Adam Barth.

onbeforeunload is broken for framesets
https://bugs.webkit.org/show_bug.cgi?id=19418

Added beforeunload event support for sub frames. WebKit's implementation tries to match
that of Internet Explorer as much as possible. beforeunload event is fired for each and
every descendent of a frame that is about to navigate.

When a value other than null is returned by a beforeunload handler, a confirmation dialog
is shown for each handler (calls chrome's runBeforeUnloadConfirmPanel) just like it is done
for main frames.

In addition, navigation is forbidden while beforeunload handlers are being called.
Setting values to location.href, location.reload, and other means of navigations are thus
ignored while beforeunload event handler is being ran, matching Internet Explorer's behavior.

Because navigation needs to prevented globally, NavigationDisablerForBeforeUnload is added to
NavigationScheduler.h, which is instantiated as a RAII object in FrameLoader::shouldClose.

Tests: fast/events/before-unload-adopt-subframe-to-outside.html

fast/events/before-unload-adopt-within-subframes.html
fast/events/before-unload-forbidden-navigation.html
fast/events/before-unload-in-multiple-subframes.html
fast/events/before-unload-in-subframe.html
fast/events/before-unload-javascript-navigation.html
fast/events/before-unload-remove-and-add-subframe.html
fact/events/before-unload-remove-itself.html
fast/events/before-unload-with-subframes.html

  • loader/FrameLoader.cpp: (WebCore::FrameLoader::shouldClose): Calls fireBeforeUnloadEvent on m_frame and m_frame's descendents. Returns true only if every call to fireBeforeUnloadEvent returned true. (WebCore::FrameLoader::fireBeforeUnloadEvent): Fires a beforeunload event and calls chrome's runBeforeUnloadConfirmPanel as needed. (WebCore::FrameLoader::continueLoadAfterNavigationPolicy): Calls shouldClose for all frames.
  • loader/FrameLoader.h:
  • loader/NavigationScheduler.cpp: (WebCore::NavigationScheduler::shouldScheduleNavigation): Checks the nullity of Page and calls NavigationDisablerForBeforeUnload::isNavigationAllowed when url is not javascript scheme. (WebCore::NavigationScheduler::scheduleRedirect): Calls shouldScheduleNavigation. (WebCore::NavigationScheduler::scheduleLocationChange): Ditto. (WebCore::NavigationScheduler::scheduleRefresh): Ditto. (WebCore::NavigationScheduler::scheduleHistoryNavigation): Ditto.
  • loader/NavigationScheduler.h: (WebCore::NavigationDisablerForBeforeUnload::NavigationDisablerForBeforeUnload): Disables navigation. (WebCore::NavigationDisablerForBeforeUnload::~NavigationDisablerForBeforeUnload): Enables navigation when called on the last instance of NavigationDisablerForBeforeUnload. (WebCore::NavigationDisablerForBeforeUnload::isNavigationAllowed): Returns true if there are no instance of NavigationDisablerForBeforeUnload left on the stack.

2011-01-06 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Adam Barth.

onbeforeunload is broken for framesets
https://bugs.webkit.org/show_bug.cgi?id=19418

Added tests to ensure WebKit fires beforeunload events for subframes,
and disallows navigation except that of javascript scheme while beforeunload event
handlers are being called.

Also added a test to ensure WebKit fires beforeunload event for subframes exactly
once even if a subframe was moved around within a beforeunload event handler.

A test that ensures beforeunload event is not fired for an iframe if the iframe
was added or removed within a beforeunload event handler is also added.

Furthermore, a test to ensure WebKit does not fire a beforeunload event to an iframe
that has been adopted by a document outside of the unloading document is added.

  • fast/events/before-unload-adopt-subframe-to-outside-expected.txt: Added.
  • fast/events/before-unload-adopt-subframe-to-outside.html: Added.
  • fast/events/before-unload-adopt-within-subframes-expected.txt: Added.
  • fast/events/before-unload-adopt-within-subframes.html: Added.
  • fast/events/before-unload-forbidden-navigation-expected.txt: Added.
  • fast/events/before-unload-forbidden-navigation.html: Added.
  • fast/events/before-unload-in-multiple-subframes-expected.txt: Added.
  • fast/events/before-unload-in-multiple-subframes.html: Added.
  • fast/events/before-unload-in-subframe-expected.txt: Added.
  • fast/events/before-unload-in-subframe.html: Added.
  • fast/events/before-unload-javascript-navigation-expected.txt: Added.
  • fast/events/before-unload-javascript-navigation.html: Added.
  • fast/events/before-unload-remove-and-add-subframe-expected.txt: Added.
  • fast/events/before-unload-remove-and-add-subframe.html: Added.
  • fact/events/before-unload-remove-itself-expected.txt: Added.
  • fact/events/before-unload-remove-itself.html: Added.
  • fast/events/before-unload-with-subframes-expected.txt: Added.
  • fast/events/before-unload-with-subframes.html: Added.
  • fast/events/resources/before-unload-in-subframe-child.html: Added.
  • fast/events/resources/before-unload-in-subframe-destination.html: Added.
  • fast/events/resources/before-unload-in-subframe-fail.html: Added.
  • fast/events/resources/before-unload-with-subframes-parent.html: Added.
Location:
trunk
Files:
22 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r75298 r75305  
     12011-01-06  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        onbeforeunload is broken for framesets
     6        https://bugs.webkit.org/show_bug.cgi?id=19418
     7
     8        Added tests to ensure WebKit fires beforeunload events for subframes,
     9        and disallows navigation except that of javascript scheme while beforeunload event
     10        handlers are being called.
     11
     12        Also added a test to ensure WebKit fires beforeunload event for subframes exactly
     13        once even if a subframe was moved around within a beforeunload event handler.
     14
     15        A test that ensures beforeunload event is not fired for an iframe if the iframe
     16        was added or removed within a beforeunload event handler is also added.
     17
     18        Furthermore, a test to ensure WebKit does not fire a beforeunload event to an iframe
     19        that has been adopted by a document outside of the unloading document is added.
     20
     21        * fast/events/before-unload-adopt-subframe-to-outside-expected.txt: Added.
     22        * fast/events/before-unload-adopt-subframe-to-outside.html: Added.
     23        * fast/events/before-unload-adopt-within-subframes-expected.txt: Added.
     24        * fast/events/before-unload-adopt-within-subframes.html: Added.
     25        * fast/events/before-unload-forbidden-navigation-expected.txt: Added.
     26        * fast/events/before-unload-forbidden-navigation.html: Added.
     27        * fast/events/before-unload-in-multiple-subframes-expected.txt: Added.
     28        * fast/events/before-unload-in-multiple-subframes.html: Added.
     29        * fast/events/before-unload-in-subframe-expected.txt: Added.
     30        * fast/events/before-unload-in-subframe.html: Added.
     31        * fast/events/before-unload-javascript-navigation-expected.txt: Added.
     32        * fast/events/before-unload-javascript-navigation.html: Added.
     33        * fast/events/before-unload-remove-and-add-subframe-expected.txt: Added.
     34        * fast/events/before-unload-remove-and-add-subframe.html: Added.
     35        * fact/events/before-unload-remove-itself-expected.txt: Added.
     36        * fact/events/before-unload-remove-itself.html: Added.
     37        * fast/events/before-unload-with-subframes-expected.txt: Added.
     38        * fast/events/before-unload-with-subframes.html: Added.
     39        * fast/events/resources/before-unload-in-subframe-child.html: Added.
     40        * fast/events/resources/before-unload-in-subframe-destination.html: Added.
     41        * fast/events/resources/before-unload-in-subframe-fail.html: Added.
     42        * fast/events/resources/before-unload-with-subframes-parent.html: Added.
     43
    1442011-01-07  Martin Robinson  <mrobinson@igalia.com>
    245
  • trunk/WebCore/ChangeLog

    r75296 r75305  
     12011-01-06  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        onbeforeunload is broken for framesets
     6        https://bugs.webkit.org/show_bug.cgi?id=19418
     7
     8        Added beforeunload event support for sub frames. WebKit's implementation tries to match
     9        that of Internet Explorer as much as possible. beforeunload event is fired for each and
     10        every descendent of a frame that is about to navigate.
     11
     12        When a value other than null is returned by a beforeunload handler, a confirmation dialog
     13        is shown for each handler (calls chrome's runBeforeUnloadConfirmPanel) just like it is done
     14        for main frames.
     15
     16        In addition, navigation is forbidden while beforeunload handlers are being called.
     17        Setting values to location.href, location.reload, and other means of navigations are thus
     18        ignored while beforeunload event handler is being ran, matching Internet Explorer's behavior.
     19
     20        Because navigation needs to prevented globally, NavigationDisablerForBeforeUnload is added to
     21        NavigationScheduler.h, which is instantiated as a RAII object in FrameLoader::shouldClose.
     22
     23        Tests: fast/events/before-unload-adopt-subframe-to-outside.html
     24               fast/events/before-unload-adopt-within-subframes.html
     25               fast/events/before-unload-forbidden-navigation.html
     26               fast/events/before-unload-in-multiple-subframes.html
     27               fast/events/before-unload-in-subframe.html
     28               fast/events/before-unload-javascript-navigation.html
     29               fast/events/before-unload-remove-and-add-subframe.html
     30               fact/events/before-unload-remove-itself.html
     31               fast/events/before-unload-with-subframes.html
     32
     33       * loader/FrameLoader.cpp:
     34       (WebCore::FrameLoader::shouldClose): Calls fireBeforeUnloadEvent on m_frame and m_frame's
     35       descendents. Returns true only if every call to fireBeforeUnloadEvent returned true.
     36       (WebCore::FrameLoader::fireBeforeUnloadEvent): Fires a beforeunload event and calls
     37       chrome's runBeforeUnloadConfirmPanel as needed.
     38       (WebCore::FrameLoader::continueLoadAfterNavigationPolicy): Calls shouldClose for all frames.
     39       * loader/FrameLoader.h:
     40       * loader/NavigationScheduler.cpp:
     41       (WebCore::NavigationScheduler::shouldScheduleNavigation): Checks the nullity of Page and calls
     42       NavigationDisablerForBeforeUnload::isNavigationAllowed when url is not javascript scheme.
     43       (WebCore::NavigationScheduler::scheduleRedirect): Calls shouldScheduleNavigation.
     44       (WebCore::NavigationScheduler::scheduleLocationChange): Ditto.
     45       (WebCore::NavigationScheduler::scheduleRefresh): Ditto.
     46       (WebCore::NavigationScheduler::scheduleHistoryNavigation): Ditto.
     47       * loader/NavigationScheduler.h:
     48       (WebCore::NavigationDisablerForBeforeUnload::NavigationDisablerForBeforeUnload): Disables navigation.
     49       (WebCore::NavigationDisablerForBeforeUnload::~NavigationDisablerForBeforeUnload): Enables navigation
     50       when called on the last instance of NavigationDisablerForBeforeUnload.
     51       (WebCore::NavigationDisablerForBeforeUnload::isNavigationAllowed): Returns true if there are no instance
     52       of NavigationDisablerForBeforeUnload left on the stack.
     53
    1542011-01-07  Martin Robinson  <mrobinson@igalia.com>
    255
  • trunk/WebCore/WebCore.xcodeproj/project.pbxproj

    r75277 r75305  
    79287928                6EBF0E7412A9868800DB1709 /* JSOESTextureFloat.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSOESTextureFloat.cpp; sourceTree = "<group>"; };
    79297929                6EBF0E7512A9868800DB1709 /* JSOESTextureFloat.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSOESTextureFloat.h; sourceTree = "<group>"; };
    7930                 93F1D5BE12D5335600832BEC /* JSWebKitLoseContext.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = JSWebKitLoseContext.cpp; path = JSWebKitLoseContext.cpp; sourceTree = "<group>"; };
    7931                 93F1D5BF12D5335600832BEC /* JSWebKitLoseContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = JSWebKitLoseContext.h; path = JSWebKitLoseContext.h; sourceTree = "<group>"; };
    79327930                6EE8A77010F803F3005A4A24 /* JSWebGLContextAttributes.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebGLContextAttributes.cpp; sourceTree = "<group>"; };
    79337931                6EE8A77110F803F3005A4A24 /* JSWebGLContextAttributes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSWebGLContextAttributes.h; sourceTree = "<group>"; };
     
    93039301                93F1D5B812D532C400832BEC /* WebKitLoseContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WebKitLoseContext.h; path = canvas/WebKitLoseContext.h; sourceTree = "<group>"; };
    93049302                93F1D5B912D532C400832BEC /* WebKitLoseContext.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = WebKitLoseContext.idl; path = canvas/WebKitLoseContext.idl; sourceTree = "<group>"; };
     9303                93F1D5BE12D5335600832BEC /* JSWebKitLoseContext.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSWebKitLoseContext.cpp; sourceTree = "<group>"; };
     9304                93F1D5BF12D5335600832BEC /* JSWebKitLoseContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSWebKitLoseContext.h; sourceTree = "<group>"; };
    93059305                93F6F1EA127F70B10055CB06 /* WebGLContextEvent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WebGLContextEvent.cpp; path = canvas/WebGLContextEvent.cpp; sourceTree = "<group>"; };
    93069306                93F6F1EB127F70B10055CB06 /* WebGLContextEvent.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WebGLContextEvent.h; path = canvas/WebGLContextEvent.h; sourceTree = "<group>"; };
  • trunk/WebCore/loader/FrameLoader.cpp

    r75129 r75305  
    28632863        return true;
    28642864
     2865    // Store all references to each subframe in advance since beforeunload's event handler may modify frame
     2866    Vector<RefPtr<Frame> > targetFrames;
     2867    targetFrames.append(m_frame);
     2868    for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->traverseNext(m_frame))
     2869        targetFrames.append(child);
     2870
     2871    bool shouldClose = false;
     2872    {
     2873        NavigationDisablerForBeforeUnload navigationDisabler;
     2874        size_t i;
     2875
     2876        for (i = 0; i < targetFrames.size(); i++) {
     2877            if (!targetFrames[i]->tree()->isDescendantOf(m_frame))
     2878                continue;
     2879            if (!targetFrames[i]->loader()->fireBeforeUnloadEvent(chrome))
     2880                break;
     2881        }
     2882
     2883        if (i == targetFrames.size())
     2884            shouldClose = true;
     2885    }
     2886
     2887    return shouldClose;
     2888}
     2889
     2890bool FrameLoader::fireBeforeUnloadEvent(Chrome* chrome)
     2891{
    28652892    DOMWindow* domWindow = m_frame->existingDOMWindow();
    28662893    if (!domWindow)
     
    28682895
    28692896    RefPtr<Document> document = m_frame->document();
    2870     HTMLElement* body = document->body();
    2871     if (!body)
     2897    if (!document->body())
    28722898        return true;
    28732899
     
    28992925    //       is the user responding Cancel to the form repost nag sheet.
    29002926    //    2) User responded Cancel to an alert popped up by the before unload event handler.
    2901     // The "before unload" event handler runs only for the main frame.
    2902     bool canContinue = shouldContinue && (!isLoadingMainFrame() || shouldClose());
     2927    bool canContinue = shouldContinue && shouldClose();
    29032928
    29042929    if (!canContinue) {
  • trunk/WebCore/loader/FrameLoader.h

    r75129 r75305  
    5454class CachedPage;
    5555class CachedResource;
     56class Chrome;
    5657class DOMWrapperWorld;
    5758class Document;
     
    362363    static void callContinueLoadAfterNewWindowPolicy(void*, const ResourceRequest&, PassRefPtr<FormState>, const String& frameName, const NavigationAction&, bool shouldContinue);
    363364    static void callContinueFragmentScrollAfterNavigationPolicy(void*, const ResourceRequest&, PassRefPtr<FormState>, bool shouldContinue);
     365   
     366    bool fireBeforeUnloadEvent(Chrome*);
    364367
    365368    void continueLoadAfterNavigationPolicy(const ResourceRequest&, PassRefPtr<FormState>, bool shouldContinue);
  • trunk/WebCore/loader/NavigationScheduler.cpp

    r73444 r75305  
    5252namespace WebCore {
    5353
     54unsigned NavigationDisablerForBeforeUnload::s_navigationDisableCount = 0;
     55
    5456class ScheduledNavigation : public Noncopyable {
    5557public:
     
    264266}
    265267
     268inline bool NavigationScheduler::shouldScheduleNavigation() const
     269{
     270    return m_frame->page();
     271}
     272
     273inline bool NavigationScheduler::shouldScheduleNavigation(const String& url) const
     274{
     275    return shouldScheduleNavigation() && (protocolIsJavaScript(url) || NavigationDisablerForBeforeUnload::isNavigationAllowed());
     276}
     277
    266278void NavigationScheduler::scheduleRedirect(double delay, const String& url)
    267279{
    268     if (!m_frame->page())
     280    if (!shouldScheduleNavigation(url))
    269281        return;
    270282    if (delay < 0 || delay > INT_MAX / 1000)
     
    298310void NavigationScheduler::scheduleLocationChange(PassRefPtr<SecurityOrigin> securityOrigin, const String& url, const String& referrer, bool lockHistory, bool lockBackForwardList)
    299311{
    300     if (!m_frame->page())
     312    if (!shouldScheduleNavigation(url))
    301313        return;
    302314    if (url.isEmpty())
     
    345357void NavigationScheduler::scheduleRefresh()
    346358{
    347     if (!m_frame->page())
     359    if (!shouldScheduleNavigation())
    348360        return;
    349361    const KURL& url = m_frame->loader()->url();
     
    356368void NavigationScheduler::scheduleHistoryNavigation(int steps)
    357369{
    358     if (!m_frame->page())
     370    if (!shouldScheduleNavigation())
    359371        return;
    360372
  • trunk/WebCore/loader/NavigationScheduler.h

    r73444 r75305  
    3535#include "Timer.h"
    3636#include <wtf/Forward.h>
     37#include <wtf/Noncopyable.h>
    3738#include <wtf/OwnPtr.h>
    3839#include <wtf/PassOwnPtr.h>
     
    4950struct FrameLoadRequest;
    5051
    51 class NavigationScheduler : public Noncopyable {
     52class NavigationDisablerForBeforeUnload {
     53    WTF_MAKE_NONCOPYABLE(NavigationDisablerForBeforeUnload);
     54
     55public:
     56    NavigationDisablerForBeforeUnload()
     57    {
     58        s_navigationDisableCount++;
     59    }
     60    ~NavigationDisablerForBeforeUnload()
     61    {
     62        ASSERT(s_navigationDisableCount);
     63        s_navigationDisableCount--;
     64    }
     65    static bool isNavigationAllowed() { return !s_navigationDisableCount; }
     66
     67private:
     68    static unsigned s_navigationDisableCount;
     69};
     70
     71class NavigationScheduler {
     72    WTF_MAKE_NONCOPYABLE(NavigationScheduler);
     73
    5274public:
    5375    NavigationScheduler(Frame*);
     
    6991
    7092private:
     93    bool shouldScheduleNavigation() const;
     94    bool shouldScheduleNavigation(const String& url) const;
     95
    7196    void timerFired(Timer<NavigationScheduler>*);
    7297    void schedule(PassOwnPtr<ScheduledNavigation>);
Note: See TracChangeset for help on using the changeset viewer.