[webkit-reviews] review denied: [Bug 48809] Chromium WebKit API needs WebFrame::lastCommittedHistoryItem() : [Attachment 72605] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Charles Reis
<creis at chromium.org>'s request for review:
Bug 48809: Chromium WebKit API needs WebFrame::lastCommittedHistoryItem()
https://bugs.webkit.org/show_bug.cgi?id=48809

Attachment 72605: Patch
https://bugs.webkit.org/attachment.cgi?id=72605&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72605&action=review

> WebKit/chromium/src/WebFrameImpl.cpp:1006
> +    return WebHistoryItem(viewImpl()->lastCommittedHistoryItem());

I think you can implement this using
m_frame->loader()->history()->currentItem()
as that provides you with the last-committed history item for this frame.

This would allow you to eliminate a lot of code in this patch.

Come to think of it, I suspect that WebFrameImpl::currentHistoryItem is
incorrectly implemented.  It is incorrectly accessing the currentItem of
the BackForwardList, which corresponds to the HistoryItem of the top-most
frame.	If a user of the WebKit API wants the HistoryItem of the top-most
frame, then they should just use WebFrame::top()->currentHistoryItem().

In other words, currentHistoryItem is supposed to mean the same thing as
lastCommittedHistoryItem.