The currentItem fixup done by BackForwardList::pushStateItem should be moved into HistoryController::pushState

• Component: Page Loading
• Status: RESOLVED FIXED

[Darin Fisher (:fishd, Google)]

The currentItem fixup done by BackForwardList::pushStateItem should be moved into HistoryController::pushState

The BackForwardList operates on the top-most HistoryItem node.  The HistoryController operates on the HistoryItem corresponding to a particular frame.

[Darin Fisher (:fishd, Google)]

Created attachment 63124 [details]
v1 patch

[Darin Fisher (:fishd, Google)]

Note: the dereference of m_previousItem in HistoryController::pushState is protected by the null check of m_currentItem at the top of the function.  This is because createItemTree has the side-effect of assigning m_previousItem.

[Darin Adler]

Comment on attachment 63124 [details]
v1 patch

> -    HistoryItem* targetItem = m_frame->loader()->history()->currentItem();

LOL, m_frame->loader()->history() is a roundabout way of writing "this"!

[Darin Fisher (:fishd, Google)]

(In reply to comment #3)
> (From update of attachment 63124 [details])
> > -    HistoryItem* targetItem = m_frame->loader()->history()->currentItem();
> 
> LOL, m_frame->loader()->history() is a roundabout way of writing "this"!

Yup, and I was so proud of myself for avoiding HistoryItem::targetItem() too ;-)

[WebKit Review Bot]

http://trac.webkit.org/changeset/64402 might have broken SnowLeopard Intel Release (Tests)

[Darin Fisher (:fishd, Google)]

(In reply to comment #5)
> http://trac.webkit.org/changeset/64402 might have broken SnowLeopard Intel Release (Tests)

^^^ fixed

[Adam Barth]

This patch appears to have been landed.