Bug 92189

Summary:

Disabling eval changes the timing of DidCreateScriptContext

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

WebCore JavaScript

Assignee:

Adam Barth <abarth>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, beidson, eric, haraken, japhet, jochen, johnjbarton, mihaip, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

93079

Attachments:

Description	Flags
one approach	none
an attempt at a test	none
more progress on a test	none
Work in progress	none
Patch	none
Patch	none

Description Adam Barth 2012-07-24 18:06:55 PDT

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

Comment 15 by mihaip@chromium.org, 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]
....

Comment 1 Adam Barth 2012-07-26 02:06:03 PDT

This is on my list for Thursday.

Comment 2 Adam Barth 2012-07-26 14:17:48 PDT

Created attachment 154749 [details]
one approach

Comment 3 Adam Barth 2012-07-26 16:47:47 PDT

Created attachment 154788 [details]
an attempt at a test

Comment 4 Alexey Proskuryakov 2012-07-26 17:16:38 PDT

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

Comment 5 Adam Barth 2012-07-26 17:25:33 PDT

> 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.

Comment 6 Adam Barth 2012-07-26 18:10:30 PDT

Created attachment 154801 [details]
more progress on a test

Comment 7 Adam Barth 2012-08-03 16:40:12 PDT

Created attachment 156480 [details]
Work in progress

Comment 8 Adam Barth 2012-08-03 16:41:47 PDT

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.  :(

Comment 9 Adam Barth 2012-08-03 17:34:27 PDT

Created attachment 156486 [details]
Patch

Comment 10 Adam Barth 2012-08-03 17:35:03 PDT

Created attachment 156487 [details]
Patch

Comment 11 Eric Seidel (no email) 2012-08-03 18:50:09 PDT

Comment on attachment 156487 [details]
Patch

OK.

Comment 12 WebKit Review Bot 2012-08-03 21:08:59 PDT

Comment on attachment 156487 [details]
Patch

Clearing flags on attachment: 156487

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

Comment 13 WebKit Review Bot 2012-08-03 21:09:04 PDT

All reviewed patches have been landed.  Closing bug.