[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
commit-queue at webkit.org
commit-queue at webkit.org
Wed Dec 22 12:42:50 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit fb16e576cd90c5a08c1a0238b1babcab85fa0b5d
Author: commit-queue at webkit.org <commit-queue at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Aug 27 18:20:27 2010 +0000
2010-08-27 Mihai Parparita <mihaip at chromium.org>
Reviewed by Darin Fisher.
Crash in HistoryController::recursiveGoToItem when navigating in a frame
while another frame has a custom window name
https://bugs.webkit.org/show_bug.cgi?id=44183
Add a test to check that we can handle navigation from a top-level frame
when one of the child frames sets window.name after load.
* fast/history/history-subframe-with-name-expected.txt: Added.
* fast/history/history-subframe-with-name.html: Added.
* fast/history/resources/history-subframe-with-name-2.html: Added.
* fast/history/resources/history-subframe-with-name-3.html: Added.
* fast/history/resources/history-subframe-with-name-container.html: Added.
2010-08-27 Mihai Parparita <mihaip at chromium.org>
Reviewed by Darin Fisher.
Crash in HistoryController::recursiveGoToItem when navigating in a frame
while another frame has a custom window name
https://bugs.webkit.org/show_bug.cgi?id=44183
Fix up HistoryController::recursiveGoToItem to better check whether the
current document frames, the curent history item frames, and the
destination history frames match up.
Test: fast/history/history-subframe-with-name.html
* history/HistoryItem.cpp:
(WebCore::HistoryItem::childItemWithDocumentSequenceNumber): Add linear
lookup of child by document sequence number
(WebCore::HistoryItem::hasSameDocuments): Remove assumption that the
other item has the children in the same order (it doesn't seem to be
true)
(WebCore::HistoryItem::hasSameFrames): Add recursive comparison of child
frames.
* history/HistoryItem.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadItem): Compare full set of documents in
history items, not just the topmost ones. Otherwise when going between
framesets where only one of the subframes changed we wouldn't trigger a
load.
* loader/HistoryController.cpp:
(WebCore::HistoryController::recursiveGoToItem): Also check that the
two history items have the same frames
(WebCore::HistoryController::currentFramesMatchItem): Refactor
childFramesMatchItem to also check the top-most frame, to make
recursiveGoToItem easier to read.
* loader/HistoryController.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66238 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2fd7094..795d23f 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,20 @@
+2010-08-27 Mihai Parparita <mihaip at chromium.org>
+
+ Reviewed by Darin Fisher.
+
+ Crash in HistoryController::recursiveGoToItem when navigating in a frame
+ while another frame has a custom window name
+ https://bugs.webkit.org/show_bug.cgi?id=44183
+
+ Add a test to check that we can handle navigation from a top-level frame
+ when one of the child frames sets window.name after load.
+
+ * fast/history/history-subframe-with-name-expected.txt: Added.
+ * fast/history/history-subframe-with-name.html: Added.
+ * fast/history/resources/history-subframe-with-name-2.html: Added.
+ * fast/history/resources/history-subframe-with-name-3.html: Added.
+ * fast/history/resources/history-subframe-with-name-container.html: Added.
+
2010-08-27 Tony Chang <tony at chromium.org>
Unreviewed, remove duplicate test expectation.
diff --git a/LayoutTests/fast/history/history-subframe-with-name-expected.txt b/LayoutTests/fast/history/history-subframe-with-name-expected.txt
new file mode 100644
index 0000000..26c44ab
--- /dev/null
+++ b/LayoutTests/fast/history/history-subframe-with-name-expected.txt
@@ -0,0 +1,14 @@
+main frame - has 1 onunload handler(s)
+main frame - has 1 onunload handler(s)
+main frame - has 1 onunload handler(s)
+Verifies that we can go back and forward from the top level of a frame set where a window changes its name after the initial load.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 2 is currentPageId
+PASS 3 is currentPageId
+PASS 2 is currentPageId
+PASS 3 is currentPageId
+PASS Complete: navigated through all the states
+
diff --git a/LayoutTests/fast/history/history-subframe-with-name.html b/LayoutTests/fast/history/history-subframe-with-name.html
new file mode 100644
index 0000000..f192010
--- /dev/null
+++ b/LayoutTests/fast/history/history-subframe-with-name.html
@@ -0,0 +1,71 @@
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<pre id="console"></pre>
+<script>
+description('Verifies that we can go back and forward from the top level of a frame set where a window changes its name after the initial load.');
+
+var testWindow;
+
+onload = function()
+{
+ if (window.layoutTestController) {
+ layoutTestController.setCanOpenWindows();
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ testWindow = window.open('resources/history-subframe-with-name-container.html');
+ if (!testWindow)
+ testFailed('Could not open test window');
+}
+
+var State = {
+ 0: 'INITIAL',
+ 1: 'FRAME_LOADED_INITIAL',
+ 2: 'FRAME_LOADED_NAV',
+ 3: 'FRAME_LOADED_BACK',
+ 4: 'FRAME_LOADED_FORWARD'
+};
+
+var currentState = 0;
+var currentPageId;
+
+function onFrameLoaded(pageId)
+{
+ // The page ID is put in a global so that the eval() inside of shouldBe can
+ // see it
+ currentPageId = pageId;
+ currentState++;
+
+ switch (currentState) {
+ case 1:
+ shouldBe('2', 'currentPageId');
+ break;
+ case 2:
+ shouldBe('3', 'currentPageId');
+ testWindow.history.back();
+ break;
+ case 3:
+ shouldBe('2', 'currentPageId');
+ testWindow.history.forward();
+ break;
+ case 4:
+ shouldBe('3', 'currentPageId');
+ break;
+ default:
+ testFailed('Should not be in state ' + currentState);
+ break;
+ }
+
+ if (currentState == 4) {
+ testPassed('Complete: navigated through all the states');
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }
+}
+</script>
+</body>
diff --git a/LayoutTests/fast/history/resources/history-subframe-with-name-2.html b/LayoutTests/fast/history/resources/history-subframe-with-name-2.html
new file mode 100644
index 0000000..4cd6587
--- /dev/null
+++ b/LayoutTests/fast/history/resources/history-subframe-with-name-2.html
@@ -0,0 +1,11 @@
+<body onload="runTest()">
+<script>
+ function runTest()
+ {
+ parent.opener.onFrameLoaded(2);
+ // Make sure we generate a history entry
+ setTimeout(function() {location.href = 'history-subframe-with-name-3.html';}, 0);
+ }
+</script>
+FAIL: frame 2
+</body>
\ No newline at end of file
diff --git a/LayoutTests/fast/history/resources/history-subframe-with-name-3.html b/LayoutTests/fast/history/resources/history-subframe-with-name-3.html
new file mode 100644
index 0000000..7f3a285
--- /dev/null
+++ b/LayoutTests/fast/history/resources/history-subframe-with-name-3.html
@@ -0,0 +1,3 @@
+<body onload="parent.opener.onFrameLoaded(3)">
+PASS: frame 3
+</body>
\ No newline at end of file
diff --git a/LayoutTests/fast/history/resources/history-subframe-with-name-container.html b/LayoutTests/fast/history/resources/history-subframe-with-name-container.html
new file mode 100644
index 0000000..3ab8db8
--- /dev/null
+++ b/LayoutTests/fast/history/resources/history-subframe-with-name-container.html
@@ -0,0 +1,6 @@
+<script>
+// Disable the page cache when running inside of Safari
+onunload = function() {};
+</script>
+<iframe src="history-subframe-with-name-2.html"></iframe>
+<iframe src="data:text/html,<script>window.name='foo';</script>window that changes its name"></iframe>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index a5d314d..63d78e4 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2010-08-27 Mihai Parparita <mihaip at chromium.org>
+
+ Reviewed by Darin Fisher.
+
+ Crash in HistoryController::recursiveGoToItem when navigating in a frame
+ while another frame has a custom window name
+ https://bugs.webkit.org/show_bug.cgi?id=44183
+
+ Fix up HistoryController::recursiveGoToItem to better check whether the
+ current document frames, the curent history item frames, and the
+ destination history frames match up.
+
+ Test: fast/history/history-subframe-with-name.html
+
+ * history/HistoryItem.cpp:
+ (WebCore::HistoryItem::childItemWithDocumentSequenceNumber): Add linear
+ lookup of child by document sequence number
+ (WebCore::HistoryItem::hasSameDocuments): Remove assumption that the
+ other item has the children in the same order (it doesn't seem to be
+ true)
+ (WebCore::HistoryItem::hasSameFrames): Add recursive comparison of child
+ frames.
+ * history/HistoryItem.h:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadItem): Compare full set of documents in
+ history items, not just the topmost ones. Otherwise when going between
+ framesets where only one of the subframes changed we wouldn't trigger a
+ load.
+ * loader/HistoryController.cpp:
+ (WebCore::HistoryController::recursiveGoToItem): Also check that the
+ two history items have the same frames
+ (WebCore::HistoryController::currentFramesMatchItem): Refactor
+ childFramesMatchItem to also check the top-most frame, to make
+ recursiveGoToItem easier to read.
+ * loader/HistoryController.h:
+
2010-08-27 Patrick Gansterer <paroga at paroga.com>
Reviewed by Nikolas Zimmermann.
diff --git a/WebCore/history/HistoryItem.cpp b/WebCore/history/HistoryItem.cpp
index 42adcfb..8a84e2e 100644
--- a/WebCore/history/HistoryItem.cpp
+++ b/WebCore/history/HistoryItem.cpp
@@ -446,6 +446,16 @@ HistoryItem* HistoryItem::childItemWithTarget(const String& target) const
return 0;
}
+HistoryItem* HistoryItem::childItemWithDocumentSequenceNumber(long long number) const
+{
+ unsigned size = m_children.size();
+ for (unsigned i = 0; i < size; ++i) {
+ if (m_children[i]->documentSequenceNumber() == number)
+ return m_children[i].get();
+ }
+ return 0;
+}
+
// <rdar://problem/4895849> HistoryItem::findTargetItem() should be replaced with a non-recursive method.
HistoryItem* HistoryItem::findTargetItem()
{
@@ -480,6 +490,8 @@ void HistoryItem::clearChildren()
m_children.clear();
}
+// Does a recursive check that this item and its descendants have the same
+// document sequence numbers as the other item.
bool HistoryItem::hasSameDocuments(HistoryItem* otherItem)
{
if (documentSequenceNumber() != otherItem->documentSequenceNumber())
@@ -487,12 +499,32 @@ bool HistoryItem::hasSameDocuments(HistoryItem* otherItem)
if (children().size() != otherItem->children().size())
return false;
-
+
for (size_t i = 0; i < children().size(); i++) {
- if (!children()[i]->hasSameDocuments(otherItem->children()[i].get()))
+ HistoryItem* child = children()[i].get();
+ HistoryItem* otherChild = otherItem->childItemWithDocumentSequenceNumber(child->documentSequenceNumber());
+ if (!otherChild || !child->hasSameDocuments(otherChild))
return false;
}
-
+
+ return true;
+}
+
+// Does a non-recursive check that this item and its immediate children have the
+// same frames as the other item.
+bool HistoryItem::hasSameFrames(HistoryItem* otherItem)
+{
+ if (target() != otherItem->target())
+ return false;
+
+ if (children().size() != otherItem->children().size())
+ return false;
+
+ for (size_t i = 0; i < children().size(); i++) {
+ if (!otherItem->childItemWithTarget(children()[i]->target()))
+ return false;
+ }
+
return true;
}
diff --git a/WebCore/history/HistoryItem.h b/WebCore/history/HistoryItem.h
index dfdbc43..b11a92e 100644
--- a/WebCore/history/HistoryItem.h
+++ b/WebCore/history/HistoryItem.h
@@ -155,12 +155,14 @@ public:
void addChildItem(PassRefPtr<HistoryItem>);
void setChildItem(PassRefPtr<HistoryItem>);
HistoryItem* childItemWithTarget(const String&) const;
+ HistoryItem* childItemWithDocumentSequenceNumber(long long number) const;
HistoryItem* targetItem();
const HistoryItemVector& children() const;
bool hasChildren() const;
void clearChildren();
bool hasSameDocuments(HistoryItem* otherItem);
+ bool hasSameFrames(HistoryItem* otherItem);
// This should not be called directly for HistoryItems that are already included
// in GlobalHistory. The WebKit api for this is to use -[WebHistory setLastVisitedTimeInterval:forItem:] instead.
diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index c40f76a..1b4e489 100644
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -3234,11 +3234,11 @@ void FrameLoader::navigateToDifferentDocument(HistoryItem* item, FrameLoadType l
void FrameLoader::loadItem(HistoryItem* item, FrameLoadType loadType)
{
// We do same-document navigation in the following cases:
- // - The HistoryItem corresponds to the same document.
+ // - The HistoryItem corresponds to the same document (or documents in the case of frames).
// - The HistoryItem is not the same as the current item.
HistoryItem* currentItem = history()->currentItem();
bool sameDocumentNavigation = currentItem && item != currentItem
- && item->documentSequenceNumber() == currentItem->documentSequenceNumber();
+ && item->hasSameDocuments(currentItem);
#if ENABLE(WML)
// All WML decks should go through the real load mechanism, not the scroll-to-anchor code
diff --git a/WebCore/loader/HistoryController.cpp b/WebCore/loader/HistoryController.cpp
index 144faa5..11a0277 100644
--- a/WebCore/loader/HistoryController.cpp
+++ b/WebCore/loader/HistoryController.cpp
@@ -558,9 +558,10 @@ void HistoryController::recursiveGoToItem(HistoryItem* item, HistoryItem* fromIt
// to match.
// Note: If item and fromItem are the same, then we need to create a new
// document.
- if (item != fromItem && item->itemSequenceNumber() == fromItem->itemSequenceNumber()
- && ((m_frame->tree()->name().isEmpty() && item->target().isEmpty()) || m_frame->tree()->name() == item->target())
- && childFramesMatchItem(item))
+ if (item != fromItem
+ && item->itemSequenceNumber() == fromItem->itemSequenceNumber()
+ && currentFramesMatchItem(item)
+ && fromItem->hasSameFrames(item))
{
// This content is good, so leave it alone and look for children that need reloading
// Save form state (works from currentItem, since prevItem is nil)
@@ -585,7 +586,7 @@ void HistoryController::recursiveGoToItem(HistoryItem* item, HistoryItem* fromIt
for (int i = 0; i < size; ++i) {
String childFrameName = childItems[i]->target();
HistoryItem* fromChildItem = fromItem->childItemWithTarget(childFrameName);
- ASSERT(fromChildItem || fromItem->isTargetItem());
+ ASSERT(fromChildItem);
Frame* childFrame = m_frame->tree()->child(childFrameName);
ASSERT(childFrame);
childFrame->loader()->history()->recursiveGoToItem(childItems[i].get(), fromChildItem, type);
@@ -595,10 +596,12 @@ void HistoryController::recursiveGoToItem(HistoryItem* item, HistoryItem* fromIt
}
}
-// helper method that determines whether the subframes described by the item's subitems
-// match our own current frameset
-bool HistoryController::childFramesMatchItem(HistoryItem* item) const
+// Helper method that determines whether the current frame tree matches given history item's.
+bool HistoryController::currentFramesMatchItem(HistoryItem* item) const
{
+ if ((!m_frame->tree()->name().isEmpty() || !item->target().isEmpty()) && m_frame->tree()->name() != item->target())
+ return false;
+
const HistoryItemVector& childItems = item->children();
if (childItems.size() != m_frame->tree()->childCount())
return false;
@@ -609,7 +612,6 @@ bool HistoryController::childFramesMatchItem(HistoryItem* item) const
return false;
}
- // Found matches for all item targets
return true;
}
diff --git a/WebCore/loader/HistoryController.h b/WebCore/loader/HistoryController.h
index 19902f8..487fdc9 100644
--- a/WebCore/loader/HistoryController.h
+++ b/WebCore/loader/HistoryController.h
@@ -86,7 +86,7 @@ private:
PassRefPtr<HistoryItem> createItemTree(Frame* targetFrame, bool clipAtTarget);
void recursiveGoToItem(HistoryItem*, HistoryItem*, FrameLoadType);
- bool childFramesMatchItem(HistoryItem*) const;
+ bool currentFramesMatchItem(HistoryItem*) const;
void updateBackForwardListClippedAtTarget(bool doClip);
Frame* m_frame;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list