[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
mihaip at chromium.org
mihaip at chromium.org
Wed Dec 22 15:24:20 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 5f3cb7843cf135727f89c45f79cbe60d25021b4a
Author: mihaip at chromium.org <mihaip at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Nov 2 21:23:43 2010 +0000
2010-11-02 Mihai Parparita <mihaip at chromium.org>
Reviewed by Adam Barth.
[Chromium] Crash when encountering history.back() call during Page::goToItem execution
https://bugs.webkit.org/show_bug.cgi?id=48477
Add a reduced version of the page that was originally reported at
http://crbug.com/59554.
* http/tests/history/back-during-onload-triggered-by-back-expected.txt: Added.
* http/tests/history/back-during-onload-triggered-by-back.html: Added.
* http/tests/history/resources/back-during-onload-container.html: Added.
* http/tests/history/resources/back-during-onload-hung-page.php: Added.
* http/tests/history/resources/back-during-onload-middle.html: Added.
2010-11-02 Mihai Parparita <mihaip at chromium.org>
Reviewed by Adam Barth.
[Chromium] Crash when encountering history.back() call during Page::goToItem execution
https://bugs.webkit.org/show_bug.cgi?id=48477
For the Chromium port, BackForwardList::itemAtIndex synthesizes a
HistoryItem and saves a pointer to it in m_pendingItem. During
Page::goToItem we call FrameLoader::stopAllLoaders, which can trigger
onload handlers (if a subframe was not considered committed by the frame
loader). If one of those handlers calls calls history.back() or another
operation that ends up in NavigationScheduler::scheduleHistoryNavigation,
we would call BackForwardList::itemAtIndex, which means that we would
lose the m_pendingItem RefPtr that pointed to the item being navigated
to, causing its ref count to go to 0*, and thus for the HistoryItem to
be deleted before we were done navigating to it.
This is fixed in two ways:
- Add a protector RefPtr in Page::goToItem to make sure that the item is
still around for when we pass it to HistoryController:goToItem.
- Change NavigationScheduler::scheduleHistoryNavigation to not use
BackForwardList::itemAtIndex and instead look at the
forward/backListCount() (since it doesn't actually care about the
returned HistoryItem).
* Full annotated stack trace of this is at http://crbug.com/59554#c9.
Test: http/tests/history/back-during-onload-triggered-by-back.html
* loader/NavigationScheduler.cpp:
(WebCore::NavigationScheduler::scheduleHistoryNavigation):
* page/Page.cpp:
(WebCore::Page::goToItem):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71170 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index cf4f8e1..4dc1951 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2010-11-02 Mihai Parparita <mihaip at chromium.org>
+
+ Reviewed by Adam Barth.
+
+ [Chromium] Crash when encountering history.back() call during Page::goToItem execution
+ https://bugs.webkit.org/show_bug.cgi?id=48477
+
+ Add a reduced version of the page that was originally reported at
+ http://crbug.com/59554.
+
+ * http/tests/history/back-during-onload-triggered-by-back-expected.txt: Added.
+ * http/tests/history/back-during-onload-triggered-by-back.html: Added.
+ * http/tests/history/resources/back-during-onload-container.html: Added.
+ * http/tests/history/resources/back-during-onload-hung-page.php: Added.
+ * http/tests/history/resources/back-during-onload-middle.html: Added.
+
2010-11-02 Dmitry Titov <dimich at chromium.org>
[Chromium] Unreviewed update of test expectations.
diff --git a/LayoutTests/http/tests/history/back-during-onload-triggered-by-back-expected.txt b/LayoutTests/http/tests/history/back-during-onload-triggered-by-back-expected.txt
new file mode 100644
index 0000000..3b416e9
--- /dev/null
+++ b/LayoutTests/http/tests/history/back-during-onload-triggered-by-back-expected.txt
@@ -0,0 +1,10 @@
+CONSOLE MESSAGE: line 23: Starting test.
+Tests that an onload handler that runs history.back() that's triggered by a history.back() doesn't crash the browser (see http://crbug.com/59554 for more details).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/history/back-during-onload-triggered-by-back.html b/LayoutTests/http/tests/history/back-during-onload-triggered-by-back.html
new file mode 100644
index 0000000..eeca21d
--- /dev/null
+++ b/LayoutTests/http/tests/history/back-during-onload-triggered-by-back.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <link rel="stylesheet" href="../../js-test-resources/js-test-style.css">
+ <script src="../../js-test-resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+description('Tests that an onload handler that runs history.back() that\'s triggered by a history.back() doesn\'t crash the browser (see http://crbug.com/59554 for more details).');
+
+onload = function()
+{
+ if (window.localStorage.started) {
+ delete window.localStorage.started;
+ finishJSTest();
+ } else {
+ // To make sure that we hit this branch, log this to the console so that
+ // it shows up in expected output (debug() will be blown away once we
+ // navigate out).
+ console.log('Starting test.');
+ window.localStorage.started = true;
+ // Navigate in a timeout to make sure we create a history entry.
+ setTimeout(function() {
+ window.location.href = 'resources/back-during-onload-container.html';
+ }, 0);
+ }
+};
+
+var successfullyParsed = true;
+var jsTestIsAsync = true;
+</script>
+
+<script src="../../js-test-resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/history/resources/back-during-onload-container.html b/LayoutTests/http/tests/history/resources/back-during-onload-container.html
new file mode 100644
index 0000000..398f16a
--- /dev/null
+++ b/LayoutTests/http/tests/history/resources/back-during-onload-container.html
@@ -0,0 +1,8 @@
+<script>
+onload = function() {
+ history.back();
+};
+</script>
+container
+<iframe src="back-during-onload-middle.html"></iframe>
+<script>history.back();</script>
\ No newline at end of file
diff --git a/LayoutTests/http/tests/history/resources/back-during-onload-hung-page.php b/LayoutTests/http/tests/history/resources/back-during-onload-hung-page.php
new file mode 100644
index 0000000..6031615
--- /dev/null
+++ b/LayoutTests/http/tests/history/resources/back-during-onload-hung-page.php
@@ -0,0 +1,2 @@
+<? sleep(1000); ?>
+FAIL: Should never see this
\ No newline at end of file
diff --git a/LayoutTests/http/tests/history/resources/back-during-onload-middle.html b/LayoutTests/http/tests/history/resources/back-during-onload-middle.html
new file mode 100644
index 0000000..6035e40
--- /dev/null
+++ b/LayoutTests/http/tests/history/resources/back-during-onload-middle.html
@@ -0,0 +1,2 @@
+middle<br>
+<iframe src="back-during-onload-hung-page.php"></iframe>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 1ec2c30..e08773b 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,38 @@
+2010-11-02 Mihai Parparita <mihaip at chromium.org>
+
+ Reviewed by Adam Barth.
+
+ [Chromium] Crash when encountering history.back() call during Page::goToItem execution
+ https://bugs.webkit.org/show_bug.cgi?id=48477
+
+ For the Chromium port, BackForwardList::itemAtIndex synthesizes a
+ HistoryItem and saves a pointer to it in m_pendingItem. During
+ Page::goToItem we call FrameLoader::stopAllLoaders, which can trigger
+ onload handlers (if a subframe was not considered committed by the frame
+ loader). If one of those handlers calls calls history.back() or another
+ operation that ends up in NavigationScheduler::scheduleHistoryNavigation,
+ we would call BackForwardList::itemAtIndex, which means that we would
+ lose the m_pendingItem RefPtr that pointed to the item being navigated
+ to, causing its ref count to go to 0*, and thus for the HistoryItem to
+ be deleted before we were done navigating to it.
+
+ This is fixed in two ways:
+ - Add a protector RefPtr in Page::goToItem to make sure that the item is
+ still around for when we pass it to HistoryController:goToItem.
+ - Change NavigationScheduler::scheduleHistoryNavigation to not use
+ BackForwardList::itemAtIndex and instead look at the
+ forward/backListCount() (since it doesn't actually care about the
+ returned HistoryItem).
+
+ * Full annotated stack trace of this is at http://crbug.com/59554#c9.
+
+ Test: http/tests/history/back-during-onload-triggered-by-back.html
+
+ * loader/NavigationScheduler.cpp:
+ (WebCore::NavigationScheduler::scheduleHistoryNavigation):
+ * page/Page.cpp:
+ (WebCore::Page::goToItem):
+
2010-10-28 Zhenyao Mo <zmo at google.com>
Reviewed by Kenneth Russell.
diff --git a/WebCore/loader/NavigationScheduler.cpp b/WebCore/loader/NavigationScheduler.cpp
index 66e6bac..28fda9a 100644
--- a/WebCore/loader/NavigationScheduler.cpp
+++ b/WebCore/loader/NavigationScheduler.cpp
@@ -352,8 +352,8 @@ void NavigationScheduler::scheduleHistoryNavigation(int steps)
// Invalid history navigations (such as history.forward() during a new load) have the side effect of cancelling any scheduled
// redirects. We also avoid the possibility of cancelling the current load by avoiding the scheduled redirection altogether.
- HistoryItem* specifiedEntry = m_frame->page()->backForward()->itemAtIndex(steps);
- if (!specifiedEntry) {
+ BackForwardController* backForward = m_frame->page()->backForward();
+ if (steps > backForward->forwardCount() || -steps > backForward->backCount()) {
cancel();
return;
}
diff --git a/WebCore/page/Page.cpp b/WebCore/page/Page.cpp
index 3b546e9..746e53e 100644
--- a/WebCore/page/Page.cpp
+++ b/WebCore/page/Page.cpp
@@ -347,6 +347,10 @@ void Page::goToItem(HistoryItem* item, FrameLoadType type)
{
if (defersLoading())
return;
+
+ // stopAllLoaders may end up running onload handlers, which could cause further history traversals that may lead to the passed in HistoryItem
+ // being deref()-ed. Make sure we can still use it with HistoryController::goToItem later.
+ RefPtr<HistoryItem> protector(item);
// Abort any current load unless we're navigating the current document to a new state object
HistoryItem* currentItem = m_mainFrame->loader()->history()->currentItem();
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list