Changeset 53861 in webkit


Ignore:
Timestamp:
Jan 26, 2010 11:30:17 AM (14 years ago)
Author:
darin@chromium.org
Message:

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

Reviewed by Brady Eidson.

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):
Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r53859 r53861  
     12010-01-25  Darin Fisher  <darin@chromium.org>
     2
     3        Reviewed by Brady Eidson.
     4
     5        Chains of history items representing same-document navigation need to
     6        always remember that association
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=33224
     9
     10        * fast/loader/stateobjects/document-destroyed-navigate-back-expected.txt:
     11        * fast/loader/stateobjects/document-destroyed-navigate-back.html: Revised
     12        test results to match corrected behavior.  Also removed the redundant
     13        history.back() call, which helps make the test a bit easier to understand.
     14
    1152010-01-26  Chris Fleizach  <cfleizach@apple.com>
    216
  • trunk/LayoutTests/fast/loader/stateobjects/document-destroyed-navigate-back-expected.txt

    r53361 r53861  
    66ALERT: Last path component of location is document-destroyed-navigate-back.html?SecondEntryWillLaterBeReactivated
    77ALERT: State popped - SecondEntryWillLaterBeReactivated (type string)
    8 main frame - has 1 onunload handler(s)
    9 ALERT: Last path component of location is document-destroyed-navigate-back.html?FirstEntryWillLaterBeReactivated
    108ALERT: State popped - FirstEntryWillLaterBeReactivated (type string)
    119ALERT: Test complete
  • trunk/LayoutTests/fast/loader/stateobjects/document-destroyed-navigate-back.html

    r53361 r53861  
    2525{
    2626    alert("Last path component of location is " + lastPathComponent());
    27     history.back();
    2827}
    2928
  • trunk/WebCore/ChangeLog

    r53857 r53861  
     12010-01-25  Darin Fisher  <darin@chromium.org>
     2
     3        Reviewed by Brady Eidson.
     4
     5        Chains of history items representing same-document navigation need to
     6        always remember that association
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=33224
     9
     10        Replace HistoryItem's Document pointer with a DocumentSequenceNumber.
     11        During session history traversal, if the current HistoryItem and the
     12        target HistoryItem have the same DocumentSequenceNumber, then it means
     13        that the current Document should remain.
     14
     15        NOTE: To support Chromium's serialization of HistoryItems, I generate
     16        DocumentSequenceNumbers that are unique across application launches.
     17        DocumentSequenceNumbers are generated using a counter initialized with
     18        the time of day.
     19
     20        Test: fast/loader/stateobjects/document-destroyed-navigate-back.html
     21
     22        * dom/Document.cpp:
     23        (WebCore::Document::detach):
     24        * dom/Document.h:
     25        * history/BackForwardList.cpp:
     26        (WebCore::BackForwardList::pushStateItem):
     27        * history/BackForwardListChromium.cpp:
     28        (WebCore::BackForwardList::pushStateItem):
     29        * history/HistoryItem.cpp:
     30        (WebCore::generateDocumentSequenceNumber):
     31        (WebCore::HistoryItem::HistoryItem):
     32        (WebCore::HistoryItem::~HistoryItem):
     33        (WebCore::HistoryItem::setStateObject):
     34        * history/HistoryItem.h:
     35        (WebCore::HistoryItem::setDocumentSequenceNumber):
     36        (WebCore::HistoryItem::documentSequenceNumber):
     37        * loader/FrameLoader.cpp:
     38        (WebCore::FrameLoader::navigateWithinDocument):
     39        (WebCore::FrameLoader::loadItem):
     40        * loader/HistoryController.cpp:
     41        (WebCore::HistoryController::updateBackForwardListForFragmentScroll):
     42        (WebCore::HistoryController::pushState):
     43        (WebCore::HistoryController::replaceState):
     44        * loader/RedirectScheduler.cpp:
     45        (WebCore::RedirectScheduler::scheduleHistoryNavigation):
     46        * page/History.cpp:
     47        (WebCore::History::stateObjectAdded):
     48        * page/Page.cpp:
     49        (WebCore::Page::goToItem):
     50
    1512010-01-26  Chris Fleizach  <cfleizach@apple.com>
    252
  • trunk/WebCore/dom/Document.cpp

    r53840 r53861  
    15491549        render->destroy();
    15501550   
    1551     HashSet<RefPtr<HistoryItem> > associatedHistoryItems;
    1552     associatedHistoryItems.swap(m_associatedHistoryItems);
    1553     HashSet<RefPtr<HistoryItem> >::iterator end = associatedHistoryItems.end();
    1554     for (HashSet<RefPtr<HistoryItem> >::iterator i = associatedHistoryItems.begin(); i != end; ++i)
    1555         (*i)->documentDetached(this);
    1556    
    15571551    // This is required, as our Frame might delete itself as soon as it detaches
    15581552    // us. However, this violates Node::detach() semantics, as it's never
     
    44954489}
    44964490
    4497 void Document::registerHistoryItem(HistoryItem* item)
    4498 {
    4499     ASSERT(!m_associatedHistoryItems.contains(item));
    4500     m_associatedHistoryItems.add(item);
    4501 }
    4502 
    4503 void Document::unregisterHistoryItem(HistoryItem* item)
    4504 {
    4505     ASSERT(m_associatedHistoryItems.contains(item) || m_associatedHistoryItems.isEmpty());
    4506     m_associatedHistoryItems.remove(item);
    4507 }
    4508 
    45094491void Document::updateSandboxFlags()
    45104492{
  • trunk/WebCore/dom/Document.h

    r53840 r53861  
    908908    void updateURLForPushOrReplaceState(const KURL&);
    909909    void statePopped(SerializedScriptValue*);
    910     void registerHistoryItem(HistoryItem* item);
    911     void unregisterHistoryItem(HistoryItem* item);
    912910
    913911    void updateSandboxFlags(); // Set sandbox flags as determined by the frame.
  • trunk/WebCore/history/BackForwardList.cpp

    r53324 r53861  
    240240{
    241241    ASSERT(newItem);
    242     ASSERT(newItem->document());
    243242    ASSERT(newItem->stateObject());
    244243   
     
    246245    ASSERT(current);
    247246
    248     Document* newItemDocument = newItem->document();
    249 
    250247    addItem(newItem);
    251248   
    252     if (!current->document()) {
    253         current->setDocument(newItemDocument);
     249    if (!current->stateObject())
    254250        current->setStateObject(SerializedScriptValue::create());
    255     }
    256251}
    257252
  • trunk/WebCore/history/BackForwardListChromium.cpp

    r51681 r53861  
    124124void BackForwardList::pushStateItem(PassRefPtr<HistoryItem> newItem)
    125125{
    126   // FIXME: Need to implement state support for chromium.
     126    RefPtr<HistoryItem> current = m_client->currentItem();
     127
     128    addItem(newItem);
     129
     130    if (!current->stateObject())
     131        current->setStateObject(SerializedScriptValue::create());
    127132}
    128133
  • trunk/WebCore/history/HistoryItem.cpp

    r53361 r53861  
    3434#include "ResourceRequest.h"
    3535#include <stdio.h>
     36#include <wtf/CurrentTime.h>
    3637
    3738namespace WebCore {
     39
     40static long long generateDocumentSequenceNumber()
     41{
     42    // Initialize to the current time to reduce the likelihood of generating
     43    // identifiers that overlap with those from past/future browser sessions.
     44    static long long next = static_cast<long long>(currentTime() * 1000000.0);
     45    return ++next;
     46}
    3847
    3948static void defaultNotifyHistoryItemChanged(HistoryItem*)
     
    4958    , m_isTargetItem(false)
    5059    , m_visitCount(0)
    51     , m_document(0)
     60    , m_documentSequenceNumber(generateDocumentSequenceNumber())
    5261{
    5362}
     
    6271    , m_isTargetItem(false)
    6372    , m_visitCount(0)
    64     , m_document(0)
     73    , m_documentSequenceNumber(generateDocumentSequenceNumber())
    6574{   
    6675    iconDatabase()->retainIconForPageURL(m_urlString);
     
    7786    , m_isTargetItem(false)
    7887    , m_visitCount(0)
    79     , m_document(0)
     88    , m_documentSequenceNumber(generateDocumentSequenceNumber())
    8089{
    8190    iconDatabase()->retainIconForPageURL(m_urlString);
     
    93102    , m_isTargetItem(false)
    94103    , m_visitCount(0)
    95     , m_document(0)
     104    , m_documentSequenceNumber(generateDocumentSequenceNumber())
    96105{   
    97106    iconDatabase()->retainIconForPageURL(m_urlString);
     
    101110{
    102111    ASSERT(!m_cachedPage);
    103     ASSERT(!m_document);
    104112    iconDatabase()->releaseIconForPageURL(m_urlString);
    105113#if PLATFORM(ANDROID)
     
    126134    , m_dailyVisitCounts(item.m_dailyVisitCounts)
    127135    , m_weeklyVisitCounts(item.m_weeklyVisitCounts)
    128     , m_document(0)
     136    , m_documentSequenceNumber(generateDocumentSequenceNumber())
    129137    , m_formContentType(item.m_formContentType)
    130138{
     
    398406void HistoryItem::setStateObject(PassRefPtr<SerializedScriptValue> object)
    399407{
    400     ASSERT(m_document);
    401408    m_stateObject = object;
    402 }
    403 
    404 void HistoryItem::setDocument(Document* document)
    405 {
    406     if (m_document == document)
    407         return;
    408    
    409     if (m_document)
    410         m_document->unregisterHistoryItem(this);
    411     if (document)
    412         document->registerHistoryItem(this);
    413        
    414     m_document = document;
    415 }
    416 
    417 void HistoryItem::documentDetached(Document* document)
    418 {
    419     ASSERT_UNUSED(document, m_document == document);
    420     m_document = 0;
    421409}
    422410
  • trunk/WebCore/history/HistoryItem.h

    r51644 r53861  
    136136    void setStateObject(PassRefPtr<SerializedScriptValue> object);
    137137    SerializedScriptValue* stateObject() const { return m_stateObject.get(); }
    138     void setDocument(Document* document);
    139     Document* document() const { return m_document; }
    140     void documentDetached(Document*);
     138
     139    void setDocumentSequenceNumber(long long number) { m_documentSequenceNumber = number; }
     140    long long documentSequenceNumber() const { return m_documentSequenceNumber; }
    141141   
    142142    void setFormInfoFromRequest(const ResourceRequest&);
     
    245245    // Support for HTML5 History
    246246    RefPtr<SerializedScriptValue> m_stateObject;
    247     Document* m_document;
     247    long long m_documentSequenceNumber;
    248248   
    249249    // info used to repost form data
  • trunk/WebCore/loader/FrameLoader.cpp

    r53809 r53861  
    36943694void FrameLoader::navigateWithinDocument(HistoryItem* item)
    36953695{
    3696     ASSERT(!item->document() || item->document() == m_frame->document());
     3696    ASSERT(item->documentSequenceNumber() == history()->currentItem()->documentSequenceNumber());
    36973697
    36983698    // Save user view state to the current history item here since we don't do a normal load.
     
    38153815    // - Navigating to an anchor within the page, with no form data stored on the target item or the current history entry,
    38163816    //   and the URLs in the frame tree match the history item for fragment scrolling.
    3817     bool sameDocumentNavigation = (!item->formData() && !(history()->currentItem() && history()->currentItem()->formData()) && history()->urlsMatchItem(item)) || item->document() == m_frame->document();
     3817    bool sameDocumentNavigation = (!item->formData() && !(history()->currentItem() && history()->currentItem()->formData()) && history()->urlsMatchItem(item)) || item->documentSequenceNumber() == history()->currentItem()->documentSequenceNumber();
    38183818
    38193819#if ENABLE(WML)
  • trunk/WebCore/loader/HistoryController.cpp

    r53650 r53861  
    107107{
    108108    updateBackForwardListClippedAtTarget(false);
     109   
     110    // Since the document isn't changed as a result of a fragment scroll, we should
     111    // preserve the DocumentSequenceNumber of the previous item.
     112    m_currentItem->setDocumentSequenceNumber(m_previousItem->documentSequenceNumber());
    109113}
    110114
     
    636640   
    637641    // Override data in the target item to reflect the pushState() arguments.
    638     item->setDocument(m_frame->document());
    639642    item->setTitle(title);
    640643    item->setStateObject(stateObject);
    641644    item->setURLString(urlString);
     645
     646    // Since the document isn't changed as a result of a pushState call, we
     647    // should preserve the DocumentSequenceNumber of the previous item.
     648    item->setDocumentSequenceNumber(m_previousItem->documentSequenceNumber());
    642649   
    643650    page->backForwardList()->pushStateItem(item.release());
     
    650657    HistoryItem* current = page->backForwardList()->currentItem();
    651658    ASSERT(current);
    652    
    653     ASSERT(!current->document() || current->document() == m_frame->document());
    654     current->setDocument(m_frame->document());
    655    
     659
    656660    if (!urlString.isEmpty())
    657661        current->setURLString(urlString);
  • trunk/WebCore/loader/RedirectScheduler.cpp

    r53672 r53861  
    278278    // traversal (or both) and should be performed synchronously.
    279279    HistoryItem* currentEntry = m_frame->loader()->history()->currentItem();
    280     if (currentEntry != specifiedEntry && specifiedEntry->document() && currentEntry->document() == specifiedEntry->document()) {
     280    if (currentEntry != specifiedEntry && currentEntry->documentSequenceNumber() == specifiedEntry->documentSequenceNumber()) {
    281281        m_frame->loader()->history()->goToItem(specifiedEntry, FrameLoadTypeIndexedBackForward);
    282282        return;
  • trunk/WebCore/page/History.cpp

    r53472 r53861  
    113113        m_frame->loader()->history()->replaceState(data, title, fullURL.string());
    114114           
    115     if (!urlString.isEmpty()) {
     115    if (!urlString.isEmpty())
    116116        m_frame->document()->updateURLForPushOrReplaceState(fullURL);
    117         if (stateObjectType == StateObjectPush)
    118             m_frame->loader()->client()->dispatchDidPushStateWithinPage();
    119         else if (stateObjectType == StateObjectReplace)
    120             m_frame->loader()->client()->dispatchDidReplaceStateWithinPage();
    121     }
     117
     118    if (stateObjectType == StateObjectPush)
     119        m_frame->loader()->client()->dispatchDidPushStateWithinPage();
     120    else if (stateObjectType == StateObjectReplace)
     121        m_frame->loader()->client()->dispatchDidReplaceStateWithinPage();
    122122}
    123123
  • trunk/WebCore/page/Page.cpp

    r53361 r53861  
    291291{
    292292    // Abort any current load unless we're navigating the current document to a new state object
    293     if (!item->stateObject() || item->document() != m_mainFrame->document()) {
     293    if (!item->stateObject() || item->documentSequenceNumber() != m_mainFrame->loader()->history()->currentItem()->documentSequenceNumber()) {
    294294        // Define what to do with any open database connections. By default we stop them and terminate the database thread.
    295295        DatabasePolicy databasePolicy = DatabasePolicyStop;
Note: See TracChangeset for help on using the changeset viewer.