Bug 16588 - ResourceHandle::loadResourceSynchronously needs access to the FrameLoader
Summary: ResourceHandle::loadResourceSynchronously needs access to the FrameLoader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2007-12-23 15:09 PST by Holger Freyther
Modified: 2008-01-10 09:14 PST (History)
4 users (show)

See Also:


Attachments
Add FrameLoader* as parameter to the function (5.65 KB, patch)
2007-12-23 15:12 PST, Holger Freyther
darin: review-
Details | Formatted Diff | Diff
add WebCore::Frame * as parameter to loadResourceSynchronously (5.09 KB, patch)
2008-01-07 06:02 PST, Simon Hausmann
lars.knoll: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2007-12-23 15:09:42 PST
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.
Comment 1 Holger Freyther 2007-12-23 15:12:09 PST
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).
Comment 2 Darin Adler 2007-12-29 22:45:33 PST
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.
Comment 3 Darin Adler 2007-12-30 23:48:24 PST
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.
Comment 4 Simon Hausmann 2008-01-07 06:02:07 PST
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.
Comment 5 Lars Knoll 2008-01-10 06:36:52 PST
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.
Comment 6 Simon Hausmann 2008-01-10 07:48:59 PST
Landed in r29352
Comment 7 Darin Adler 2008-01-10 09:14:46 PST
(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.