[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 #44 from Charles Reis <creis at chromium.org>  2011-01-13 09:42:59 PST ---
(In reply to comment #43)
> (From update of attachment 75044 [details])
> Sorry this patch has been sitting here with an r+ for awhile and that I'm just doing this, but reading my bugzilla CC email I noticed something I really dislike here.
> 
> I understand the motivation for reordering the arguments was the "null vs bool" ambiguity.  A much better way to fix that besides reordering the arguments would be to take this opportunity to replace the bool with an enum.  
> 
> Since it's been so long I'm going to change to r- in hopes this can be done before landing.

Ok, I'll take a look.

I also tried to bring the patch up to date and discovered there's another problem.  Some clients (e.g., DumpRenderTree but not test_shell or chromium) will re-use the same HistoryItem for repeated back navigations.  That means if you click back twice, we can't distinguish between the two navigations and thus interpret it as a stop.  Drat.

Now that the fix for 50254 has landed and ensured we have provisional items in the history for all frames, I'd like to approach this another way.  Rather than trying to determine whether a new navigation has begun and skipping the state reset, we should only do the state reset if we know that no navigation is in progress.  We could detect that if history()->provisionalItem() was set to 0 after a stop.

I'll see if I can get that approach to work.

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