Bug 63295 – Remove some unneeded functions from FrameLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Darin Adler]

Remove some unneeded functions from FrameLoader

[Darin Adler]

Created attachment 98445 [details]
Patch

[WebKit Review Bot]

Comment on attachment 98445 [details]
Patch

Attachment 98445 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8934306

[WebKit Review Bot]

Comment on attachment 98445 [details]
Patch

Attachment 98445 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8935230

[WebKit Review Bot]

Comment on attachment 98445 [details]
Patch

Attachment 98445 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8935238

[Darin Adler]

Comment on attachment 98445 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98445&action=review

> Source/WebCore/loader/FrameLoader.h:-283
> -    bool suppressOpenerInNewFrame() const { return m_suppressOpenerInNewFrame; }

Looks like this is actually Chromium-only rather than unused.

[Adam Barth]

Comment on attachment 98445 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98445&action=review

R+, modulo compile errors, of course.

> Source/WebCore/loader/FrameLoader.cpp:1570
> -    stopLoadingSubframes(clearProvisionalItemPolicy);
> +    for (RefPtr<Frame> child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())
> +        child->loader()->stopAllLoaders(clearProvisionalItemPolicy);

Inlining this function is very helpful.

> Source/WebCore/loader/FrameLoader.h:159
> -    // FIXME: Move this method to ResourceLoader with the rest of the ResourceError accessors.
>      ResourceError cancelledError(const ResourceRequest&) const;

It seems like this function should be removed.  Are there clients of this method that aren't better off calling the ResourceLoader version?

[Darin Adler]

Comment on attachment 98445 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98445&action=review

>> Source/WebCore/loader/FrameLoader.h:159
>>      ResourceError cancelledError(const ResourceRequest&) const;
> 
> It seems like this function should be removed.  Are there clients of this method that aren't better off calling the ResourceLoader version?

I thought the same thing, believed the FIXME removed it, and then learned how this error is different from the other resource errors.

There are multiple places where a cancelled error has to be synthesized when there is no resource loader. In WebCore, this happens in DocumentLoader::stopLoading, FrameLoader::requestFromDelegate, and possibly in PluginStream::stop.

While there may be some good way to deal with this, simply removing the function isn’t clearly the correct step forward, which is why I removed the FIXME.

[Adam Barth]

Ok.  That explains why it was harder to move than the other ones.  :)

[Darin Adler]

Committed r89715: <http://trac.webkit.org/changeset/89715>