Bug 143155 - Frame::stopAllLoaders() does not always cancel scheduled redirects
Summary: Frame::stopAllLoaders() does not always cancel scheduled redirects
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-03-27 14:54 PDT by Brady Eidson
Modified: 2015-03-27 15:30 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-03-27 14:54:20 PDT
Frame::stopAllLoaders() does not always cancel scheduled redirects

The developer impact is that using API to stop a frame from loading (e.g. WKBundleFrameStopLoading) doesn't actually halt an already scheduled redirect.
The user impact is that hitting the "stop" button in their browser - or performing some other user gesture to freeze loading on the current page - will not always cancel a scheduled redirect away from the current page.

----

Calling FrameLoader::stopLoading() on each FrameLoader is an important part of "stop all loaders" but unfortunately it doesn't always get called.

stopAllLoaders() walks the frame tree, calling in to each Frame, which calls stopLoading() on their DocumentLoaders.

DocumentLoaders are who are in charge of calling FrameLoader::stopLoading() directly for their Frame, but they often do not actually call it.

There is a (apparently long standing) optimization to only call stopLoading() on the Frame if the DocumentLoader is currently loading or parsing. Unfortunately, the DocumentLoader can be done loading and parsing while its Frame still has a "load" active in the form of a scheduled redirect.

I propose DocumentLoader be much less conditional about when it tells its FrameLoader to stopLoading(), and looking at the body of FrameLoader::stopLoading() suggests there will be no negative side effects *besides* actually cancelling scheduled redirects.

This will all make more sense with the upcoming patch.

rdar://problem/20331979
Comment 1 Brady Eidson 2015-03-27 15:30:50 PDT
*sigh*

Nevermind.

A single layouttest that broke with this change revealed why we cannot make it without causing much larger disruption.

That change would be inappropriate at this time.