[Webkit-unassigned] [Bug 48812] FrameLoader::checkLoadCompleteForThisFrame uses wrong history item

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48812


Charles Reis <creis at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #74305|review?                     |
               Flag|                            |




--- Comment #38 from Charles Reis <creis at chromium.org>  2010-11-29 10:27:53 PST ---
(From update of attachment 74305)
View in context: https://bugs.webkit.org/attachment.cgi?id=74305&action=review

>> WebCore/loader/FrameLoader.cpp:2380
>> +                    if (pdl->triggeringAction().historyItem() == page->backForward()->currentItem()) {
> 
> Can we make use HistoryController::currentItem() or perhaps HistoryController::provisionalItem()?
> It seems that in the back/forward navigation case, HistoryController::goToItem sets
> HistoryController::m_provisionalItem, and then when we commit the load, that gets promoted to
> HistoryController::m_currentItem.  If another load comes in after that, then the BackForwardList's
> currentItem would change to correspond to the new provisional HistoryItem.
> 
> I'm also a bit confused as to why this code only assigns |item| when the current frame is the
> main frame.  This makes me wonder if calling BackForwardList::setCurrentItem here is really
> the right thing to do.  If a navigation only occurs in a subframe, we'd still want to
> potentially update the BackForwardList.

Yes, I think history()->provisionalItem() should be the same as backForward()->currentItem() here, and it's a bit clearer to understand that way.  I've updated the comment as well to make this easier to follow.  I'll upload that once the tests pass.

I'm less sure about the frame issue.  I'll take a look while the tests are running, but we may want to tackle that after this fix goes in.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.