Commit f53381bf authored by darin@apple.com's avatar darin@apple.com

Crash when website does a history.back() followed by an alert()

https://bugs.webkit.org/show_bug.cgi?id=29686
rdar://problem/6984996

Patch by Darin Adler <darin@apple.com> on 2009-09-23
Reviewed by Sam Weinig.

When loading is deferred, we need to defer timer-based loads
too, not just networking-driven loads. Otherwise we can get
syncronouse navigation while running a script, which leads to
crashes and other badness.

This patch includes a manual test; an automated test may be
possible some time in the future.

* dom/Document.cpp:
(WebCore::Document::processHttpEquiv): Use scheduleLocationChange
instead of scheduleHTTPRedirection to implement the navigation
needed for x-frame-options.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader): Updated for data members with
new names and new data members.
(WebCore::FrameLoader::setDefersLoading): When turning deferral
off, call startRedirectionTimer and startCheckCompleteTimer, since
either of them might have been fired and ignored while defersLoading
was true.
(WebCore::FrameLoader::clear): Updated for replacement of the
m_checkCompletedTimer and m_checkLoadCompleteTimer timers.
(WebCore::FrameLoader::allAncestorsAreComplete): Added.
(WebCore::FrameLoader::checkCompleted): Added code to set
m_shouldCallCheckCompleted to false. Changed code that calls
startRedirectionTimer to call it unconditionally, since that
function now knows when to do work and doesn't expect callers
to handle that any more.
(WebCore::FrameLoader::checkTimerFired): Added. Replaces the old
timer fired callbacks. Calls checkCompleted and checkLoadComplete
as appropriate, but not when defersLoading is true.
(WebCore::FrameLoader::startCheckCompleteTimer): Added. Replaces
the two different calls to start timers before. Only starts the
timers if they are needed.
(WebCore::FrameLoader::scheduleCheckCompleted): Changed to call
startCheckCompleteTimer after setting boolean.
(WebCore::FrameLoader::scheduleCheckLoadComplete): Ditto.
(WebCore::FrameLoader::scheduleHistoryNavigation): Removed
canGoBackOrForward check. The logic works more naturally when
we don't do anything until the timer fires.
(WebCore::FrameLoader::redirectionTimerFired): Do nothing if
defersLoading is true. Also moved canGoBackOrForward check here.
(WebCore::FrameLoader::scheduleRedirection): Changed code that
calls startRedirectionTimer to do so unconditionally. That
function now handles the rules about when to start the timer
rather than expecting the caller to do so.
(WebCore::FrameLoader::startRedirectionTimer): Added code to
handle the case where there is no redirection scheduled,
where the timer is already active, or where this is a classic
redirection and there is an ancestor that has not yet completed
loading.
(WebCore::FrameLoader::completed): Call startRedirectionTimer
here directly instead of calling a cover named parentCompleted.
Hooray! One less function in the giant FrameLoader class!
(WebCore::FrameLoader::checkLoadComplete): Added code to set
m_shouldCallCheckLoadComplete to false.

* loader/FrameLoader.h: Replaced the two functions
checkCompletedTimerFired and checkLoadCompleteTimerFired with
one function, checkTimerFired. Removed the parentCompleted
function. Added the startCheckCompleteTimer and
allAncestorsAreComplete functions. Replaced the
m_checkCompletedTimer and m_checkLoadCompleteTimer data
members with m_checkTimer, m_shouldCallCheckCompleted, and
m_shouldCallCheckLoadComplete.

* manual-tests/go-back-after-alert.html: Added.
* manual-tests/resources/alert-and-go-back.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@48687 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 39faa029
2009-09-23 Darin Adler <darin@apple.com>
Reviewed by Sam Weinig.
Crash when website does a history.back() followed by an alert()
https://bugs.webkit.org/show_bug.cgi?id=29686
rdar://problem/6984996
When loading is deferred, we need to defer timer-based loads
too, not just networking-driven loads. Otherwise we can get
syncronouse navigation while running a script, which leads to
crashes and other badness.
This patch includes a manual test; an automated test may be
possible some time in the future.
* dom/Document.cpp:
(WebCore::Document::processHttpEquiv): Use scheduleLocationChange
instead of scheduleHTTPRedirection to implement the navigation
needed for x-frame-options.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::FrameLoader): Updated for data members with
new names and new data members.
(WebCore::FrameLoader::setDefersLoading): When turning deferral
off, call startRedirectionTimer and startCheckCompleteTimer, since
either of them might have been fired and ignored while defersLoading
was true.
(WebCore::FrameLoader::clear): Updated for replacement of the
m_checkCompletedTimer and m_checkLoadCompleteTimer timers.
(WebCore::FrameLoader::allAncestorsAreComplete): Added.
(WebCore::FrameLoader::checkCompleted): Added code to set
m_shouldCallCheckCompleted to false. Changed code that calls
startRedirectionTimer to call it unconditionally, since that
function now knows when to do work and doesn't expect callers
to handle that any more.
(WebCore::FrameLoader::checkTimerFired): Added. Replaces the old
timer fired callbacks. Calls checkCompleted and checkLoadComplete
as appropriate, but not when defersLoading is true.
(WebCore::FrameLoader::startCheckCompleteTimer): Added. Replaces
the two different calls to start timers before. Only starts the
timers if they are needed.
(WebCore::FrameLoader::scheduleCheckCompleted): Changed to call
startCheckCompleteTimer after setting boolean.
(WebCore::FrameLoader::scheduleCheckLoadComplete): Ditto.
(WebCore::FrameLoader::scheduleHistoryNavigation): Removed
canGoBackOrForward check. The logic works more naturally when
we don't do anything until the timer fires.
(WebCore::FrameLoader::redirectionTimerFired): Do nothing if
defersLoading is true. Also moved canGoBackOrForward check here.
(WebCore::FrameLoader::scheduleRedirection): Changed code that
calls startRedirectionTimer to do so unconditionally. That
function now handles the rules about when to start the timer
rather than expecting the caller to do so.
(WebCore::FrameLoader::startRedirectionTimer): Added code to
handle the case where there is no redirection scheduled,
where the timer is already active, or where this is a classic
redirection and there is an ancestor that has not yet completed
loading.
(WebCore::FrameLoader::completed): Call startRedirectionTimer
here directly instead of calling a cover named parentCompleted.
Hooray! One less function in the giant FrameLoader class!
(WebCore::FrameLoader::checkLoadComplete): Added code to set
m_shouldCallCheckLoadComplete to false.
* loader/FrameLoader.h: Replaced the two functions
checkCompletedTimerFired and checkLoadCompleteTimerFired with
one function, checkTimerFired. Removed the parentCompleted
function. Added the startCheckCompleteTimer and
allAncestorsAreComplete functions. Replaced the
m_checkCompletedTimer and m_checkLoadCompleteTimer data
members with m_checkTimer, m_shouldCallCheckCompleted, and
m_shouldCallCheckLoadComplete.
* manual-tests/go-back-after-alert.html: Added.
* manual-tests/resources/alert-and-go-back.html: Added.
2009-09-23 David Kilzer <ddkilzer@apple.com>
<http://webkit.org/b/29660> Move "Generate 64-bit Export File" build phase script into DerivedSources.make
......@@ -2173,7 +2173,7 @@ void Document::processHttpEquiv(const String& equiv, const String& content)
FrameLoader* frameLoader = frame->loader();
if (frameLoader->shouldInterruptLoadForXFrameOptions(content, url())) {
frameLoader->stopAllLoaders();
frameLoader->scheduleHTTPRedirection(0, blankURL());
frameLoader->scheduleLocationChange(blankURL(), String());
}
}
}
......
......@@ -272,8 +272,9 @@ FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
, m_encodingWasChosenByUser(false)
, m_containsPlugIns(false)
, m_redirectionTimer(this, &FrameLoader::redirectionTimerFired)
, m_checkCompletedTimer(this, &FrameLoader::checkCompletedTimerFired)
, m_checkLoadCompleteTimer(this, &FrameLoader::checkLoadCompleteTimerFired)
, m_checkTimer(this, &FrameLoader::checkTimerFired)
, m_shouldCallCheckCompleted(false)
, m_shouldCallCheckLoadComplete(false)
, m_opener(0)
, m_openedByDOM(false)
, m_creatingInitialEmptyDocument(false)
......@@ -323,6 +324,11 @@ void FrameLoader::setDefersLoading(bool defers)
m_provisionalDocumentLoader->setDefersLoading(defers);
if (m_policyDocumentLoader)
m_policyDocumentLoader->setDefersLoading(defers);
if (!defers) {
startRedirectionTimer();
startCheckCompleteTimer();
}
}
Frame* FrameLoader::createWindow(FrameLoader* frameLoaderForFrameLookup, const FrameLoadRequest& request, const WindowFeatures& features, bool& created)
......@@ -849,8 +855,9 @@ void FrameLoader::clear(bool clearWindowProperties, bool clearScriptObjects, boo
m_redirectionTimer.stop();
m_scheduledRedirection.clear();
m_checkCompletedTimer.stop();
m_checkLoadCompleteTimer.stop();
m_checkTimer.stop();
m_shouldCallCheckCompleted = false;
m_shouldCallCheckLoadComplete = false;
m_receivedData = false;
m_isDisplayingInitialEmptyDocument = false;
......@@ -1252,8 +1259,19 @@ bool FrameLoader::allChildrenAreComplete() const
return true;
}
bool FrameLoader::allAncestorsAreComplete() const
{
for (Frame* ancestor = m_frame; ancestor; ancestor = ancestor->tree()->parent()) {
if (!ancestor->loader()->m_isComplete)
return false;
}
return true;
}
void FrameLoader::checkCompleted()
{
m_shouldCallCheckCompleted = false;
if (m_frame->view())
m_frame->view()->checkStopDelayingDeferredRepaints();
......@@ -1279,38 +1297,44 @@ void FrameLoader::checkCompleted()
RefPtr<Frame> protect(m_frame);
checkCallImplicitClose(); // if we didn't do it before
// Do not start a redirection timer for subframes here.
// That is deferred until the parent is completed.
if (m_scheduledRedirection && !m_frame->tree()->parent())
startRedirectionTimer();
startRedirectionTimer();
completed();
if (m_frame->page())
checkLoadComplete();
}
void FrameLoader::checkCompletedTimerFired(Timer<FrameLoader>*)
void FrameLoader::checkTimerFired(Timer<FrameLoader>*)
{
checkCompleted();
if (Page* page = m_frame->page()) {
if (page->defersLoading())
return;
}
if (m_shouldCallCheckCompleted)
checkCompleted();
if (m_shouldCallCheckLoadComplete)
checkLoadComplete();
}
void FrameLoader::scheduleCheckCompleted()
void FrameLoader::startCheckCompleteTimer()
{
if (!m_checkCompletedTimer.isActive())
m_checkCompletedTimer.startOneShot(0);
if (!(m_shouldCallCheckCompleted || m_shouldCallCheckLoadComplete))
return;
if (m_checkTimer.isActive())
return;
m_checkTimer.startOneShot(0);
}
void FrameLoader::checkLoadCompleteTimerFired(Timer<FrameLoader>*)
void FrameLoader::scheduleCheckCompleted()
{
if (!m_frame->page())
return;
checkLoadComplete();
m_shouldCallCheckCompleted = true;
startCheckCompleteTimer();
}
void FrameLoader::scheduleCheckLoadComplete()
{
if (!m_checkLoadCompleteTimer.isActive())
m_checkLoadCompleteTimer.startOneShot(0);
m_shouldCallCheckLoadComplete = true;
startCheckCompleteTimer();
}
void FrameLoader::checkCallImplicitClose()
......@@ -1439,12 +1463,6 @@ void FrameLoader::scheduleHistoryNavigation(int steps)
if (!m_frame->page())
return;
// navigation will always be allowed in the 0 steps case, which is OK because that's supposed to force a reload.
if (!canGoBackOrForward(steps)) {
cancelRedirection();
return;
}
scheduleRedirection(new ScheduledRedirection(steps));
}
......@@ -1482,6 +1500,9 @@ void FrameLoader::redirectionTimerFired(Timer<FrameLoader>*)
{
ASSERT(m_frame->page());
if (m_frame->page()->defersLoading())
return;
OwnPtr<ScheduledRedirection> redirection(m_scheduledRedirection.release());
switch (redirection->type) {
......@@ -1498,7 +1519,8 @@ void FrameLoader::redirectionTimerFired(Timer<FrameLoader>*)
}
// go(i!=0) from a frame navigates into the history of the frame only,
// in both IE and NS (but not in Mozilla). We can't easily do that.
goBackOrForward(redirection->historySteps);
if (canGoBackOrForward(redirection->historySteps))
goBackOrForward(redirection->historySteps);
return;
case ScheduledRedirection::formSubmission:
// The submitForm function will find a target frame before using the redirection timer.
......@@ -1722,12 +1744,6 @@ bool FrameLoader::loadPlugin(RenderPart* renderer, const KURL& url, const String
return widget != 0;
}
void FrameLoader::parentCompleted()
{
if (m_scheduledRedirection && !m_redirectionTimer.isActive())
startRedirectionTimer();
}
String FrameLoader::outgoingReferrer() const
{
return m_outgoingReferrer;
......@@ -2139,16 +2155,22 @@ void FrameLoader::scheduleRedirection(PassOwnPtr<ScheduledRedirection> redirecti
m_scheduledRedirection = redirection;
if (!m_isComplete && m_scheduledRedirection->type != ScheduledRedirection::redirection)
completed();
if (m_isComplete || m_scheduledRedirection->type != ScheduledRedirection::redirection)
startRedirectionTimer();
startRedirectionTimer();
}
void FrameLoader::startRedirectionTimer()
{
if (!m_scheduledRedirection)
return;
ASSERT(m_frame->page());
ASSERT(m_scheduledRedirection);
m_redirectionTimer.stop();
if (m_redirectionTimer.isActive())
return;
if (m_scheduledRedirection->type == ScheduledRedirection::redirection && !allAncestorsAreComplete())
return;
m_redirectionTimer.startOneShot(m_scheduledRedirection->delay);
switch (m_scheduledRedirection->type) {
......@@ -2187,7 +2209,7 @@ void FrameLoader::completed()
{
RefPtr<Frame> protect(m_frame);
for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())
child->loader()->parentCompleted();
child->loader()->startRedirectionTimer();
if (Frame* parent = m_frame->tree()->parent())
parent->loader()->checkCompleted();
if (m_frame->view())
......@@ -3559,6 +3581,8 @@ void FrameLoader::checkLoadComplete()
{
ASSERT(m_client->hasWebView());
m_shouldCallCheckLoadComplete = false;
// FIXME: Always traversing the entire frame tree is a bit inefficient, but
// is currently needed in order to null out the previous history item for all frames.
if (Page* page = m_frame->page())
......
......@@ -412,15 +412,13 @@ namespace WebCore {
void updateHistoryForAnchorScroll();
void redirectionTimerFired(Timer<FrameLoader>*);
void checkCompletedTimerFired(Timer<FrameLoader>*);
void checkLoadCompleteTimerFired(Timer<FrameLoader>*);
void checkTimerFired(Timer<FrameLoader>*);
void cancelRedirection(bool newLoadInProgress = false);
void started();
void completed();
void parentCompleted();
bool shouldUsePlugin(const KURL&, const String& mimeType, bool hasFallback, bool& useFallback);
bool loadPlugin(RenderPart*, const KURL&, const String& mimeType,
......@@ -539,6 +537,7 @@ namespace WebCore {
void scheduleCheckCompleted();
void scheduleCheckLoadComplete();
void startCheckCompleteTimer();
KURL originalRequestURL() const;
......@@ -546,6 +545,7 @@ namespace WebCore {
void saveScrollPositionAndViewStateToItem(HistoryItem*);
bool allAncestorsAreComplete() const; // including this
bool allChildrenAreComplete() const; // immediate children, not all descendants
Frame* m_frame;
......@@ -612,8 +612,9 @@ namespace WebCore {
KURL m_submittedFormURL;
Timer<FrameLoader> m_redirectionTimer;
Timer<FrameLoader> m_checkCompletedTimer;
Timer<FrameLoader> m_checkLoadCompleteTimer;
Timer<FrameLoader> m_checkTimer;
bool m_shouldCallCheckCompleted;
bool m_shouldCallCheckLoadComplete;
Frame* m_opener;
HashSet<Frame*> m_openedFrames;
......
<html>
<body>
This tests a bug where going back just before putting up an alert can lead to a crash.
<hr>
<a href="resources/alert-and-go-back.html">Click this link to run the test.</a>
</body>
</html>
<script>
history.back();
alert("Wait a moment and then dismiss this alert. If there is no crash, the test succeeded.");
</script>
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