Title: [97821] trunk
- Revision
- 97821
- Author
- [email protected]
- Date
- 2011-10-18 17:52:24 -0700 (Tue, 18 Oct 2011)
Log Message
Assertion failure when going back in page with navigated subframes
https://bugs.webkit.org/show_bug.cgi?id=70389
<rdar://problem/8988444>
Reviewed by Darin Adler.
Source/WebCore:
Test: fast/history/history-back-twice-with-subframes-assert.html
If a single navigation ends up loading multiple frame, the first committed frame will
end up calling recursiveUpdateForCommit on the main frame which will null out the provisional item
for all frames on the page. This means that it can null out the provisional item for any frames
that are still yet to be committed which causes the aforementioned assertion failure.
Fix this by only nulling out the provisional history item (and saving/restoring the scroll position and
some other things) for frames that already contain the URL that the item requested. If a frame is being loaded,
it will null out its provisional history item when it's committed.
* loader/HistoryController.cpp:
(WebCore::HistoryController::recursiveUpdateForCommit):
LayoutTests:
* fast/history/history-back-twice-with-subframes-assert-expected.txt: Added.
* fast/history/history-back-twice-with-subframes-assert.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (97820 => 97821)
--- trunk/LayoutTests/ChangeLog 2011-10-19 00:47:26 UTC (rev 97820)
+++ trunk/LayoutTests/ChangeLog 2011-10-19 00:52:24 UTC (rev 97821)
@@ -1,3 +1,14 @@
+2011-10-18 Anders Carlsson <[email protected]>
+
+ Assertion failure when going back in page with navigated subframes
+ https://bugs.webkit.org/show_bug.cgi?id=70389
+ <rdar://problem/8988444>
+
+ Reviewed by Darin Adler.
+
+ * fast/history/history-back-twice-with-subframes-assert-expected.txt: Added.
+ * fast/history/history-back-twice-with-subframes-assert.html: Added.
+
2011-10-18 Sheriff Bot <[email protected]>
Unreviewed, rolling out r97765.
Added: trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert-expected.txt (0 => 97821)
--- trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert-expected.txt 2011-10-19 00:52:24 UTC (rev 97821)
@@ -0,0 +1,4 @@
+This tests that navigating two subframes and then going back using history.go(-2) won't assert.
+
+This tests that navigating two subframes and then going back using history.go(-2) won't assert.
+
Added: trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert.html (0 => 97821)
--- trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert.html (rev 0)
+++ trunk/LayoutTests/fast/history/history-back-twice-with-subframes-assert.html 2011-10-19 00:52:24 UTC (rev 97821)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<script>
+if (layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+function runTest() {
+ var loadCount = 2;
+ var didGoBack = false;
+
+ var frame1 = document.getElementById('frame1');
+ var frame2 = document.getElementById('frame2');
+ frame1._onload_ = frame2._onload_ = function() {
+ loadCount--;
+ if (loadCount)
+ return;
+
+ if (!didGoBack) {
+ // We've navigated both frames, now go back 2 steps.
+ loadCount = 2;
+ history.go(-2);
+ didGoBack = true;
+ return;
+ }
+
+ if (layoutTestController)
+ layoutTestController.notifyDone();
+ }
+
+ frame1.src = "" = '';
+}
+</script>
+<body _onload_="setTimeout(runTest, 0)">
+<div>This tests that navigating two subframes and then going back using history.go(-2) won't assert.</div>
+<iframe id='frame1' src=''></iframe>
+<iframe id='frame2' src=''></iframe>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (97820 => 97821)
--- trunk/Source/WebCore/ChangeLog 2011-10-19 00:47:26 UTC (rev 97820)
+++ trunk/Source/WebCore/ChangeLog 2011-10-19 00:52:24 UTC (rev 97821)
@@ -1,3 +1,25 @@
+2011-10-18 Anders Carlsson <[email protected]>
+
+ Assertion failure when going back in page with navigated subframes
+ https://bugs.webkit.org/show_bug.cgi?id=70389
+ <rdar://problem/8988444>
+
+ Reviewed by Darin Adler.
+
+ Test: fast/history/history-back-twice-with-subframes-assert.html
+
+ If a single navigation ends up loading multiple frame, the first committed frame will
+ end up calling recursiveUpdateForCommit on the main frame which will null out the provisional item
+ for all frames on the page. This means that it can null out the provisional item for any frames
+ that are still yet to be committed which causes the aforementioned assertion failure.
+
+ Fix this by only nulling out the provisional history item (and saving/restoring the scroll position and
+ some other things) for frames that already contain the URL that the item requested. If a frame is being loaded,
+ it will null out its provisional history item when it's committed.
+
+ * loader/HistoryController.cpp:
+ (WebCore::HistoryController::recursiveUpdateForCommit):
+
2011-10-18 Sheriff Bot <[email protected]>
Unreviewed, rolling out r97765.
Modified: trunk/Source/WebCore/loader/HistoryController.cpp (97820 => 97821)
--- trunk/Source/WebCore/loader/HistoryController.cpp 2011-10-19 00:47:26 UTC (rev 97820)
+++ trunk/Source/WebCore/loader/HistoryController.cpp 2011-10-19 00:52:24 UTC (rev 97821)
@@ -472,24 +472,26 @@
// For each frame that already had the content the item requested (based on
// (a matching URL and frame tree snapshot), just restore the scroll position.
// Save form state (works from currentItem, since m_frameLoadComplete is true)
- ASSERT(m_frameLoadComplete);
- saveDocumentState();
- saveScrollPositionAndViewStateToItem(m_currentItem.get());
+ if (itemsAreClones(m_currentItem.get(), m_provisionalItem.get())) {
+ ASSERT(m_frameLoadComplete);
+ saveDocumentState();
+ saveScrollPositionAndViewStateToItem(m_currentItem.get());
- if (FrameView* view = m_frame->view())
- view->setWasScrolledByUser(false);
+ if (FrameView* view = m_frame->view())
+ view->setWasScrolledByUser(false);
- // Now commit the provisional item
- m_frameLoadComplete = false;
- m_previousItem = m_currentItem;
- m_currentItem = m_provisionalItem;
- m_provisionalItem = 0;
+ // Now commit the provisional item
+ m_frameLoadComplete = false;
+ m_previousItem = m_currentItem;
+ m_currentItem = m_provisionalItem;
+ m_provisionalItem = 0;
- // Restore form state (works from currentItem)
- restoreDocumentState();
+ // Restore form state (works from currentItem)
+ restoreDocumentState();
- // Restore the scroll position (we choose to do this rather than going back to the anchor point)
- restoreScrollPositionAndViewState();
+ // Restore the scroll position (we choose to do this rather than going back to the anchor point)
+ restoreScrollPositionAndViewState();
+ }
// Iterate over the rest of the tree
for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
