[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-34-g43a6bb2
Gustavo Noronha Silva
gustavo.noronha at collabora.co.uk
Wed Oct 7 06:25:49 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit 65dc47e6234e9d28773738aeaff06ead7d776b95
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/dom/Document.cpp b/WebCore/dom/Document.cpp
index 0a57972..ccfef8d 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 2b886ca..fb95361 100644
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -267,8 +267,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)
@@ -318,6 +319,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)
@@ -844,8 +850,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;
@@ -1238,8 +1245,28 @@ void FrameLoader::loadDone()
checkCompleted();
}
+bool FrameLoader::allChildrenAreComplete() const
+{
+ for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling()) {
+ if (!child->loader()->m_isComplete)
+ return false;
+ }
+ 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();
@@ -1266,38 +1293,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()
@@ -1427,12 +1460,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));
}
@@ -1470,6 +1497,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) {
@@ -1486,7 +1516,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.
@@ -1710,12 +1741,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;
@@ -2127,16 +2152,22 @@ void FrameLoader::scheduleRedirection(ScheduledRedirection* redirection)
m_scheduledRedirection.set(redirection);
if (!m_isComplete && redirection->type != ScheduledRedirection::redirection)
completed();
- if (m_isComplete || redirection->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) {
@@ -2189,7 +2220,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())
@@ -3560,6 +3591,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 26206e6..93576c9 100644
--- a/WebCore/loader/FrameLoader.h
+++ b/WebCore/loader/FrameLoader.h
@@ -411,15 +411,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,
@@ -538,6 +536,7 @@ namespace WebCore {
void scheduleCheckCompleted();
void scheduleCheckLoadComplete();
+ void startCheckCompleteTimer();
KURL originalRequestURL() const;
@@ -545,6 +544,9 @@ namespace WebCore {
void saveScrollPositionAndViewStateToItem(HistoryItem*);
+ bool allAncestorsAreComplete() const; // including this
+ bool allChildrenAreComplete() const; // immediate children, not all descendants
+
Frame* m_frame;
FrameLoaderClient* m_client;
@@ -609,8 +611,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