[webkit-reviews] review denied: [Bug 5727] We want to evaluate scripts in viewless documents : [Attachment 25249] Fixed version - Should not leak

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 5727: We want to evaluate scripts in viewless documents
https://bugs.webkit.org/show_bug.cgi?id=5727

Attachment 25249: Fixed version - Should not leak
https://bugs.webkit.org/attachment.cgi?id=25249&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
You could ASSERT here that the page is a dummy page:
80     // Changing the Document frame should not occur under normal conditions.

 781	 // Currently only used by the dummyPage mechanism.
 782	 void setFrame(Frame* frame) { m_frame = frame; }

You should ASSERT(document->frame()) after:
     if (!document->frame())
142	     return;
 141	     dummyPage.set(Page::createDummyPage(document));


We're allowing Scripts to execute in viewless documents even though the
user/embedder could have turned them off via settings:

3     // Giving a document implies that we need to execute JavaScript.
 644	 page->settings()->setJavaScriptEnabled(document);

I think dummy page might be the wrong model.  We may need a way to always get
to a page, even if you're not in a frame.  And use that way when executing
scripts.  We can't allow scripts to execute when embedders have turned them
off.