Reviewed by Sam.

        - fixed <rdar://problem/5220706> REGRESSION (TOT): repro crash in -[WebView(WebViewInternal) _addObject:forIdentifier:] [14425]
        http://bugs.webkit.org/show_bug.cgi?id=14425

        * bindings/js/kjs_window.cpp:
        (KJS::createWindow): No longer take an immediate argument - always do immediate loads
        on a newly created Window. Also, do a load of "" to make sure that the right info makes
        it to the app.
        (KJS::showModalDialog): Updated for above.
        (KJS::WindowFunc::callAsFunction): Updated for above.
        * dom/Document.cpp:
        (WebCore::Document::shouldBeAllowedToLoadLocalResources): If our URL is about:blank,
        we're allowed if our opener is (since the opener must have written the contents).
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::changeLocation): Add a variant which takes a KURL, which it
        expects to be pre-completed. This is to avoid completing "" to the opener URL.
        (WebCore::FrameLoader::urlSelected): Allow loading empty URLs.
        * loader/FrameLoader.h:

        Test case is manual only, since it takes particular app behavior to reproduce:

        * manual-tests/new-window-subresource-crash.html: Added.

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

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 5c84f64a693932330bfd9d086ba67617cbc1fa78..04cb785f1f4b76efbf00b40581fa585d81b39f3d 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2007-07-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Sam.
+
+        - fixed <rdar://problem/5220706> REGRESSION (TOT): repro crash in -[WebView(WebViewInternal) _addObject:forIdentifier:] [14425]
+        http://bugs.webkit.org/show_bug.cgi?id=14425
+
+        * bindings/js/kjs_window.cpp:
+        (KJS::createWindow): No longer take an immediate argument - always do immediate loads
+        on a newly created Window. Also, do a load of "" to make sure that the right info makes
+        it to the app.
+        (KJS::showModalDialog): Updated for above.
+        (KJS::WindowFunc::callAsFunction): Updated for above.
+        * dom/Document.cpp:
+        (WebCore::Document::shouldBeAllowedToLoadLocalResources): If our URL is about:blank,
+        we're allowed if our opener is (since the opener must have written the contents).
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::changeLocation): Add a variant which takes a KURL, which it
+        expects to be pre-completed. This is to avoid completing "" to the opener URL.
+        (WebCore::FrameLoader::urlSelected): Allow loading empty URLs.
+        * loader/FrameLoader.h:
+        
+        Test case is manual only, since it takes particular app behavior to reproduce:
+        
+        * manual-tests/new-window-subresource-crash.html: Added.
+
 2007-07-08  Mitz Pettel  <mitz@webkit.org>
 
         Reviewed by Maciej.

diff --git a/WebCore/bindings/js/kjs_window.cpp b/WebCore/bindings/js/kjs_window.cpp
index 0a152fc28424e6339c0e45ca25b8079eb3d7b7e2..cfd9756ced09bbcd5eeaa117c83e05e9dee05659 100644 (file)
--- a/WebCore/bindings/js/kjs_window.cpp
+++ b/WebCore/bindings/js/kjs_window.cpp
@@ -391,7 +391,7 @@ static float floatFeature(const HashMap<String, String> &features, const char *k
 }
 
 static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& url,
-    const String& frameName, const WindowFeatures& windowFeatures, JSValue* dialogArgs, bool immediate)
+    const String& frameName, const WindowFeatures& windowFeatures, JSValue* dialogArgs)
 {
     Frame* activeFrame = Window::retrieveActive(exec)->frame();
     
@@ -420,19 +420,17 @@ static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& ur
     if (dialogArgs)
         newWindow->putDirect("dialogArguments", dialogArgs);
 
-    if (created)
-        if (Document* oldDoc = openerFrame->document()) {
-            newFrame->document()->setDomain(oldDoc->domain(), true);
-            newFrame->document()->setBaseURL(oldDoc->baseURL());
-        }
-        
-    if (!url.isEmpty() && (!url.startsWith("javascript:", false) || newWindow->isSafeScript(exec))) {
-        String completedURL = activeFrame->document()->completeURL(url);
+    if (!url.startsWith("javascript:", false) || newWindow->isSafeScript(exec)) {
+        String completedURL = url.isEmpty() ? url : activeFrame->document()->completeURL(url);
         bool userGesture = static_cast<ScriptInterpreter *>(exec->dynamicInterpreter())->wasRunByUserGesture();
         
-        if (immediate)
-            newFrame->loader()->changeLocation(completedURL, activeFrame->loader()->outgoingReferrer(), false, userGesture);
-        else
+        if (created) {
+            newFrame->loader()->changeLocation(KURL(completedURL.deprecatedString()), activeFrame->loader()->outgoingReferrer(), false, userGesture);
+            if (Document* oldDoc = openerFrame->document()) {
+                newFrame->document()->setDomain(oldDoc->domain(), true);
+                newFrame->document()->setBaseURL(oldDoc->baseURL());
+            }
+        } else if (!url.isEmpty())
             newFrame->loader()->scheduleLocationChange(completedURL, activeFrame->loader()->outgoingReferrer(), false, userGesture);
     }
     
@@ -504,7 +502,7 @@ static JSValue* showModalDialog(ExecState* exec, Window* openerWindow, const Lis
     wargs.locationBarVisible = false;
     wargs.fullscreen = false;
     
-    Frame* dialogFrame = createWindow(exec, openerWindow->frame(), valueToStringWithUndefinedOrNullCheck(exec, args[0]), "", wargs, args[1], true);
+    Frame* dialogFrame = createWindow(exec, openerWindow->frame(), valueToStringWithUndefinedOrNullCheck(exec, args[0]), "", wargs, args[1]);
     if (!dialogFrame)
         return jsUndefined();
 
@@ -1453,7 +1451,7 @@ JSValue *WindowFunc::callAsFunction(ExecState *exec, JSObject *thisObj, const Li
       parseWindowFeatures(features, windowFeatures);
       constrainToVisible(screenRect(page->mainFrame()->view()), windowFeatures);
 
-      frame = createWindow(exec, frame, urlString, frameName, windowFeatures, 0, false);
+      frame = createWindow(exec, frame, urlString, frameName, windowFeatures, 0);
 
       if (!frame)
           return jsUndefined();

diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index d67a1a26e346509bc567c11a0853b03e2e69e6a8..48c85dc6174b37fd7cbf9b88826e44271c13b81a 100644 (file)
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -1550,6 +1550,9 @@ bool Document::shouldBeAllowedToLoadLocalResources() const
     DocumentLoader* documentLoader = frame->loader()->documentLoader();
     if (!documentLoader)
         return false;
+
+    if (m_url == "about:blank" && frame->loader()->opener() && frame->loader()->opener()->document()->isAllowedToLoadLocalResources())
+        return true;
     
     return documentLoader->substituteData().isValid();
 }

diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index dcae6bdada3add4262cb99a3f732252ca0af7c38..24cb09cdad05a6fd8d2128352e3572a20d1b738e 100644 (file)
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -346,8 +346,13 @@ bool FrameLoader::canHandleRequest(const ResourceRequest& request)
 
 void FrameLoader::changeLocation(const String& URL, const String& referrer, bool lockHistory, bool userGesture)
 {
-    if (URL.find("javascript:", 0, false) == 0) {
-        String script = KURL::decode_string(URL.substring(strlen("javascript:")).deprecatedString());
+    changeLocation(completeURL(URL), referrer, lockHistory, userGesture);
+}
+
+void FrameLoader::changeLocation(const KURL& URL, const String& referrer, bool lockHistory, bool userGesture)
+{
+    if (URL.url().find("javascript:", 0, false) == 0) {
+        String script = KURL::decode_string(URL.url().mid(strlen("javascript:")));
         JSValue* result = executeScript(0, script, userGesture);
         String scriptResult;
         if (getString(result, scriptResult)) {
@@ -360,7 +365,7 @@ void FrameLoader::changeLocation(const String& URL, const String& referrer, bool
 
     ResourceRequestCachePolicy policy = (m_cachePolicy == CachePolicyReload) || (m_cachePolicy == CachePolicyRefresh)
         ? ReloadIgnoringCacheData : UseProtocolCachePolicy;
-    ResourceRequest request(completeURL(URL), referrer, policy);
+    ResourceRequest request(URL, referrer, policy);
     
     urlSelected(request, "_self", 0, lockHistory, userGesture);
 }
@@ -377,7 +382,7 @@ void FrameLoader::urlSelected(const ResourceRequest& request, const String& _tar
         return;
     }
 
-    if (!url.isValid())
+    if (!url.isValid() && !url.isEmpty())
         return;
 
     FrameLoadRequest frameRequest(request, target);

diff --git a/WebCore/loader/FrameLoader.h b/WebCore/loader/FrameLoader.h
index f16c87565779e63832721ed3d9f88b23975244b9..eec03fe66e9388185e5c0b95dfec3886f2b439fd 100644 (file)
--- a/WebCore/loader/FrameLoader.h
+++ b/WebCore/loader/FrameLoader.h
@@ -273,6 +273,7 @@ namespace WebCore {
         void setDefersLoading(bool);
 
         void changeLocation(const String& URL, const String& referrer, bool lockHistory = true, bool userGesture = false);
+        void changeLocation(const KURL& URL, const String& referrer, bool lockHistory = true, bool userGesture = false);
         void urlSelected(const ResourceRequest&, const String& target, Event*, bool lockHistory, bool userGesture);
         void urlSelected(const FrameLoadRequest&, Event*, bool userGesture);
       

diff --git a/WebCore/manual-tests/new-window-subresource-crash.html b/WebCore/manual-tests/new-window-subresource-crash.html
new file mode 100644 (file)
index 0000000..b287d90
--- /dev/null
+++ b/WebCore/manual-tests/new-window-subresource-crash.html
@@ -0,0 +1,20 @@
+<body onload="test()">
+<script>
+    function test()
+    {
+        var win = window.open("");
+        var doc = win.document;
+        var text = "<html><body><sc" + "ript src='data:text/javascript,'></scr" + "ipt></body></html>";
+        doc.write(text);
+    }
+</script>
+
+<p>This test verifies that document.writing into a newly-opened empty
+window does not cause crashes or assertion failures, even if it
+triggers subresource loads. If you have popup blocking enabled you can
+click the button below to test. The test only works in Safari, because
+it depends on behavior with resource identifiers, which are provided
+by the app.</p>
+
+<button onclick="test()">Crash</button>
+</body>