Bug 92189

Summary: Disabling eval changes the timing of DidCreateScriptContext
Product: WebKit Reporter: Adam Barth <abarth>
Component: WebCore JavaScriptAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, eric, haraken, japhet, jochen, johnjbarton, mihaip, webkit.review.bot
Priority: P2    
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

Adam Barth
Reported 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] ....
Attachments
one approach (2.67 KB, patch)
2012-07-26 14:17 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
an attempt at a test (2.34 KB, patch)
2012-07-26 16:47 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
more progress on a test (3.23 KB, patch)
2012-07-26 18:10 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Work in progress (4.40 KB, patch)
2012-08-03 16:40 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (5.72 KB, patch)
2012-08-03 17:34 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (5.92 KB, patch)
2012-08-03 17:35 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-07-26 02:06:03 PDT
This is on my list for Thursday.
Adam Barth
Comment 2 2012-07-26 14:17:48 PDT
Created attachment 154749 [details] one approach
Adam Barth
Comment 3 2012-07-26 16:47:47 PDT
Created attachment 154788 [details] an attempt at a test
Alexey Proskuryakov
Comment 4 2012-07-26 17:16:38 PDT
Is this bug v8-only? Looking at the code, I suspect that JSC is also affected.
Adam Barth
Comment 5 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.
Adam Barth
Comment 6 2012-07-26 18:10:30 PDT
Created attachment 154801 [details] more progress on a test
Adam Barth
Comment 7 2012-08-03 16:40:12 PDT
Created attachment 156480 [details] Work in progress
Adam Barth
Comment 8 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. :(
Adam Barth
Comment 9 2012-08-03 17:34:27 PDT
Created attachment 156486 [details] Patch
Adam Barth
Comment 10 2012-08-03 17:35:03 PDT
Created attachment 156487 [details] Patch
Eric Seidel (no email)
Comment 11 2012-08-03 18:50:09 PDT
Comment on attachment 156487 [details] Patch OK.
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2012-08-03 21:09:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.