[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75
darin at apple.com
darin at apple.com
Thu Oct 29 20:32:40 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit f53381bf0d4abdd9579ece0d8a76d13b6b231f53
Author: darin at apple.com <darin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Sep 23 23:27:01 2009 +0000
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 at 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
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index a6e5215..2eee0cb 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,80 @@
+2009-09-23 Darin Adler <darin at 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 at apple.com>
<http://webkit.org/b/29660> Move "Generate 64-bit Export File" build phase script into DerivedSources.make
diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index f26ee87..57e22e1 100644
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -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());
}
}
}
diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index 52b6d77..f48c607 100644
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -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())
diff --git a/WebCore/loader/FrameLoader.h b/WebCore/loader/FrameLoader.h
index f66e7d2..4b4959b 100644
--- a/WebCore/loader/FrameLoader.h
+++ b/WebCore/loader/FrameLoader.h
@@ -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;
diff --git a/WebCore/manual-tests/go-back-after-alert.html b/WebCore/manual-tests/go-back-after-alert.html
new file mode 100644
index 0000000..b3a1e1a
--- /dev/null
+++ b/WebCore/manual-tests/go-back-after-alert.html
@@ -0,0 +1,7 @@
+<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>
diff --git a/WebCore/manual-tests/resources/alert-and-go-back.html b/WebCore/manual-tests/resources/alert-and-go-back.html
new file mode 100644
index 0000000..55a5b50
--- /dev/null
+++ b/WebCore/manual-tests/resources/alert-and-go-back.html
@@ -0,0 +1,4 @@
+<script>
+history.back();
+alert("Wait a moment and then dismiss this alert. If there is no crash, the test succeeded.");
+</script>
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list