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

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from Darin Adler <darin at apple.com>  2010-11-17 10:11:26 PST ---
(From update of attachment 73550)
View in context: https://bugs.webkit.org/attachment.cgi?id=73550&action=review

This change looks OK. I think we might want to refine the coding of it a bit.

> WebCore/loader/FrameLoader.cpp:2385
> +                    if (!pdl->triggeringAction().isEmpty()
> +                        && pdl->triggeringAction().historyItem() == page->backForward()->currentItem()) {

Why do we need the isEmpty check? Presumably if the action is empty then the history item will be 0. Can currentItem be 0?

I’d prefer to not have the isEmpty function.

> WebCore/loader/FrameLoader.cpp:3207
> +            action = NavigationAction(itemURL, loadType, false, 0, item);

With all the overloads for creating a NavigationAction, for some reason we have to specify both false and 0 here. Maybe we should do different overloads instead?

> WebCore/loader/FrameLoader.cpp:3235
> +        action = NavigationAction(itemOriginalURL, loadType, false, 0, item);

Same thing happens again here. I think we should consider whether we can do a different overload.

> WebCore/loader/NavigationAction.h:48
>          NavigationAction();
>          NavigationAction(const KURL&, NavigationType);
> +        NavigationAction(const KURL&, NavigationType, PassRefPtr<HistoryItem>);
>          NavigationAction(const KURL&, FrameLoadType, bool isFormSubmission);
>          NavigationAction(const KURL&, NavigationType, PassRefPtr<Event>);
>          NavigationAction(const KURL&, FrameLoadType, bool isFormSubmission, PassRefPtr<Event>);
> +        NavigationAction(const KURL&, FrameLoadType, bool isFormSubmission, PassRefPtr<Event>, PassRefPtr<HistoryItem>);

These are sorted in a strange order that makes it hard to read them. Also, I think we can use default argument values to cut down a bit on the overloading. Both PassRefPtr<Event> and PassRefPtr<HistoryItem> can have a default value of 0.

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