[Webkit-unassigned] [Bug 30155] Factor LoaderPolicy out of FrameLoader

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30155


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #40774|review?                     |review+
               Flag|                            |




--- Comment #3 from Darin Adler <darin at apple.com>  2009-10-07 09:54:47 PDT ---
(From update of attachment 40774)
It's great to do this.

I'm don't think the object should be named "loader policy", because the object
is not a policy. I think it's a "policy xxx" where "xxx" needs to be some other
noun. PolicyChecker perhaps? I know it's elegant to have this have a single
word name, "policy", but I don't think it makes enough logical sense.

> +    // Is the activeDocumentLoader always us?
> +    ASSERT(frameLoader()->activeDocumentLoader());

What is "us" here? This is the main resource loader, not a document loader. I
suppose you mean the document loader that's asking for this main resource load,
so maybe I'm just being too picky.

> +        // FIXME: Seriously?  This is why we can't have nice things.

This code was definitely controversial when first created. I don't want to rain
on the fun, but I don't think the comment adds much. A more sober comment might
carry some information that could help future programmers. It's obvious to you
what's wrong, but might not be to them. Your point is that loading a document
should be possible without having to create a Frame and a FrameView, I suppose.
Or perhaps without so much fakery and so many function calls? As I said, the
point should be obvious, but it's not, even to me!

I'm going to say review+ but I have serious reservations about the name of this
new object, and I'm not entirely sure I understand the boundaries of what does
and does not go in the object.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.