1 files changed, 68 insertions, 33 deletions

diff --git a/Source/WebCore/loader/HistoryController.cpp b/Source/WebCore/loader/HistoryController.cpp
index dda4e56..7c0fc97 100644
--- a/Source/WebCore/loader/HistoryController.cpp
+++ b/Source/WebCore/loader/HistoryController.cpp
@@ -260,6 +260,10 @@ void HistoryController::updateForBackForwardNavigation()
     // Must grab the current scroll position before disturbing it
     if (!m_frameLoadComplete)
         saveScrollPositionAndViewStateToItem(m_previousItem.get());
+
+    // When traversing history, we may end up redirecting to a different URL
+    // this time (e.g., due to cookies).  See http://webkit.org/b/49654.
+    updateCurrentItem();
 }
 
 void HistoryController::updateForReload()
@@ -274,11 +278,11 @@ void HistoryController::updateForReload()
     
         if (m_frame->loader()->loadType() == FrameLoadTypeReload || m_frame->loader()->loadType() == FrameLoadTypeReloadFromOrigin)
             saveScrollPositionAndViewStateToItem(m_currentItem.get());
-    
-        // Sometimes loading a page again leads to a different result because of cookies. Bugzilla 4072
-        if (m_frame->loader()->documentLoader()->unreachableURL().isEmpty())
-            m_currentItem->setURL(m_frame->loader()->documentLoader()->requestURL());
     }
+
+    // When reloading the page, we may end up redirecting to a different URL
+    // this time (e.g., due to cookies).  See http://webkit.org/b/4072.
+    updateCurrentItem();
 }
 
 // There are 3 things you might think of as "history", all of which are handled by these functions.
@@ -310,9 +314,9 @@ void HistoryController::updateForStandardLoad(HistoryUpdateType updateType)
             if (Page* page = m_frame->page())
                 page->setGlobalHistoryItem(needPrivacy ? 0 : page->backForward()->currentItem());
         }
-    } else if (frameLoader->documentLoader()->unreachableURL().isEmpty() && m_currentItem) {
-        m_currentItem->setURL(frameLoader->documentLoader()->url());
-        m_currentItem->setFormInfoFromRequest(frameLoader->documentLoader()->request());
+    } else {
+        // The client redirect replaces the current history item.
+        updateCurrentItem();
     }
 
     if (!historyURL.isEmpty() && !needPrivacy) {
@@ -349,14 +353,12 @@ void HistoryController::updateForRedirectWithLockedBackForwardList()
                     page->setGlobalHistoryItem(needPrivacy ? 0 : page->backForward()->currentItem());
             }
         }
-        if (m_currentItem) {
-            m_currentItem->setURL(m_frame->loader()->documentLoader()->url());
-            m_currentItem->setFormInfoFromRequest(m_frame->loader()->documentLoader()->request());
-        }
+        // The client redirect replaces the current history item.
+        updateCurrentItem();
     } else {
         Frame* parentFrame = m_frame->tree()->parent();
         if (parentFrame && parentFrame->loader()->history()->m_currentItem)
-            parentFrame->loader()->history()->m_currentItem->setChildItem(createItem(true));
+            parentFrame->loader()->history()->m_currentItem->setChildItem(createItem());
     }
 
     if (!historyURL.isEmpty() && !needPrivacy) {
@@ -506,12 +508,13 @@ void HistoryController::setProvisionalItem(HistoryItem* item)
     m_provisionalItem = item;
 }
 
-PassRefPtr<HistoryItem> HistoryController::createItem(bool useOriginal)
+void HistoryController::initializeItem(HistoryItem* item)
 {
     DocumentLoader* documentLoader = m_frame->loader()->documentLoader();
-    
-    KURL unreachableURL = documentLoader ? documentLoader->unreachableURL() : KURL();
-    
+    ASSERT(documentLoader);
+
+    KURL unreachableURL = documentLoader->unreachableURL();
+
     KURL url;
     KURL originalURL;
 
@@ -519,15 +522,10 @@ PassRefPtr<HistoryItem> HistoryController::createItem(bool useOriginal)
         url = unreachableURL;
         originalURL = unreachableURL;
     } else {
-        originalURL = documentLoader ? documentLoader->originalURL() : KURL();
-        if (useOriginal)
-            url = originalURL;
-        else if (documentLoader)
-            url = documentLoader->requestURL();
+        url = documentLoader->url();
+        originalURL = documentLoader->originalURL();
     }
 
-    LOG(History, "WebCoreHistory: Creating item for %s", url.string().ascii().data());
-    
     // Frames that have never successfully loaded any content
     // may have no URL at all. Currently our history code can't
     // deal with such things, so we nip that in the bud here.
@@ -540,21 +538,25 @@ PassRefPtr<HistoryItem> HistoryController::createItem(bool useOriginal)
     
     Frame* parentFrame = m_frame->tree()->parent();
     String parent = parentFrame ? parentFrame->tree()->uniqueName() : "";
-    String title = documentLoader ? documentLoader->title() : "";
+    String title = documentLoader->title();
 
-    RefPtr<HistoryItem> item = HistoryItem::create(url, m_frame->tree()->uniqueName(), parent, title);
+    item->setURL(url);
+    item->setTarget(m_frame->tree()->uniqueName());
+    item->setParent(parent);
+    item->setTitle(title);
     item->setOriginalURLString(originalURL.string());
 
-    if (!unreachableURL.isEmpty() || !documentLoader || documentLoader->response().httpStatusCode() >= 400)
+    if (!unreachableURL.isEmpty() || documentLoader->response().httpStatusCode() >= 400)
         item->setLastVisitWasFailure(true);
 
     // Save form state if this is a POST
-    if (documentLoader) {
-        if (useOriginal)
-            item->setFormInfoFromRequest(documentLoader->originalRequest());
-        else
-            item->setFormInfoFromRequest(documentLoader->request());
-    }
+    item->setFormInfoFromRequest(documentLoader->request());
+}
+
+PassRefPtr<HistoryItem> HistoryController::createItem()
+{
+    RefPtr<HistoryItem> item = HistoryItem::create();
+    initializeItem(item.get());
     
     // Set the item for which we will save document state
     m_frameLoadComplete = false;
@@ -566,7 +568,7 @@ PassRefPtr<HistoryItem> HistoryController::createItem(bool useOriginal)
 
 PassRefPtr<HistoryItem> HistoryController::createItemTree(Frame* targetFrame, bool clipAtTarget)
 {
-    RefPtr<HistoryItem> bfItem = createItem(m_frame->tree()->parent() ? true : false);
+    RefPtr<HistoryItem> bfItem = createItem();
     if (!m_frameLoadComplete)
         saveScrollPositionAndViewStateToItem(m_previousItem.get());
 
@@ -656,6 +658,14 @@ void HistoryController::recursiveGoToItem(HistoryItem* item, HistoryItem* fromIt
 
 bool HistoryController::itemsAreClones(HistoryItem* item1, HistoryItem* item2) const
 {
+    // It appears that one of the items can be null in release builds, leading
+    // to the crashes seen in http://webkit.org/b/52819.  For now, try to
+    // narrow it down with a more specific crash.
+    if (!item1)
+        CRASH();
+    if (!item2)
+        CRASH();
+
     // If the item we're going to is a clone of the item we're at, then we do
     // not need to load it again.  The current frame tree and the frame tree
     // snapshot in the item have to match.
@@ -713,6 +723,31 @@ void HistoryController::updateBackForwardListClippedAtTarget(bool doClip)
     page->backForward()->addItem(topItem.release());
 }
 
+void HistoryController::updateCurrentItem()
+{
+    if (!m_currentItem)
+        return;
+
+    DocumentLoader* documentLoader = m_frame->loader()->documentLoader();
+
+    if (!documentLoader->unreachableURL().isEmpty())
+        return;
+
+    if (m_currentItem->url() != documentLoader->url()) {
+        // We ended up on a completely different URL this time, so the HistoryItem
+        // needs to be re-initialized.  Preserve the isTargetItem flag as it is a
+        // property of how this HistoryItem was originally created and is not
+        // dependent on the document.
+        bool isTargetItem = m_currentItem->isTargetItem();
+        m_currentItem->reset();
+        initializeItem(m_currentItem.get());
+        m_currentItem->setIsTargetItem(isTargetItem);
+    } else {
+        // Even if the final URL didn't change, the form data may have changed.
+        m_currentItem->setFormInfoFromRequest(documentLoader->request());
+    }
+}
+
 void HistoryController::pushState(PassRefPtr<SerializedScriptValue> stateObject, const String& title, const String& urlString)
 {
     if (!m_currentItem)