Commit 3d613f1f authored by ggaren's avatar ggaren

WebCore:

        Added test case for <rdar://problem/4268278> Submitting a form in onUnload event
        handler causes crash in -[WebDataSource(WebPrivate) _commitIfReady:]

        * manual-tests/onunload-form-submit-crash.html: Added.

WebKit:

        Reviewed by Eric.

        Manual testcase added:
        WebCore/manual-tests/onunload-form-submit-crash.html

        - Fixed <rdar://problem/4268278> Submitting a form in onUnload event
        handler causes crash in -[WebDataSource(WebPrivate) _commitIfReady:]

        The problem is that the form submission in the unload event kicks off
        a new load in the midst of the load that caused the unload event to
        fire in the first place, so the two loads stomp each other.

        The solution is to cancel the first load and let the unload handler's
        load win. (Firefox does the same.)

        * WebView.subproj/WebFrame.m:
        (-[WebFrame _transitionToCommitted:]): Moved call to -closeURL up
        the call stack to _continueLoadRequest. (See below.) This has the
        side-effect of always firing the unload event, even if the new
        datasource never becomes committed, which seems like a good thing.

        (-[WebFrame _continueLoadRequestAfterNavigationPolicy:formState:]):
        Call -closeURL here, instead of in _transitionToCommitted,  so that the
        unload handler can fire before we initialize any part of the load.

        Check provisionalDataSource for nil to discover if the unload event
        kicked off its own load.

        Cleared up some coments.

        (-[WebFrame _detachFromParent]):
        It turns out that if you close the window instead of just navigating
        to a new page, you get an alternate assertion failure/crash because
        the load kicked off by the unload event handler generates resource
        loader callbacks after the associated WebFrame/WebView has disappeared.

        The nifty solution here is just to reverse the order of calls to
        -stopLoading and -closeURL, thus guaranteeing that -stopLoading has the
        last word when you close a window.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@11841 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 79f068c6
2005-12-29 Geoffrey Garen <ggaren@apple.com>
Added test case for <rdar://problem/4268278> Submitting a form in onUnload event
handler causes crash in -[WebDataSource(WebPrivate) _commitIfReady:]
* manual-tests/onunload-form-submit-crash.html: Added.
2005-12-30 Anders Carlsson <andersca@mac.com>
Reviewed by Eric.
......@@ -195,6 +202,7 @@
* ksvg2/svg/SVGRectElementImpl.cpp:
(SVGRectElementImpl::toPathData): fixed round rect calculations
>>>>>>> 1.66
2005-12-29 Mark Rowe <opendarwin.org@bdash.net.nz>
Reviewed by eseidel, ggaren, darin.
......
<html>
<body onUnload="document.myForm.submit()">
<a href="" onClick="location.href=location.href; return false;">Click here and see if Safari crashes.</a>
<p>Still with me? Now close the window and see if Safari crashes.</p>
<form name="myForm">
</form>
</body>
<html>
2005-12-29 Geoffrey Garen <ggaren@apple.com>
Reviewed by Eric.
Manual testcase added:
WebCore/manual-tests/onunload-form-submit-crash.html
- Fixed <rdar://problem/4268278> Submitting a form in onUnload event
handler causes crash in -[WebDataSource(WebPrivate) _commitIfReady:]
The problem is that the form submission in the unload event kicks off
a new load in the midst of the load that caused the unload event to
fire in the first place, so the two loads stomp each other.
The solution is to cancel the first load and let the unload handler's
load win. (Firefox does the same.)
* WebView.subproj/WebFrame.m:
(-[WebFrame _transitionToCommitted:]): Moved call to -closeURL up
the call stack to _continueLoadRequest. (See below.) This has the
side-effect of always firing the unload event, even if the new
datasource never becomes committed, which seems like a good thing.
(-[WebFrame _continueLoadRequestAfterNavigationPolicy:formState:]):
Call -closeURL here, instead of in _transitionToCommitted, so that the
unload handler can fire before we initialize any part of the load.
Check provisionalDataSource for nil to discover if the unload event
kicked off its own load.
Cleared up some coments.
(-[WebFrame _detachFromParent]):
It turns out that if you close the window instead of just navigating
to a new page, you get an alternate assertion failure/crash because
the load kicked off by the unload event handler generates resource
loader callbacks after the associated WebFrame/WebView has disappeared.
The nifty solution here is just to reverse the order of calls to
-stopLoading and -closeURL, thus guaranteeing that -stopLoading has the
last word when you close a window.
2005-12-30 Mitz Pettel <opendarwin.org@mitzpettel.com>
Reviewed by Eric, committed by Maciej.
......@@ -691,11 +691,10 @@ - (void)_detachFromParent
{
WebBridge *bridge = _private->bridge;
[self stopLoading];
[self _saveScrollPositionAndViewStateToItem:[_private currentItem]];
[bridge closeURL];
[self stopLoading];
[self _saveScrollPositionAndViewStateToItem:[_private currentItem]];
[self _detachChildren];
[_private setWebView:nil];
......@@ -851,8 +850,6 @@ - (void)_transitionToCommitted: (NSDictionary *)pageCache
[_private setProvisionalItem:nil];
}
[[self _bridge] closeURL];
// Set the committed data source on the frame.
[self _setDataSource:_private->provisionalDataSource];
......@@ -2400,21 +2397,36 @@ -(void)_continueLoadRequestAfterNavigationPolicy:(NSURLRequest *)request formSta
WebHistoryItem *item = [_private provisionalItem];
// Two reasons we can't continue:
// 1) navigation policy delegate said we can't so request is nil.
// 2) before unload event handler caused an alert to come up and the user said cancel.
// 1) Navigation policy delegate said we can't so request is nil. A primary case of this
// is the user responding Cancel to the form repost nag sheet.
// 2) User responded Cancel to an alert popped up by the before unload event handler.
// The "before unload" event handler runs only for the main frame.
BOOL canContinue = request && ([[self webView] mainFrame] != self || [_private->bridge shouldClose]);
// The call to closeURL below invokes the unload event handler, which can execute arbitrary
// JavaScript. If the script initiates a new load, we need to cancel the requested load,
// or the two will stomp each other.
// Cache the load type of the current request, since _private->policyLoadType will change
// if the unload handler initiates a load. (We use this value to determine if we need to
// reset the back/forward cursor.)
WebFrameLoadType requestLoadType = _private->policyLoadType;
if (canContinue) {
[self stopLoading];
[[self _bridge] closeURL];
canContinue = [self provisionalDataSource] == nil;
}
if (!canContinue) {
[self _setPolicyDataSource:nil];
// If the delegate punts on the navigation, we have the problem that we have optimistically moved
// the b/f cursor already, so move it back. For sanity we only do this if the navigation of the
// target frame or top-level frame is canceled. A primary case of this is the user responding
// Cancel to the form repost nag sheet.
// If the navigation request came from the back/forward menu, and we punt on it, we have the
// problem that we have optimistically moved the b/f cursor already, so move it back. For sanity,
// we only do this when punting a navigation for the target frame or top-level frame.
if (([item isTargetItem] || [[self webView] mainFrame] == self)
&& (_private->policyLoadType == WebFrameLoadTypeForward
|| _private->policyLoadType == WebFrameLoadTypeBack
|| _private->policyLoadType == WebFrameLoadTypeIndexedBackForward))
&& (requestLoadType == WebFrameLoadTypeForward
|| requestLoadType == WebFrameLoadTypeBack
|| requestLoadType == WebFrameLoadTypeIndexedBackForward))
[[[self webView] mainFrame] _resetBackForwardList];
return;
}
......@@ -2422,7 +2434,6 @@ -(void)_continueLoadRequestAfterNavigationPolicy:(NSURLRequest *)request formSta
WebFrameLoadType loadType = _private->policyLoadType;
WebDataSource *dataSource = [_private->policyDataSource retain];
[self stopLoading];
[self _setLoadType:loadType];
[self _setProvisionalDataSource:dataSource];
[dataSource release];
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment