Commit e5915f11 authored by darin@chromium.org's avatar darin@chromium.org

2010-01-26 Darin Fisher <darin@chromium.org>

        Reviewed by Brady Eidson and David Levin.

        Chains of history items representing same-document navigation need to
        always remember that association

        https://bugs.webkit.org/show_bug.cgi?id=33224

        Replace HistoryItem's Document pointer with a DocumentSequenceNumber.
        During session history traversal, if the current HistoryItem and the
        target HistoryItem have the same DocumentSequenceNumber, then it means
        that the current Document should remain.

        NOTE: To support Chromium's serialization of HistoryItems, I generate
        DocumentSequenceNumbers that are unique across application launches.
        DocumentSequenceNumbers are generated using a counter initialized with
        the time of day.

        Test: fast/loader/stateobjects/document-destroyed-navigate-back.html

        * dom/Document.cpp:
        (WebCore::Document::detach):
        * dom/Document.h:
        * history/BackForwardList.cpp:
        (WebCore::BackForwardList::pushStateItem):
        * history/BackForwardListChromium.cpp:
        (WebCore::BackForwardList::pushStateItem):
        * history/HistoryItem.cpp:
        (WebCore::generateDocumentSequenceNumber):
        (WebCore::HistoryItem::HistoryItem):
        (WebCore::HistoryItem::~HistoryItem):
        (WebCore::HistoryItem::setStateObject):
        * history/HistoryItem.h:
        (WebCore::HistoryItem::setDocumentSequenceNumber):
        (WebCore::HistoryItem::documentSequenceNumber):
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::navigateWithinDocument):
        (WebCore::FrameLoader::loadItem):
        * loader/HistoryController.cpp:
        (WebCore::HistoryController::updateBackForwardListForFragmentScroll):
        (WebCore::HistoryController::pushState):
        (WebCore::HistoryController::replaceState):
        * loader/RedirectScheduler.cpp:
        (WebCore::RedirectScheduler::scheduleHistoryNavigation):
        * page/History.cpp:
        (WebCore::History::stateObjectAdded):
        * page/Page.cpp:
        (WebCore::Page::goToItem):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53950 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 4d60cbde
2010-01-26 Darin Fisher <darin@chromium.org>
Reviewed by Brady Eidson and David Levin.
Chains of history items representing same-document navigation need to
always remember that association
https://bugs.webkit.org/show_bug.cgi?id=33224
* fast/dom/location-hash-expected.txt:
* fast/dom/location-hash.html: Revert changes from r43274 since
history.back to a reference fragment is now synchronous.
* fast/history/back-forward-is-asynchronous-expected.txt: Removed.
* fast/history/back-forward-is-asynchronous.html: Removed.
This test is no longer valid (revert r43274).
* fast/loader/stateobjects/document-destroyed-navigate-back-expected.txt:
* fast/loader/stateobjects/document-destroyed-navigate-back.html: Revised
test results to match corrected behavior. Also removed the redundant
history.back() call, which helps make the test a bit easier to understand.
2010-01-27 Yael Aharon <yael.aharon@nokia.com>
Reviewed by Laszlo Gombos.
......@@ -11,16 +11,10 @@ PASS: window.history.length == originalHistoryLength + 2 should be true and is.
PASS: window.location.hash should be #bar and is.
PASS: window.location == originalLocation + '#bar' should be true and is.
PASS: window.history.length == originalHistoryLength + 2 should be true and is.
PASS: window.location.hash should be #bar and is.
PASS: window.location == originalLocation + '#bar' should be true and is.
PASS: window.location.hash should be #foo and is.
PASS: window.location == originalLocation + '#foo' should be true and is.
PASS: window.location.hash should be #foo and is.
PASS: window.location == originalLocation + '#foo' should be true and is.
PASS: window.location.hash should be and is.
PASS: window.location == originalLocation should be true and is.
PASS: window.location.hash should be and is.
PASS: window.location == originalLocation should be true and is.
PASS: window.location.hash should be #foo and is.
PASS: window.location == originalLocation + '#foo' should be true and is.
SUCCESS!
......@@ -24,7 +24,7 @@
function step() {
switch (state) {
case 0:
case 0:
shouldBe('window.history.length == originalHistoryLength', true);
shouldBe('window.location.hash', '');
window.location.hash = 'foo';
......@@ -45,33 +45,21 @@
shouldBe('window.history.length == originalHistoryLength + 2', true);
break;
case 3:
window.history.back(); // This should take effect asynchronously
shouldBe('window.location.hash', '#bar');
shouldBe("window.location == originalLocation + '#bar'", true);
break;
case 4:
shouldBe('window.location.hash', '#foo');
shouldBe("window.location == originalLocation + '#foo'", true);
break;
case 5:
window.history.back(); // This should take effect asynchronously
window.history.back();
shouldBe('window.location.hash', '#foo');
shouldBe("window.location == originalLocation + '#foo'", true);
break;
case 6:
shouldBe('window.location.hash', '');
shouldBe("window.location == originalLocation", true);
break;
case 7:
window.history.forward(); // This should take effect asynchronously
case 4:
window.history.back();
shouldBe('window.location.hash', '');
shouldBe("window.location == originalLocation", true);
break;
case 8:
case 5:
window.history.forward();
shouldBe('window.location.hash', '#foo');
shouldBe("window.location == originalLocation + '#foo'", true);
break;
case 9:
case 6:
window.location.hash = '';
if (numErrors == 0)
print("SUCCESS!", "green")
......@@ -85,7 +73,7 @@
}
state ++;
setTimeout(step, 0);
step();
}
function runTests() {
......
<html>
<head>
<script>
var scrollCounter = 0;
function runTest()
{
if (window.layoutTestController) {
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
// This takes effect synchronously, and the result is that we scroll to "foo".
location.hash = "#foo";
// This should take effect asynchronously, and the result should be that we
// scroll back to the top of the page. If this happens synchronously, then
// scrollCounter will be incremented again by the time history.back returns.
history.back();
var didScrollAgain = scrollCounter != 1;
document.body.replaceChild(
document.createTextNode(didScrollAgain ? "FAIL" : "PASS"),
document.body.getElementsByTagName("DIV")[0])
// Defer calling notifyDone until the history.back() call completes. This
// should not be necessary, but the test framework might get confused if it
// navigates to the next test before the history.back() call completes.
if (window.layoutTestController)
setTimeout(function() { layoutTestController.notifyDone() }, 0);
}
</script>
</head>
<body onload="runTest()" onscroll="scrollCounter++">
<div style="padding-top: 10000px">
<a name="foo">foo</a>
</div>
</body>
</html>
......@@ -5,8 +5,6 @@ ALERT: Navigating back...
main frame - has 1 onunload handler(s)
ALERT: Last path component of location is document-destroyed-navigate-back.html?SecondEntryWillLaterBeReactivated
ALERT: State popped - SecondEntryWillLaterBeReactivated (type string)
main frame - has 1 onunload handler(s)
ALERT: Last path component of location is document-destroyed-navigate-back.html?FirstEntryWillLaterBeReactivated
ALERT: State popped - FirstEntryWillLaterBeReactivated (type string)
ALERT: Test complete
This test:
......
......@@ -24,7 +24,6 @@ function runFirstStageOfTest()
function runSecondStageOfTest()
{
alert("Last path component of location is " + lastPathComponent());
history.back();
}
function runThirdStageOfTest()
......
2010-01-26 Darin Fisher <darin@chromium.org>
Reviewed by Brady Eidson and David Levin.
Chains of history items representing same-document navigation need to
always remember that association
https://bugs.webkit.org/show_bug.cgi?id=33224
Replace HistoryItem's Document pointer with a DocumentSequenceNumber.
During session history traversal, if the current HistoryItem and the
target HistoryItem have the same DocumentSequenceNumber, then it means
that the current Document should remain.
NOTE: To support Chromium's serialization of HistoryItems, I generate
DocumentSequenceNumbers that are unique across application launches.
DocumentSequenceNumbers are generated using a counter initialized with
the time of day.
Test: fast/loader/stateobjects/document-destroyed-navigate-back.html
* dom/Document.cpp:
(WebCore::Document::detach):
* dom/Document.h:
* history/BackForwardList.cpp:
(WebCore::BackForwardList::pushStateItem):
* history/BackForwardListChromium.cpp:
(WebCore::BackForwardList::pushStateItem):
* history/HistoryItem.cpp:
(WebCore::generateDocumentSequenceNumber):
(WebCore::HistoryItem::HistoryItem):
(WebCore::HistoryItem::~HistoryItem):
(WebCore::HistoryItem::setStateObject):
* history/HistoryItem.h:
(WebCore::HistoryItem::setDocumentSequenceNumber):
(WebCore::HistoryItem::documentSequenceNumber):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::navigateWithinDocument):
(WebCore::FrameLoader::loadItem):
* loader/HistoryController.cpp:
(WebCore::HistoryController::updateBackForwardListForFragmentScroll):
(WebCore::HistoryController::pushState):
(WebCore::HistoryController::replaceState):
* loader/RedirectScheduler.cpp:
(WebCore::RedirectScheduler::scheduleHistoryNavigation):
* page/History.cpp:
(WebCore::History::stateObjectAdded):
* page/Page.cpp:
(WebCore::Page::goToItem):
2010-01-27 Oliver Hunt <oliver@apple.com>
Reviewed by Simon Fraser.
......@@ -1553,12 +1553,6 @@ void Document::detach()
if (render)
render->destroy();
HashSet<RefPtr<HistoryItem> > associatedHistoryItems;
associatedHistoryItems.swap(m_associatedHistoryItems);
HashSet<RefPtr<HistoryItem> >::iterator end = associatedHistoryItems.end();
for (HashSet<RefPtr<HistoryItem> >::iterator i = associatedHistoryItems.begin(); i != end; ++i)
(*i)->documentDetached(this);
// This is required, as our Frame might delete itself as soon as it detaches
// us. However, this violates Node::detach() semantics, as it's never
// possible to re-attach. Eventually Document::detach() should be renamed,
......@@ -4499,18 +4493,6 @@ void Document::statePopped(SerializedScriptValue* stateObject)
m_pendingStateObject = stateObject;
}
void Document::registerHistoryItem(HistoryItem* item)
{
ASSERT(!m_associatedHistoryItems.contains(item));
m_associatedHistoryItems.add(item);
}
void Document::unregisterHistoryItem(HistoryItem* item)
{
ASSERT(m_associatedHistoryItems.contains(item) || m_associatedHistoryItems.isEmpty());
m_associatedHistoryItems.remove(item);
}
void Document::updateSandboxFlags()
{
if (m_frame && securityOrigin())
......
......@@ -907,8 +907,6 @@ public:
void updateURLForPushOrReplaceState(const KURL&);
void statePopped(SerializedScriptValue*);
void registerHistoryItem(HistoryItem* item);
void unregisterHistoryItem(HistoryItem* item);
void updateSandboxFlags(); // Set sandbox flags as determined by the frame.
......
......@@ -239,20 +239,15 @@ HistoryItemVector& BackForwardList::entries()
void BackForwardList::pushStateItem(PassRefPtr<HistoryItem> newItem)
{
ASSERT(newItem);
ASSERT(newItem->document());
ASSERT(newItem->stateObject());
RefPtr<HistoryItem> current = currentItem();
ASSERT(current);
Document* newItemDocument = newItem->document();
addItem(newItem);
if (!current->document()) {
current->setDocument(newItemDocument);
if (!current->stateObject())
current->setStateObject(SerializedScriptValue::create());
}
}
void BackForwardList::close()
......
......@@ -123,7 +123,12 @@ HistoryItem* BackForwardList::itemAtIndex(int index)
void BackForwardList::pushStateItem(PassRefPtr<HistoryItem> newItem)
{
// FIXME: Need to implement state support for chromium.
RefPtr<HistoryItem> current = m_client->currentItem();
addItem(newItem);
if (!current->stateObject())
current->setStateObject(SerializedScriptValue::create());
}
HistoryItemVector& BackForwardList::entries()
......
......@@ -33,9 +33,18 @@
#include "PageCache.h"
#include "ResourceRequest.h"
#include <stdio.h>
#include <wtf/CurrentTime.h>
namespace WebCore {
static long long generateDocumentSequenceNumber()
{
// Initialize to the current time to reduce the likelihood of generating
// identifiers that overlap with those from past/future browser sessions.
static long long next = static_cast<long long>(currentTime() * 1000000.0);
return ++next;
}
static void defaultNotifyHistoryItemChanged(HistoryItem*)
{
}
......@@ -48,7 +57,7 @@ HistoryItem::HistoryItem()
, m_lastVisitWasFailure(false)
, m_isTargetItem(false)
, m_visitCount(0)
, m_document(0)
, m_documentSequenceNumber(generateDocumentSequenceNumber())
{
}
......@@ -61,7 +70,7 @@ HistoryItem::HistoryItem(const String& urlString, const String& title, double ti
, m_lastVisitWasFailure(false)
, m_isTargetItem(false)
, m_visitCount(0)
, m_document(0)
, m_documentSequenceNumber(generateDocumentSequenceNumber())
{
iconDatabase()->retainIconForPageURL(m_urlString);
}
......@@ -76,7 +85,7 @@ HistoryItem::HistoryItem(const String& urlString, const String& title, const Str
, m_lastVisitWasFailure(false)
, m_isTargetItem(false)
, m_visitCount(0)
, m_document(0)
, m_documentSequenceNumber(generateDocumentSequenceNumber())
{
iconDatabase()->retainIconForPageURL(m_urlString);
}
......@@ -92,7 +101,7 @@ HistoryItem::HistoryItem(const KURL& url, const String& target, const String& pa
, m_lastVisitWasFailure(false)
, m_isTargetItem(false)
, m_visitCount(0)
, m_document(0)
, m_documentSequenceNumber(generateDocumentSequenceNumber())
{
iconDatabase()->retainIconForPageURL(m_urlString);
}
......@@ -100,7 +109,6 @@ HistoryItem::HistoryItem(const KURL& url, const String& target, const String& pa
HistoryItem::~HistoryItem()
{
ASSERT(!m_cachedPage);
ASSERT(!m_document);
iconDatabase()->releaseIconForPageURL(m_urlString);
#if PLATFORM(ANDROID)
if (m_bridge)
......@@ -125,7 +133,7 @@ inline HistoryItem::HistoryItem(const HistoryItem& item)
, m_visitCount(item.m_visitCount)
, m_dailyVisitCounts(item.m_dailyVisitCounts)
, m_weeklyVisitCounts(item.m_weeklyVisitCounts)
, m_document(0)
, m_documentSequenceNumber(generateDocumentSequenceNumber())
, m_formContentType(item.m_formContentType)
{
if (item.m_formData)
......@@ -397,29 +405,9 @@ void HistoryItem::setIsTargetItem(bool flag)
void HistoryItem::setStateObject(PassRefPtr<SerializedScriptValue> object)
{
ASSERT(m_document);
m_stateObject = object;
}
void HistoryItem::setDocument(Document* document)
{
if (m_document == document)
return;
if (m_document)
m_document->unregisterHistoryItem(this);
if (document)
document->registerHistoryItem(this);
m_document = document;
}
void HistoryItem::documentDetached(Document* document)
{
ASSERT_UNUSED(document, m_document == document);
m_document = 0;
}
void HistoryItem::addChildItem(PassRefPtr<HistoryItem> child)
{
ASSERT(!childItemWithTarget(child->target()));
......
......@@ -135,9 +135,9 @@ public:
void setStateObject(PassRefPtr<SerializedScriptValue> object);
SerializedScriptValue* stateObject() const { return m_stateObject.get(); }
void setDocument(Document* document);
Document* document() const { return m_document; }
void documentDetached(Document*);
void setDocumentSequenceNumber(long long number) { m_documentSequenceNumber = number; }
long long documentSequenceNumber() const { return m_documentSequenceNumber; }
void setFormInfoFromRequest(const ResourceRequest&);
void setFormData(PassRefPtr<FormData>);
......@@ -244,7 +244,7 @@ private:
// Support for HTML5 History
RefPtr<SerializedScriptValue> m_stateObject;
Document* m_document;
long long m_documentSequenceNumber;
// info used to repost form data
RefPtr<FormData> m_formData;
......
......@@ -3693,7 +3693,7 @@ Frame* FrameLoader::findFrameForNavigation(const AtomicString& name)
void FrameLoader::navigateWithinDocument(HistoryItem* item)
{
ASSERT(!item->document() || item->document() == m_frame->document());
ASSERT(item->documentSequenceNumber() == history()->currentItem()->documentSequenceNumber());
// Save user view state to the current history item here since we don't do a normal load.
// FIXME: Does form state need to be saved here too?
......@@ -3814,7 +3814,7 @@ void FrameLoader::loadItem(HistoryItem* item, FrameLoadType loadType)
// - The HistoryItem has a history state object
// - Navigating to an anchor within the page, with no form data stored on the target item or the current history entry,
// and the URLs in the frame tree match the history item for fragment scrolling.
bool sameDocumentNavigation = (!item->formData() && !(history()->currentItem() && history()->currentItem()->formData()) && history()->urlsMatchItem(item)) || item->document() == m_frame->document();
bool sameDocumentNavigation = (!item->formData() && !(history()->currentItem() && history()->currentItem()->formData()) && history()->urlsMatchItem(item)) || item->documentSequenceNumber() == history()->currentItem()->documentSequenceNumber();
#if ENABLE(WML)
// All WML decks should go through the real load mechanism, not the scroll-to-anchor code
......
......@@ -106,6 +106,10 @@ void HistoryController::restoreScrollPositionAndViewState()
void HistoryController::updateBackForwardListForFragmentScroll()
{
updateBackForwardListClippedAtTarget(false);
// Since the document isn't changed as a result of a fragment scroll, we should
// preserve the DocumentSequenceNumber of the previous item.
m_currentItem->setDocumentSequenceNumber(m_previousItem->documentSequenceNumber());
}
void HistoryController::saveDocumentState()
......@@ -635,10 +639,13 @@ void HistoryController::pushState(PassRefPtr<SerializedScriptValue> stateObject,
ASSERT(item->isTargetItem());
// Override data in the target item to reflect the pushState() arguments.
item->setDocument(m_frame->document());
item->setTitle(title);
item->setStateObject(stateObject);
item->setURLString(urlString);
// Since the document isn't changed as a result of a pushState call, we
// should preserve the DocumentSequenceNumber of the previous item.
item->setDocumentSequenceNumber(m_previousItem->documentSequenceNumber());
page->backForwardList()->pushStateItem(item.release());
}
......@@ -649,10 +656,7 @@ void HistoryController::replaceState(PassRefPtr<SerializedScriptValue> stateObje
ASSERT(page);
HistoryItem* current = page->backForwardList()->currentItem();
ASSERT(current);
ASSERT(!current->document() || current->document() == m_frame->document());
current->setDocument(m_frame->document());
if (!urlString.isEmpty())
current->setURLString(urlString);
current->setTitle(title);
......
......@@ -277,7 +277,7 @@ void RedirectScheduler::scheduleHistoryNavigation(int steps)
// If the specified entry and the current entry have the same document, this is either a state object traversal or a fragment
// traversal (or both) and should be performed synchronously.
HistoryItem* currentEntry = m_frame->loader()->history()->currentItem();
if (currentEntry != specifiedEntry && specifiedEntry->document() && currentEntry->document() == specifiedEntry->document()) {
if (currentEntry != specifiedEntry && currentEntry->documentSequenceNumber() == specifiedEntry->documentSequenceNumber()) {
m_frame->loader()->history()->goToItem(specifiedEntry, FrameLoadTypeIndexedBackForward);
return;
}
......
......@@ -112,13 +112,13 @@ void History::stateObjectAdded(PassRefPtr<SerializedScriptValue> data, const Str
else if (stateObjectType == StateObjectReplace)
m_frame->loader()->history()->replaceState(data, title, fullURL.string());
if (!urlString.isEmpty()) {
if (!urlString.isEmpty())
m_frame->document()->updateURLForPushOrReplaceState(fullURL);
if (stateObjectType == StateObjectPush)
m_frame->loader()->client()->dispatchDidPushStateWithinPage();
else if (stateObjectType == StateObjectReplace)
m_frame->loader()->client()->dispatchDidReplaceStateWithinPage();
}
if (stateObjectType == StateObjectPush)
m_frame->loader()->client()->dispatchDidPushStateWithinPage();
else if (stateObjectType == StateObjectReplace)
m_frame->loader()->client()->dispatchDidReplaceStateWithinPage();
}
} // namespace WebCore
......@@ -290,7 +290,7 @@ void Page::goBackOrForward(int distance)
void Page::goToItem(HistoryItem* item, FrameLoadType type)
{
// Abort any current load unless we're navigating the current document to a new state object
if (!item->stateObject() || item->document() != m_mainFrame->document()) {
if (!item->stateObject() || item->documentSequenceNumber() != m_mainFrame->loader()->history()->currentItem()->documentSequenceNumber()) {
// Define what to do with any open database connections. By default we stop them and terminate the database thread.
DatabasePolicy databasePolicy = DatabasePolicyStop;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment