Bug 43282 - The currentItem fixup done by BackForwardList::pushStateItem should be moved into HistoryController::pushState
Summary: The currentItem fixup done by BackForwardList::pushStateItem should be moved ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-30 16:14 PDT by Darin Fisher (:fishd, Google)
Modified: 2010-08-10 22:51 PDT (History)
5 users (show)

See Also:


Attachments
v1 patch (9.60 KB, patch)
2010-07-30 16:49 PDT, Darin Fisher (:fishd, Google)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2010-07-30 16:14:32 PDT
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.
Comment 1 Darin Fisher (:fishd, Google) 2010-07-30 16:49:04 PDT
Created attachment 63124 [details]
v1 patch
Comment 2 Darin Fisher (:fishd, Google) 2010-07-30 16:52:00 PDT
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.
Comment 3 Darin Adler 2010-07-30 18:16:10 PDT
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"!
Comment 4 Darin Fisher (:fishd, Google) 2010-07-30 20:07:59 PDT
(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 ;-)
Comment 5 WebKit Review Bot 2010-07-30 21:03:32 PDT
http://trac.webkit.org/changeset/64402 might have broken SnowLeopard Intel Release (Tests)
Comment 6 Darin Fisher (:fishd, Google) 2010-07-30 21:10:46 PDT
(In reply to comment #5)
> http://trac.webkit.org/changeset/64402 might have broken SnowLeopard Intel Release (Tests)

^^^ fixed
Comment 7 Adam Barth 2010-08-10 22:51:50 PDT
This patch appears to have been landed.