Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(660)

Issue 10919034: Merge 124689 - Disabling eval changes the timing of DidCreateScriptContext (Closed)

Created:
8 years, 3 months ago by karen
Modified:
8 years, 3 months ago
Reviewers:
abarth
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/1132/
Visibility:
Public.

Description

Merge 124689 - Disabling eval changes the timing of DidCreateScriptContext https://bugs.webkit.org/show_bug.cgi?id=92189 Reviewed by Eric Seidel. When we implemented Content-Security-Policy, we added the ability to disable eval in the JavaScript engine. However, when we process the Content-Security-Policy header, we might not have initialized the script context for the given frame. Previously, we would initialize the context, but that generates a DidCreateScriptContext callback to the embedder earlier in the Document's lifetime that before. A natural thing to do in this callback is to run script to customize the script context, but Document isn't fully initialized yet, which leads to odd bugs and general confusion. In this patch, we delay actually disabling eval until we would have created the scripting context previously. From the perspective of the web platform, this has the same behavior. The only difference is that now the DidCreateScriptContext notification occurs at the same time regardless of whether Content-Security-Policy disables eval. I tried to write a test for this change, but it was unclear to me how to write a good test. I tried writing a Chromium WebKit unit test to no avail. The good news is that this patch will be covered by the PlatformAppBrowserTest.Iframes test in Chromium once https://bugs.webkit.org/show_bug.cgi?id=93079 lands. That's not the best way to test this change, but it might be sufficient. * bindings/js/ScriptController.cpp: (WebCore::ScriptController::initScript): (WebCore::ScriptController::disableEval): * bindings/v8/ScriptController.cpp: (WebCore::ScriptController::enableEval): (WebCore::ScriptController::disableEval): * bindings/v8/V8DOMWindowShell.cpp: (WebCore::V8DOMWindowShell::initContextIfNeeded): TBR=abarth@webkit.org Committed: https://trac.webkit.org/changeset/127285

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -0 lines) Patch
M Source/WebCore/bindings/v8/V8DOMWindowShell.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
karen
8 years, 3 months ago (2012-08-31 17:57:34 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698