[webkit-dev] How loading a frame ought to work

[Adam Barth]

On Tue, Oct 13, 2009 at 1:30 PM, Darin Adler <darin at apple.com> wrote:
> On Oct 13, 2009, at 10:50 AM, Adam Barth wrote:
>> * MainResourceLoader. MainResourceLoader inherits from ResourceLoader and
>> receive callbacks from the network stack. It's primary task is to take
>> callbacks from the network stack and route them to the appropriate
>> higher-level APIs. Currently, MainResourceLoader is also concerned with
>> navigation policies, but I'm not sure if that's correct.
>
> I don’t think it’s right that the code to deal with content policy is in
> MainResourceLoader. I think that code belongs in DocumentLoader or
> FrameLoader instead.

Ok.  I'll try to move this too.  It seems MainResourceLoader primary
purpose is to have it's lifetime tied to the network stack (and not to
the Frame), so that it can detach from the frame and swallow
notifications from the network stack.  I can try to move the object
more purely in that direction and more the policy bits to an object
with Frame or Document lifetime.

>> * DocumentLoader.  DocumentLoader is responsible for all the
>> document-specific loading state. For example, the DocumentLoader should know
>> if we've received any data for this document yet and should be in charge of
>> actually stuffing the received data into the Document. (Both of these
>> examples are currently done by FrameLoader.)
>
> That sounds right. Much of the code currently in FrameLoader should go into
> DocumentLoader. Anything about a single document and not about transitions
> between documents should go there.

Understood.  My plan is to move begin / write / end and friends into
DocumentLoader as a start.

>> 2) Policy Check. Ask the FrameLoaderClient's policy interface whether the
>> embedder actually wants to go through with the load and wait for a response.
>
> Some policy checks do happen at this point. But note that there are other
> policy checks that occur later. For example, the content policy can’t be
> checked until we receive data from the network.

I see.  I'm not sure what to call this phase, but these phases seem to
correspond to the three DocumentLoaders that FrameLoader holds:

http://trac.webkit.org/browser/trunk/WebCore/loader/FrameLoader.h#L457

The word "policy" is very overloaded here.  For example, the
third-party cookie blocking logic used to speak about policy URLs.

>> A) FrameNavigator. This object would handle the dispatching phase above.
>> The goal of creating this object is to let FrameLoader focus on load
>> requests that will actually result in network / cache traffic.
>
> This seems like a good idea, but I think the boundary will be tricky. For
> example, is detecting when a page load is complete including subresources
> something the frame navigator would be responsible for or the loader? What
> about loading from the back/forward cache as opposed to the network?
>
> A list of what functions and data would go in each class might help me
> understand if this refactoring is good.

Hum...  Looking at this again, it's less clear.  It might be better
for me to propose a concrete patch, which we can discuss in a bug.

In general, FrameLoader does a bunch of work to scroll the document's
view to a fragment, but that work doesn't seem related to
transitioning between documents.  Maybe that's a better part to break
off first.

>> B) LoadObserver. Currently ResourceLoader forwards many of the network
>> stack callbacks (e.g., ResourceLoader::didReceiveResponse) to FrameLoader so
>> they can be farmed out to the Web Inspector, ProgressTracker,
>> FrameLoaderClient, and others. The goal of this object is to remove that
>> complexity from FrameLoader.
>
> There is a distinction between resource load client calls and higher level
> load client calls. The resource load client calls are the ones that use a
> request identifier. And these are indeed all about the resource loader.
>
> But many other client calls do not involve the resource loader directly. If
> you don’t plan to make the LoadObserver responsible for these, then I think
> it should probably be called the ResourceLoadObserver, since it observes the
> lower level loading of resources but not the higher level loading of web
> pages.
>
> I’m also not entirely sure if “observer” is the right name here, but I have
> nothing better to suggest at this time.

The block of methods I was think of are the ones that take a
ResourceLoader as a parameter:

http://trac.webkit.org/browser/trunk/WebCore/loader/FrameLoader.h#L143

These don't appear to interact very much with FrameLoader state.
Instead, they seem tightly coupled with similarly named methods that
take a DocumentLoader and an identifier.

http://trac.webkit.org/browser/trunk/WebCore/loader/FrameLoader.h#L403

The general workflow seems to be that FrameLoader translates calls of
the first kind into calls of the second kind and then notifies a bunch
of folks that the callback happened.  Maybe ResourceLoadNotifier is a
better name because the object is not consuming the notifications but
instead routing them to the interested objects.

Adam