Title: [124689] trunk/Source/WebCore
- Revision
- 124689
- Author
- [email protected]
- Date
- 2012-08-03 21:08:43 -0700 (Fri, 03 Aug 2012)
Log Message
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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (124688 => 124689)
--- trunk/Source/WebCore/ChangeLog 2012-08-04 03:25:21 UTC (rev 124688)
+++ trunk/Source/WebCore/ChangeLog 2012-08-04 04:08:43 UTC (rev 124689)
@@ -1,3 +1,43 @@
+2012-08-03 Adam Barth <[email protected]>
+
+ 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):
+
2012-08-03 James Robinson <[email protected]>
[chromium] Stop relying on implicit WebFoo <-> WebCore conversion operators in compositor
Modified: trunk/Source/WebCore/bindings/js/ScriptController.cpp (124688 => 124689)
--- trunk/Source/WebCore/bindings/js/ScriptController.cpp 2012-08-04 03:25:21 UTC (rev 124688)
+++ trunk/Source/WebCore/bindings/js/ScriptController.cpp 2012-08-04 04:08:43 UTC (rev 124689)
@@ -21,6 +21,7 @@
#include "config.h"
#include "ScriptController.h"
+#include "ContentSecurityPolicy.h"
#include "Event.h"
#include "EventNames.h"
#include "Frame.h"
@@ -215,6 +216,9 @@
windowShell->window()->updateDocument();
+ if (m_frame->document())
+ windowShell->window()->setEvalEnabled(m_frame->document()->contentSecurityPolicy()->allowEval(0, ContentSecurityPolicy::SuppressReport));
+
if (Page* page = m_frame->page()) {
attachDebugger(windowShell, page->debugger());
windowShell->window()->setProfileGroup(page->group().identifier());
@@ -237,13 +241,16 @@
{
JSDOMWindowShell* windowShell = existingWindowShell(mainThreadNormalWorld());
if (!windowShell)
- return; // Eval is enabled by default.
+ return;
windowShell->window()->setEvalEnabled(true);
}
void ScriptController::disableEval()
{
- windowShell(mainThreadNormalWorld())->window()->setEvalEnabled(false);
+ JSDOMWindowShell* windowShell = existingWindowShell(mainThreadNormalWorld());
+ if (!windowShell)
+ return;
+ windowShell->window()->setEvalEnabled(false);
}
bool ScriptController::processingUserGesture()
Modified: trunk/Source/WebCore/bindings/v8/ScriptController.cpp (124688 => 124689)
--- trunk/Source/WebCore/bindings/v8/ScriptController.cpp 2012-08-04 03:25:21 UTC (rev 124688)
+++ trunk/Source/WebCore/bindings/v8/ScriptController.cpp 2012-08-04 04:08:43 UTC (rev 124689)
@@ -274,8 +274,6 @@
void ScriptController::enableEval()
{
- // We don't call initContextIfNeeded because contexts have eval enabled by default.
-
v8::HandleScope handleScope;
v8::Handle<v8::Context> v8Context = proxy()->windowShell()->context();
if (v8Context.IsEmpty())
@@ -286,9 +284,6 @@
void ScriptController::disableEval()
{
- if (!proxy()->windowShell()->initContextIfNeeded())
- return;
-
v8::HandleScope handleScope;
v8::Handle<v8::Context> v8Context = proxy()->windowShell()->context();
if (v8Context.IsEmpty())
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (124688 => 124689)
--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-08-04 03:25:21 UTC (rev 124688)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-08-04 04:08:43 UTC (rev 124689)
@@ -32,6 +32,7 @@
#include "V8DOMWindowShell.h"
#include "BindingState.h"
+#include "ContentSecurityPolicy.h"
#include "DateExtension.h"
#include "DocumentLoader.h"
#include "Frame.h"
@@ -338,6 +339,9 @@
setSecurityToken();
+ if (m_frame->document())
+ v8Context->AllowCodeGenerationFromStrings(m_frame->document()->contentSecurityPolicy()->allowEval(0, ContentSecurityPolicy::SuppressReport));
+
m_frame->loader()->client()->didCreateScriptContext(m_context, 0, 0);
// FIXME: This is wrong. We should actually do this for the proper world once
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
