Bug 16588 – ResourceHandle::loadResourceSynchronously needs access to the FrameLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Holger Freyther]

With the Qt port/platform a QWebPage can have its own QNetworkAccessManager set. We want all network access for a page/frame to go through this QNetworkAccessManager. For the static ResourceHandle::loadResourceSynchronously function we have currently no way to get the QNetworkAccessManager of the QWebPage. All the current callers of this function do have the necessary information though.

What we did/propose is to add the FrameLoader* as last parameter to the above named function. With the chain of FrameLoader->FrameLoaderClient->QWebFrame we can access the data we need.

[Holger Freyther]

Created attachment 18077 [details]
Add FrameLoader* as parameter to the function

We want to add the FrameLoader* to the loadResourceSynchronously function. This patch adds it and is updating the other ports (or will once landed to trunk).

[Darin Adler]

I suppose this is OK, but it's really a layering violation. We want to build the FrameLoader on top of the ResourceHandle, without a circular dependency.

I'll see if I can think of something better.

[Darin Adler]

Comment on attachment 18077 [details]
Add FrameLoader* as parameter to the function

If you are going to add a parameter, I suggest adding a Frame* rather than a FrameLoader* to match ResourceHandle::create.

But since this is a layering violation, we should figure out a cleaner way to do this.

Perhaps we should have a ResourceHandleFactory class and FrameLoaderClient can return a ResourceHandleFactory. Then you'd use the ResourceHandleFactory to make the ResourceHandle objects.

Or we could have a class with a name like ResourceLoadingContext, add a FrameLoaderClient function that returns one, and we could pass that in to the static members of ResourceHandle.

A typical client could implement both interfaces on the FrameLoaderClient and it could just return "this". This might be enough allow us to remove the Frame* parameter from both functions and remove the circular dependency.

[Simon Hausmann]

Created attachment 18311 [details]
add WebCore::Frame * as parameter to loadResourceSynchronously

Revised patch that uses Frame * instead of FrameLoader * as last argument.

I agree about the use of Frame * instead of FrameLoader. For some reason I didn't see that the ResourceHandle constructor was using a Frame *, too.

[Lars Knoll]

Comment on attachment 18311 [details]
add WebCore::Frame * as parameter to loadResourceSynchronously

Since ResourceHandle::create() also has a pointer to the Frame, I see no reason why loadResourceSynchronously should be any different.

[Simon Hausmann]

Landed in r29352

[Darin Adler]

(In reply to comment #5)
> Since ResourceHandle::create() also has a pointer to the Frame, I see no reason
> why loadResourceSynchronously should be any different.

The Frame* in ResourceHandle::create() *is* the layering violation. I agree that adding it to the synchronous form doesn't make things any worse, but our goal is to eliminate this in the longer run and come up with a better way to create the resource handles with the right context that allows it to be a lower-level I/O subsystem rather than yet another part of the massive super-object Frame.