Crash when reloading a Chromium "platform" app
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Aug 2012 20:03:37 +0000 (20:03 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Aug 2012 20:03:37 +0000 (20:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93497

Reviewed by Eric Seidel.

Source/WebCore:

The framework for Chromium "platform" apps executes a big blob of
script during the didCreateScriptContext callback. This blob of scripts
interacts with a bunch of JavaScript objects and triggers a number of
security checks.

When reloading a frame, the didCreateScriptContext is called during
Frame::setDocument (as a consequence of calling
ScriptController::updateDocument). At that time, the SecurityOrigin
object hasn't yet been copied over to the DOMWindow, and we crash
trying to grab it.

The long-term fix for this bug is to fix
https://bugs.webkit.org/show_bug.cgi?id=75793, at which point there
will no longer be a SecurityOrigin object on DOMWindow. In the
meantime, however, we can fix this crash by null checking the
DOMWindow's SecurityOrigin object.

* bindings/generic/BindingSecurity.cpp:
(WebCore::canAccessDocument):

Source/WebKit/chromium:

Test that we don't crash when executing script during the
didCreateScriptContext callback.

* tests/WebFrameTest.cpp:
* tests/data/hello_world.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@125077 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/generic/BindingSecurity.cpp
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/tests/WebFrameTest.cpp
Source/WebKit/chromium/tests/data/hello_world.html [new file with mode: 0644]

index 3e17520..0f51a5c 100644 (file)
@@ -1,3 +1,30 @@
+2012-08-08  Adam Barth  <abarth@webkit.org>
+
+        Crash when reloading a Chromium "platform" app
+        https://bugs.webkit.org/show_bug.cgi?id=93497
+
+        Reviewed by Eric Seidel.
+
+        The framework for Chromium "platform" apps executes a big blob of
+        script during the didCreateScriptContext callback. This blob of scripts
+        interacts with a bunch of JavaScript objects and triggers a number of
+        security checks.
+
+        When reloading a frame, the didCreateScriptContext is called during
+        Frame::setDocument (as a consequence of calling
+        ScriptController::updateDocument). At that time, the SecurityOrigin
+        object hasn't yet been copied over to the DOMWindow, and we crash
+        trying to grab it.
+
+        The long-term fix for this bug is to fix
+        https://bugs.webkit.org/show_bug.cgi?id=75793, at which point there
+        will no longer be a SecurityOrigin object on DOMWindow. In the
+        meantime, however, we can fix this crash by null checking the
+        DOMWindow's SecurityOrigin object.
+
+        * bindings/generic/BindingSecurity.cpp:
+        (WebCore::canAccessDocument):
+
 2012-08-08  Dean Jackson  <dino@apple.com>
 
         Unreviewed build fix for Mac port after http://trac.webkit.org/changeset/125051
index 36c9553..87f4a57 100644 (file)
@@ -51,6 +51,16 @@ static bool canAccessDocument(BindingState* state, Document* targetDocument, Sec
     if (!active)
         return false;
 
+    // If the embedder executes JavaScript synchronously during the didCreateScriptContext callback,
+    // in some cases the active SecurityOrigin will not yet be copied to the DOMWindow. For example,
+    // Frame::setDocument can trigger didCreateScriptContext during ScriptController::updateDocument.
+    //
+    // FIXME: Remove this branch once we manage to delete DOMWindow::m_securityOrigin. Ideally, we'd
+    //        get the SecurityOrigin from the Document rather than the DOMWindow. In that case, there
+    //        shouldn't ever be a chance to execute script before the SecurityOrigin object is created.
+    if (!active->securityOrigin())
+        return false;
+
     if (active->securityOrigin()->canAccess(targetDocument->securityOrigin()))
         return true;
 
index 3791035..876c0dc 100644 (file)
@@ -1,3 +1,16 @@
+2012-08-08  Adam Barth  <abarth@webkit.org>
+
+        Crash when reloading a Chromium "platform" app
+        https://bugs.webkit.org/show_bug.cgi?id=93497
+
+        Reviewed by Eric Seidel.
+
+        Test that we don't crash when executing script during the
+        didCreateScriptContext callback.
+
+        * tests/WebFrameTest.cpp:
+        * tests/data/hello_world.html: Added.
+
 2012-08-07  Joshua Bell  <jsbell@chromium.org>
 
         Layout Test storage/indexeddb/intversion-omit-parameter.html is flaky
index c628d00..cd23d62 100644 (file)
@@ -804,6 +804,25 @@ TEST_F(WebFrameTest, GetFullHtmlOfPage)
     EXPECT_TRUE(selectionHtml.isEmpty());
 }
 
+class TestExecuteScriptDuringDidCreateScriptContext : public WebFrameClient {
+public:
+    virtual void didCreateScriptContext(WebFrame* frame, v8::Handle<v8::Context> context, int extensionGroup, int worldId) OVERRIDE
+    {
+        frame->executeScript(WebScriptSource("window.history = 'replaced';"));
+    }
+};
+
+TEST_F(WebFrameTest, ExecuteScriptDuringDidCreateScriptContext)
+{
+    registerMockedHttpURLLoad("hello_world.html");
+
+    TestExecuteScriptDuringDidCreateScriptContext webFrameClient;
+    WebView* webView = FrameTestHelpers::createWebViewAndLoad(m_baseURL + "hello_world.html", true, &webFrameClient);
+
+    webView->mainFrame()->reload();
+    webkit_support::ServeAsynchronousMockedRequests();
+}
+
 class TestDidCreateFrameWebFrameClient : public WebFrameClient {
 public:
     TestDidCreateFrameWebFrameClient() : m_frameCount(0), m_parent(0)
diff --git a/Source/WebKit/chromium/tests/data/hello_world.html b/Source/WebKit/chromium/tests/data/hello_world.html
new file mode 100644 (file)
index 0000000..75ab18b
--- /dev/null
@@ -0,0 +1,3 @@
+<script>
+document.write("Hello, world.");
+</script>