Bug 92189 – Disabling eval changes the timing of DidCreateScriptContext

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Adam Barth]

See http://code.google.com/p/chromium/issues/detail?id=130200

Comment 15 by [e-mail], Today (21 minutes ago)
Specifically, here's how DidCreateScriptContext is invoked when eval is allowed:

	ExtensionDispatcher::DidCreateScriptContext() [0x7f77a03b81fd]
	chrome::ChromeContentRendererClient::DidCreateScriptContext() [0x7f77a030fc50]
	RenderViewImpl::didCreateScriptContext() [0x7f7794e83480]
	WebKit::FrameLoaderClientImpl::didCreateScriptContext() [0x7f7797efc72e]
	WebCore::V8DOMWindowShell::initContextIfNeeded() [0x7f779923424d]
	WebCore::V8Proxy::mainWorldContext() [0x7f779924f9ac]
	WebCore::V8Proxy::mainWorldContext() [0x7f779924fbc1]
	WebCore::ScriptController::evaluate() [0x7f7799200a4e]
	WebCore::ScriptElement::executeScript() [0x7f7798acd5a0]
	WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent() [0x7f7799982f3f]
	WebCore::HTMLScriptRunner::executeParsingBlockingScript() [0x7f7799982d92]
	WebCore::HTMLScriptRunner::executeParsingBlockingScripts() [0x7f77999832be]
	WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad() [0x7f779998345c]
	WebCore::HTMLDocumentParser::notifyFinished() [0x7f77999763d5]
	WebCore::CachedResource::checkNotify() [0x7f7799650a1a]
	WebCore::CachedScript::data() [0x7f77996626cb]
	WebCore::SubresourceLoader::didFinishLoading() [0x7f7799633b59]
	WebCore::ResourceLoader::didFinishLoading() [0x7f779962f2f9]
	WebCore::ResourceHandleInternal::didFinishLoading() [0x7f779904ee46]
	webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest() [0x7f779e28ee36]
	content::ResourceDispatcher::OnRequestComplete() [0x7f7794d7f1b0]
....

And when it's not:

	ExtensionDispatcher::DidCreateScriptContext() [0x7f7096cbd1fd]
	chrome::ChromeContentRendererClient::DidCreateScriptContext() [0x7f7096c14c50]
	RenderViewImpl::didCreateScriptContext() [0x7f708b788480]
	WebKit::FrameLoaderClientImpl::didCreateScriptContext() [0x7f708e80172e]
	WebCore::V8DOMWindowShell::initContextIfNeeded() [0x7f708fb3924d]
	WebCore::ScriptController::disableEval() [0x7f708fb05f93]
	WebCore::Document::disableEval() [0x7f708f31104c]
	WebCore::CSPDirectiveList::create() [0x7f708ff78c71]
	WebCore::ContentSecurityPolicy::didReceiveHeader() [0x7f708ff7bb13]
	WebCore::FrameLoader::didBeginDocument() [0x7f708ff03253]
	WebCore::DocumentWriter::begin() [0x7f708fef684d]
	WebCore::DocumentLoader::commitData() [0x7f708fee6c2f]
	WebKit::WebFrameImpl::commitDocumentData() [0x7f708e860e52]
	WebKit::FrameLoaderClientImpl::committedLoad() [0x7f708e804a87]
	WebCore::DocumentLoader::commitLoad() [0x7f708fee6b6c]
	WebCore::DocumentLoader::receivedData() [0x7f708fee706b]
	WebCore::MainResourceLoader::addData() [0x7f708ff1eb97]
	WebCore::ResourceLoader::didReceiveData() [0x7f708ff339cb]
	WebCore::MainResourceLoader::didReceiveData() [0x7f708ff20044]
	WebCore::ResourceLoader::didReceiveData() [0x7f708ff342b3]
	WebCore::ResourceHandleInternal::didReceiveData() [0x7f708f953c7e]
	webkit_glue::WebURLLoaderImpl::Context::OnReceivedData() [0x7f7094b93a4c]
	content::ResourceDispatcher::OnReceivedData() [0x7f708b683d83]
....

[Adam Barth]

This is on my list for Thursday.

[Adam Barth]

Created attachment 154749 [details]
one approach

[Adam Barth]

Created attachment 154788 [details]
an attempt at a test

[Alexey Proskuryakov]

Is this bug v8-only? Looking at the code, I suspect that JSC is also affected.

[Adam Barth]

> Is this bug v8-only?

Nope.  It affects JSC also.

> Looking at the code, I suspect that JSC is also affected.

Correct.  The issue is that disabling eval based on a header is causing us to boot up JavaScript earlier.  The main thing I'm exploring is how best to fix this issue.  My current thought is to have the bindings check whether eval is disabled when creating the script context.

The complication with this approach is that sometimes we need to disable eval after the script context exists.  That means they'll be two pathways for disabling eval.

[Adam Barth]

Created attachment 154801 [details]
more progress on a test

[Adam Barth]

Created attachment 156480 [details]
Work in progress

[Adam Barth]

The main thing missing from this patch is a test.  Anyone have idea for how to write a test?  My attempts to write a WebKit unit test kind of flopped.  :(

[Adam Barth]

Created attachment 156486 [details]
Patch

[Adam Barth]

Created attachment 156487 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 156487 [details]
Patch

OK.

[WebKit Review Bot]

Comment on attachment 156487 [details]
Patch

Clearing flags on attachment: 156487

Committed r124689: <http://trac.webkit.org/changeset/124689>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.