[Webkit-unassigned] [Bug 54517] Ensure loading has stopped in HistoryController::goToItem

[bugzilla-daemon at webkit.org]

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


Mihai Parparita <mihaip at chromium.org> changed:

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




--- Comment #8 from Mihai Parparita <mihaip at chromium.org>  2011-02-17 17:53:06 PST ---
(From update of attachment 82860)
View in context: https://bugs.webkit.org/attachment.cgi?id=82860&action=review

> Source/WebCore/page/Page.cpp:341
> +#if PLATFORM(CHROMIUM)

This feels pretty icky. Since we're not under the gun to get this into M10, I'd like to explore a cleaner solution.  The current check for the back-forward scheme is done by Chromium's implementation of FrameLoaderClient::shouldGoToHistoryItem. Adding a FrameLoaderClient::shouldStopLoadingForHistoryItem seems reasonable.

Additionally, having all this history logic in Page seems strange. Putting all these checks (lines 335 to 343) in a static HistoryController::shouldStopLoadingForHistoryItemTransition(currentItem, item) method may be cleaner.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:895
> +    // Use Page::goToItem to ensure stopAllLoaders is called first (except for

Can we hide HistoryController::goToItem so that all calls go through Page (make it private and make Page a friend?)

> LayoutTests/http/tests/navigation/resources/forward-to-fragment-fires-onload-2.html:31
> +    setTimeout(function() {

Shouldn't be necessary to do this in a timeout, history.forward() is already async.

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