[webkit-reviews] review denied: [Bug 5727] We want to evaluate scripts in viewless documents : [Attachment 21540] Draft - No test case (will be added on the final version) - Should also have used 'svn mv'

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Julien Chaffraix
<julien.chaffraix at gmail.com>'s request for review:
Bug 5727: We want to evaluate scripts in viewless documents
https://bugs.webkit.org/show_bug.cgi?id=5727

Attachment 21540: Draft - No test case (will be added on the final version) -
Should also have used 'svn mv'
https://bugs.webkit.org/attachment.cgi?id=21540&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Factoring out the empty clients is great. Maybe it would be best to do that in
a separate check-in, first.

Why does the setDummyFrame function have a "shouldEnableJavaScript" boolean
parameter? All the callers pass true.

I'm not sure setDummyFrame should be a Document member function. It seems to me
that this is more of a controller operation, and we should try to avoid adding
more controller functions into the model object, Document.

Perhaps it should just be a static member of FrameLoader that takes a Document
argument.

The fakeLoad function definitely should *not* be added to Frame. This clearly
belongs on FrameLoader.

The dummyFrame function should return PassRefPtr<Frame> rather than just a
Frame*. I think the function's name should have a verb in it, such as
createDummyFrame. While it's marginally OK to have this on Frame, I think this
function, too, would be better on FrameLoader, which has the responsibility to
create new frames. The Frame class itself is just the "hub" of Frame features,
and should have very little code in it.