WebCore:

        Reviewed by Sam Weinig and Anders Carlsson.

        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
        <rdar://problem/5657355>

        This patch makes two changes:

        1) Java calls FrameLoader::load in a slightly different way than
           JavaScript, which previously let a malicious web site bypass the
           shouldAllowNavigation check.  This patch adds that check to that
           code path.

        2) FrameLoader now wraps calls to m_frame->tree()->find(name) with
           findFrameForNavigation, which calls shouldAllowNavigation.  This
           treats disallowed frame navigations as if the named frame did not
           exist, resulting in a popup window when appropriate.

        Tests: http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
               http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html

        * WebCore.base.exp:
        * bindings/js/kjs_window.cpp:
        (KJS::WindowProtoFuncOpen::callAsFunction):
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::createWindow):
        (WebCore::FrameLoader::load):
        (WebCore::FrameLoader::post):
        (WebCore::FrameLoader::findFrameForNavigation):
        * loader/FrameLoader.h:

WebKit/mac:

        Reviewed by Anders Carlsson.

        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
        <rdar://problem/5657355>

        * Plugins/WebBaseNetscapePluginView.mm:
        (-[WebBaseNetscapePluginView loadPluginRequest:]): call findFrameForNavigation
        to ensure the shouldAllowNavigation check is made.

WebKitTools:

        Reviewed by Anders Carlsson.

        Make DRT track open windows instead of allocated windows so that
        we can avoid ASSERTION due to late deallocs out of our control.

        * DumpRenderTree/mac/DumpRenderTree.mm:
        (dumpBackForwardListForAllWindows):
        (runTest):
        * DumpRenderTree/mac/DumpRenderTreeMac.h:
        * DumpRenderTree/mac/DumpRenderTreeWindow.h:
        * DumpRenderTree/mac/DumpRenderTreeWindow.mm:
        (+[DumpRenderTreeWindow openWindows]):
        (-[DumpRenderTreeWindow initWithContentRect:styleMask:backing:defer:]):
        (-[DumpRenderTreeWindow close]):
        * DumpRenderTree/mac/LayoutTestControllerMac.mm:
        (LayoutTestController::windowCount):

LayoutTests:

        Reviewed by Anders Carlsson.

        Tests for http://bugs.webkit.org/show_bug.cgi?id=16522
        <rdar://problem/5657355>

        * http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html: Added.
        * http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html: Added.
        * http/tests/security/frameNavigation/resources/navigation-happened.html: Added.
        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt: Added.
        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html: Added.
        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt: Added.
        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html: Added.
        * platform/win/Skipped:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8513c30..0eb3ae1 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2008-01-10  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Anders Carlsson.
+
+        Tests for http://bugs.webkit.org/show_bug.cgi?id=16522
+        <rdar://problem/5657355>
+
+        * http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html: Added.
+        * http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html: Added.
+        * http/tests/security/frameNavigation/resources/navigation-happened.html: Added.
+        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt: Added.
+        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html: Added.
+        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt: Added.
+        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html: Added.
+        * platform/win/Skipped:
+
 2008-01-10  Adam Roben  <aroben@apple.com>
 
         * platform/win/Skipped: Removed a test that was fixed in r26826.

diff --git a/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html b/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html
new file mode 100644 (file)
index 0000000..a4b95f5
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html
@@ -0,0 +1,59 @@
+<html>
+<head>
+    <script src="../../resources/cross-frame-access.js"></script>
+    <script>
+        window.onload = function()
+        {
+            document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+            if (window.layoutTestController) {
+                setTimeout(pollForTest, 1);
+            } else {
+                log("Click the link to run the test.");
+            }
+        }
+
+        pollForTest = function()
+        {
+            if (!layoutTestController.globalFlag) {
+                setTimeout(pollForTest, 1);
+                return;
+            }
+            runTest();
+        }
+
+        runTest = function()
+        {
+            if (!window.layoutTestController)
+                return;
+
+            var event = document.createEvent('MouseEvent');
+            event.initEvent( 'click', true, true );
+            document.getElementById('anchorLink').dispatchEvent(event);
+
+            start = new Date();
+            myInterval = setInterval(checkIfDone, 1);
+        }
+
+        checkIfDone = function()
+        {
+            var numOpenWindows = layoutTestController.windowCount();
+            var now = new Date();
+            if (numOpenWindows == 2) {
+                log("Test PASSED");
+                clearInterval(myInterval);
+                layoutTestController.notifyDone();            
+            } else if (now - start > 10000) {
+                log('TEST FAILED: Window count ' + numOpenWindows);
+                clearInterval(myInterval);
+                layoutTestController.notifyDone();
+            }
+        }
+    </script>
+</head>
+<body>
+    <h3>Frame-with-link-to-navigate</h3>
+    <h4>DOMAIN</h4>
+    <pre id='console'></pre>
+    <a id="anchorLink" href="navigation-happened.html" target="toNavigate">Click me</a>
+</body>
+</html>

diff --git a/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html b/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html
new file mode 100644 (file)
index 0000000..bf3d752
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html
@@ -0,0 +1,57 @@
+<html>
+<head>
+    <script src="../../resources/cross-frame-access.js"></script>
+    <script>
+        window.onload = function()
+        {
+            document.getElementsByTagName('h4')[0].innerHTML = document.domain;
+            if (window.layoutTestController) {
+                setTimeout(pollForTest, 1);
+            } else {
+                log("Click the link to run the test.");
+            }
+        }
+
+        pollForTest = function()
+        {
+            if (!layoutTestController.globalFlag) {
+                setTimeout(pollForTest, 1);
+                return;
+            }
+            runTest();
+        }
+
+        runTest = function()
+        {
+            if (!window.layoutTestController)
+                return;
+
+            plg.getURL("navigation-happened.html", "toNavigate");
+
+            start = new Date();
+            myInterval = setInterval(checkIfDone, 500);
+        }
+
+        checkIfDone = function()
+        {
+            var numOpenWindows = layoutTestController.windowCount();
+            var now = new Date();
+            if (numOpenWindows == 2) {
+                log("Test PASSED");
+                clearInterval(myInterval);
+                layoutTestController.notifyDone();            
+            } else if (now - start > 10000) {
+                log('TEST FAILED: Window count ' + numOpenWindows);
+                clearInterval(myInterval);
+                layoutTestController.notifyDone();
+            }
+        }
+    </script>
+</head>
+<body>
+    <embed name="plg" type="application/x-webkit-test-netscape"></embed>
+    <h3>Frame-with-plugin-to-navigate</h3>
+    <h4>DOMAIN</h4>
+    <pre id='console'></pre>
+</body>
+</html>

diff --git a/LayoutTests/http/tests/security/frameNavigation/resources/navigation-happened.html b/LayoutTests/http/tests/security/frameNavigation/resources/navigation-happened.html
new file mode 100644 (file)
index 0000000..0ad953b
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/resources/navigation-happened.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+    Navigated page
+</body>
+</html>

diff --git a/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt
new file mode 100644 (file)
index 0000000..81d5b7f
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt
@@ -0,0 +1,19 @@
+CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to initiate a navigation change for frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe.html from frame with URL http://localhost:8000/security/frameNavigation/resources/frame-with-plugin-to-navigate.html.
+
+ 
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+
+Frame-with-plugin-to-navigate
+
+localhost
+
+Test PASSED
+
+
+--------
+Frame: 'toNavigate'
+--------
+Inner iframe.

diff --git a/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
new file mode 100644 (file)
index 0000000..4413ac7
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
@@ -0,0 +1,22 @@
+<html>
+<head>
+    <script src="../resources/cross-frame-access.js"></script>
+    <script>
+        window.onload = function()
+        {
+            if (window.layoutTestController) {
+                layoutTestController.dumpAsText();
+                layoutTestController.dumpChildFramesAsText();
+                layoutTestController.waitUntilDone();
+                layoutTestController.setCanOpenWindows();
+                layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+            }
+        }
+    </script>
+</head>
+<body>
+<pre id='console'></pre>
+<iframe src="http://localhost:8000/security/frameNavigation/resources/frame-with-plugin-to-navigate.html"></iframe>
+<iframe name="toNavigate" src="http://127.0.0.1:8000/security/resources/cross-frame-iframe.html"></iframe>
+</body>
+</html>

diff --git a/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt
new file mode 100644 (file)
index 0000000..9ceb021
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt
@@ -0,0 +1,20 @@
+CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to initiate a navigation change for frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe.html from frame with URL http://localhost:8000/security/frameNavigation/resources/frame-with-link-to-navigate.html.
+
+CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to initiate a navigation change for frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe.html from frame with URL http://localhost:8000/security/frameNavigation/resources/frame-with-link-to-navigate.html.
+
+ 
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+Frame-with-link-to-navigate
+
+localhost
+
+Test PASSED
+Click me
+
+--------
+Frame: 'toNavigate'
+--------
+Inner iframe.

diff --git a/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
new file mode 100644 (file)
index 0000000..5005ecd
--- /dev/null
+++ b/LayoutTests/http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
@@ -0,0 +1,22 @@
+<html>
+<head>
+    <script src="../resources/cross-frame-access.js"></script>
+    <script>
+        window.onload = function()
+        {
+            if (window.layoutTestController) {
+                layoutTestController.dumpAsText();
+                layoutTestController.dumpChildFramesAsText();
+                layoutTestController.waitUntilDone();
+                layoutTestController.setCanOpenWindows();
+                layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+            }
+        }
+    </script>
+</head>
+<body>
+<pre id='console'></pre>
+<iframe src="http://localhost:8000/security/frameNavigation/resources/frame-with-link-to-navigate.html"></iframe>
+<iframe name="toNavigate" src="http://127.0.0.1:8000/security/resources/cross-frame-iframe.html"></iframe>
+</body>
+</html>

diff --git a/LayoutTests/platform/win/Skipped b/LayoutTests/platform/win/Skipped
index 07f0893..324e971 100644 (file)
--- a/LayoutTests/platform/win/Skipped
+++ b/LayoutTests/platform/win/Skipped
@@ -202,6 +202,8 @@ http/tests/security/javascriptURL/xss-DENIED-to-javascript-url-in-foreign-domain
 http/tests/security/aboutBlank/xss-DENIED-set-opener.html
 http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html
 http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
+http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
+http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
 
 # DRT is not fully implemented in boomer <rdar://problem/5128261>
 fast/dom/Window/setting-properties-on-closed-window.html

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 8629586..d0e7a99 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2008-01-10  Adam Barth  <hk9565@gmail.com>
+
+        Reviewed by Sam Weinig and Anders Carlsson.
+
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
+        <rdar://problem/5657355>
+
+        This patch makes two changes:
+
+        1) Java calls FrameLoader::load in a slightly different way than
+           JavaScript, which previously let a malicious web site bypass the
+           shouldAllowNavigation check.  This patch adds that check to that
+           code path.
+
+        2) FrameLoader now wraps calls to m_frame->tree()->find(name) with
+           findFrameForNavigation, which calls shouldAllowNavigation.  This
+           treats disallowed frame navigations as if the named frame did not
+           exist, resulting in a popup window when appropriate.
+
+        Tests: http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
+               http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
+
+        * WebCore.base.exp:
+        * bindings/js/kjs_window.cpp:
+        (KJS::WindowProtoFuncOpen::callAsFunction):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::createWindow):
+        (WebCore::FrameLoader::load):
+        (WebCore::FrameLoader::post):
+        (WebCore::FrameLoader::findFrameForNavigation):
+        * loader/FrameLoader.h:
+
 2008-01-10  John Sullivan  <sullivan@apple.com>
 
         Written by Hyatt, reviewed by me

diff --git a/WebCore/WebCore.base.exp b/WebCore/WebCore.base.exp
index d40c0db..f2ae5ac 100644 (file)
--- a/WebCore/WebCore.base.exp
+++ b/WebCore/WebCore.base.exp
@@ -1,4 +1,3 @@
-
 .objc_class_name_DOMAbstractView
 .objc_class_name_DOMAttr
 .objc_class_name_DOMCDATASection
@@ -152,6 +151,7 @@ __ZN7WebCore11FrameLoader18currentHistoryItemEv
 __ZN7WebCore11FrameLoader18shouldHideReferrerERKNS_4KURLERKNS_6StringE
 __ZN7WebCore11FrameLoader20continueLoadWithDataEPNS_12SharedBufferERKNS_6StringES5_RKNS_4KURLE
 __ZN7WebCore11FrameLoader21setCurrentHistoryItemEN3WTF10PassRefPtrINS_11HistoryItemEEE
+__ZN7WebCore11FrameLoader22findFrameForNavigationERKNS_12AtomicStringE
 __ZN7WebCore11FrameLoader22setPreviousHistoryItemEN3WTF10PassRefPtrINS_11HistoryItemEEE
 __ZN7WebCore11FrameLoader23reloadAllowingStaleDataERKNS_6StringE
 __ZN7WebCore11FrameLoader23timeOfLastCompletedLoadEv
@@ -732,6 +732,14 @@ _wkDrawBezeledTextArea
 _wkDrawBezeledTextFieldCell
 _wkDrawCapsLockIndicator
 _wkDrawFocusRing
+_wkDrawMediaFullscreenButton
+_wkDrawMediaMuteButton
+_wkDrawMediaPauseButton
+_wkDrawMediaPlayButton
+_wkDrawMediaSeekBackButton
+_wkDrawMediaSeekForwardButton
+_wkDrawMediaSliderThumb
+_wkDrawMediaUnMuteButton
 _wkDrawTextFieldCellFocusRing
 _wkFontSmoothingModeIsLCD
 _wkGetATSStyleGroup
@@ -746,14 +754,6 @@ _wkGetGlyphVectorNumGlyphs
 _wkGetGlyphVectorRecordSize
 _wkGetMIMETypeForExtension
 _wkGetMediaControlBackgroundImageData
-_wkDrawMediaFullscreenButton
-_wkDrawMediaMuteButton
-_wkDrawMediaPauseButton
-_wkDrawMediaPlayButton
-_wkDrawMediaSeekBackButton
-_wkDrawMediaSeekForwardButton
-_wkDrawMediaSliderThumb
-_wkDrawMediaUnMuteButton
 _wkGetNSFontATSUFontId
 _wkGetNSURLResponseCalculatedExpiration
 _wkGetNSURLResponseLastModifiedDate

diff --git a/WebCore/bindings/js/kjs_window.cpp b/WebCore/bindings/js/kjs_window.cpp
index 93bb5cb..3918a22 100644 (file)
--- a/WebCore/bindings/js/kjs_window.cpp
+++ b/WebCore/bindings/js/kjs_window.cpp
@@ -329,6 +329,18 @@ static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& ur
         request.setHTTPReferrer(activeFrame->loader()->outgoingReferrer());
     FrameLoadRequest frameRequest(request, frameName);
 
+    FrameLoader* loader;
+    if (activeFrame)
+        // We need to use the active frame's loader to let FrameLoader know
+        // which principal is requesting the navigation.  Unfortunately, there
+        // might not be an activeFrame, in which case we resort to using the
+        // opener's loader.
+        //
+        // See http://bugs.webkit.org/show_bug.cgi?id=16522
+        loader = activeFrame->loader();
+    else
+        loader = openerFrame->loader();
+
     // FIXME: It's much better for client API if a new window starts with a URL, here where we
     // know what URL we are going to open. Unfortunately, this code passes the empty string
     // for the URL, but there's a reason for that. Before loading we have to set up the opener,
@@ -337,7 +349,7 @@ static Frame* createWindow(ExecState* exec, Frame* openerFrame, const String& ur
     // We'd have to resolve all those issues to pass the URL instead of "".
 
     bool created;
-    Frame* newFrame = openerFrame->loader()->createWindow(frameRequest, windowFeatures, created);
+    Frame* newFrame = loader->createWindow(frameRequest, windowFeatures, created);
     if (!newFrame)
         return 0;
 
@@ -1063,11 +1075,12 @@ JSValue* WindowProtoFuncOpen::callAsFunction(ExecState* exec, JSObject* thisObj,
     } else if (frameName == "_parent") {
         if (Frame* parent = frame->tree()->parent())
             frame = parent;
-        if (!activeFrame->loader()->shouldAllowNavigation(frame))
-            return jsUndefined();
         topOrParent = true;
     }
     if (topOrParent) {
+        if (!activeFrame->loader()->shouldAllowNavigation(frame))
+            return jsUndefined();
+
         String completedURL;
         if (!urlString.isEmpty())
             completedURL = activeFrame->document()->completeURL(urlString);

diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp
index 2d2d1ee..6696d3b 100644 (file)
--- a/WebCore/loader/FrameLoader.cpp
+++ b/WebCore/loader/FrameLoader.cpp
@@ -309,9 +309,7 @@ Frame* FrameLoader::createWindow(const FrameLoadRequest& request, const WindowFe
     ASSERT(!features.dialog || request.frameName().isEmpty());
 
     if (!request.frameName().isEmpty() && request.frameName() != "_blank")
-        if (Frame* frame = m_frame->tree()->find(request.frameName())) {
-            if (!shouldAllowNavigation(frame))
-                return 0;
+        if (Frame* frame = findFrameForNavigation(request.frameName())) {
             if (!request.resourceRequest().url().isEmpty())
                 frame->loader()->load(request, false, true, 0, 0, HashMap<String, String>());
             if (Page* page = frame->page())
@@ -1948,9 +1946,7 @@ void FrameLoader::load(const FrameLoadRequest& request, bool lockHistory, bool u
     if (shouldHideReferrer(url, referrer))
         referrer = String();
     
-    Frame* targetFrame = m_frame->tree()->find(request.frameName());
-    if (!shouldAllowNavigation(targetFrame))
-        return;
+    Frame* targetFrame = findFrameForNavigation(request.frameName());
         
     if (request.resourceRequest().httpMethod() != "POST") {
         FrameLoadType loadType;
@@ -1993,7 +1989,7 @@ void FrameLoader::load(const KURL& newURL, const String& referrer, FrameLoadType
     NavigationAction action(newURL, newLoadType, isFormSubmission, event);
 
     if (!frameName.isEmpty()) {
-        if (Frame* targetFrame = m_frame->tree()->find(frameName))
+        if (Frame* targetFrame = findFrameForNavigation(frameName))
             targetFrame->loader()->load(newURL, referrer, newLoadType, String(), event, formState);
         else
             checkNewWindowPolicy(action, request, formState, frameName);
@@ -2065,7 +2061,7 @@ void FrameLoader::load(const ResourceRequest& request, const String& frameName)
         return;
     }
 
-    Frame* frame = m_frame->tree()->find(frameName);
+    Frame* frame = findFrameForNavigation(frameName);
     if (frame) {
         frame->loader()->load(request);
         return;
@@ -3244,7 +3240,7 @@ void FrameLoader::post(const KURL& url, const String& referrer, const String& fr
         formState = FormState::create(form, formValues, m_frame);
 
     if (!frameName.isEmpty()) {
-        if (Frame* targetFrame = m_frame->tree()->find(frameName))
+        if (Frame* targetFrame = findFrameForNavigation(frameName))
             targetFrame->loader()->load(request, action, FrameLoadTypeStandard, formState.release());
         else
             checkNewWindowPolicy(action, request, formState.release(), frameName);
@@ -3871,6 +3867,14 @@ PassRefPtr<HistoryItem> FrameLoader::createHistoryItemTree(Frame* targetFrame, b
     return bfItem;
 }
 
+Frame* FrameLoader::findFrameForNavigation(const AtomicString& name)
+{
+    Frame* frame = m_frame->tree()->find(name);
+    if (shouldAllowNavigation(frame))
+        return frame;  
+    return 0;
+}
+
 void FrameLoader::saveScrollPositionAndViewStateToItem(HistoryItem* item)
 {
     if (!item || !m_frame->view())

diff --git a/WebCore/loader/FrameLoader.h b/WebCore/loader/FrameLoader.h
index d59413f..b01054b 100644 (file)
--- a/WebCore/loader/FrameLoader.h
+++ b/WebCore/loader/FrameLoader.h
@@ -435,6 +435,7 @@ namespace WebCore {
         void iconLoadDecisionAvailable();
 
         bool shouldAllowNavigation(Frame* targetFrame) const;
+        Frame* findFrameForNavigation(const AtomicString& name);
 
     private:
         PassRefPtr<HistoryItem> createHistoryItem(bool useOriginal);

diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
index efaa200..1df06ad 100644 (file)
--- a/WebKit/mac/ChangeLog
+++ b/WebKit/mac/ChangeLog
@@ -1,3 +1,14 @@
+2008-01-10  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Anders Carlsson.
+
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
+        <rdar://problem/5657355>
+
+        * Plugins/WebBaseNetscapePluginView.mm:
+        (-[WebBaseNetscapePluginView loadPluginRequest:]): call findFrameForNavigation
+        to ensure the shouldAllowNavigation check is made.
+
 2008-01-07  Nikolas Zimmermann  <zimmermann@kde.org>
 
         Reviewed by Mark.

diff --git a/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm b/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm
index b84a649..00a7b7d 100644 (file)
--- a/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm
+++ b/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm
@@ -2130,8 +2130,7 @@ static OSStatus TSMEventHandler(EventHandlerCallRef inHandlerRef, EventRef inEve
     if (frameName) {
         // FIXME - need to get rid of this window creation which
         // bypasses normal targeted link handling
-        frame = [[self webFrame] findFrameNamed:frameName];
-    
+        frame = kit([[self webFrame] _frameLoader]->findFrameForNavigation(frameName));
         if (frame == nil) {
             WebView *currentWebView = [self webView];
             NSDictionary *features = [[NSDictionary alloc] init];

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 1b22e3e..f188265 100644 (file)
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,22 @@
+2008-01-10  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Anders Carlsson.
+
+        Make DRT track open windows instead of allocated windows so that
+        we can avoid ASSERTION due to late deallocs out of our control.
+
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (dumpBackForwardListForAllWindows):
+        (runTest):
+        * DumpRenderTree/mac/DumpRenderTreeMac.h:
+        * DumpRenderTree/mac/DumpRenderTreeWindow.h:
+        * DumpRenderTree/mac/DumpRenderTreeWindow.mm:
+        (+[DumpRenderTreeWindow openWindows]):
+        (-[DumpRenderTreeWindow initWithContentRect:styleMask:backing:defer:]):
+        (-[DumpRenderTreeWindow close]):
+        * DumpRenderTree/mac/LayoutTestControllerMac.mm:
+        (LayoutTestController::windowCount):
+
 2008-01-10  Ada Chan  <adachan@apple.com>
 
         Meta key is not the same as Alt key on windows.

diff --git a/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm b/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
index 55a6bcb..1af8efe 100644 (file)
--- a/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
+++ b/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
@@ -706,10 +706,10 @@ static const char *methodNameStringForFailedTest()
 
 static void dumpBackForwardListForAllWindows()
 {
-    CFArrayRef allWindows = (CFArrayRef)[DumpRenderTreeWindow allWindows];
-    unsigned count = CFArrayGetCount(allWindows);
+    CFArrayRef openWindows = (CFArrayRef)[DumpRenderTreeWindow openWindows];
+    unsigned count = CFArrayGetCount(openWindows);
     for (unsigned i = 0; i < count; i++) {
-        NSWindow *window = (NSWindow *)CFArrayGetValueAtIndex(allWindows, i);
+        NSWindow *window = (NSWindow *)CFArrayGetValueAtIndex(openWindows, i);
         WebView *webView = [[[window contentView] subviews] objectAtIndex:0];
         dumpBackForwardListForWebView(webView);
     }
@@ -866,8 +866,8 @@ static void runTest(const char *pathOrURL)
     WorkQueue::shared()->clear();
 
     if (layoutTestController->closeRemainingWindowsWhenComplete()) {
-        NSArray* array = [DumpRenderTreeWindow allWindows];
-        
+        NSArray* array = [DumpRenderTreeWindow openWindows];
+
         unsigned count = [array count];
         for (unsigned i = 0; i < count; i++) {
             NSWindow *window = [array objectAtIndex:i];
@@ -888,9 +888,9 @@ static void runTest(const char *pathOrURL)
     
     [pool release];
 
-    // We should only have our main window left when we're done
-    ASSERT(CFArrayGetCount(allWindowsRef) == 1);
-    ASSERT(CFArrayGetValueAtIndex(allWindowsRef, 0) == [[mainFrame webView] window]);
+    // We should only have our main window left open when we're done
+    ASSERT(CFArrayGetCount(openWindowsRef) == 1);
+    ASSERT(CFArrayGetValueAtIndex(openWindowsRef, 0) == [[mainFrame webView] window]);
 
     delete layoutTestController;
     layoutTestController = 0;

diff --git a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeMac.h b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeMac.h
index fcfe14d..eb77461 100644 (file)
--- a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeMac.h
+++ b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeMac.h
@@ -38,7 +38,7 @@
 @class WebFrame;
 @class WebView;
 
-extern CFMutableArrayRef allWindowsRef;
+extern CFMutableArrayRef openWindowsRef;
 extern CFMutableSetRef disallowedURLs;
 extern WebFrame* mainFrame;
 extern WebFrame* topLoadingFrame;

diff --git a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.h b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.h
index 33c72d6..3ac3223 100644 (file)
--- a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.h
+++ b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.h
@@ -32,5 +32,5 @@
 
 @interface DumpRenderTreeWindow : NSWindow
 // I'm not sure why we can't just use [NSApp windows]
-+ (NSArray *)allWindows;
++ (NSArray *)openWindows;
 @end

diff --git a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.mm b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.mm
index 1ea7fcb..9e5e104 100644 (file)
--- a/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.mm
+++ b/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.mm
@@ -35,7 +35,7 @@
 // FIXME: This file is ObjC++ only because of this include. :(
 #import "LayoutTestController.h"
 
-CFMutableArrayRef allWindowsRef = 0;
+CFMutableArrayRef openWindowsRef = 0;
 
 static CFArrayCallBacks NonRetainingArrayCallbacks = {
     0,
@@ -47,29 +47,28 @@ static CFArrayCallBacks NonRetainingArrayCallbacks = {
 
 @implementation DumpRenderTreeWindow
 
-+ (NSArray *)allWindows
++ (NSArray *)openWindows
 {
-    return [[(NSArray *)allWindowsRef copy] autorelease];
+    return [[(NSArray *)openWindowsRef copy] autorelease];
 }
 
 - (id)initWithContentRect:(NSRect)contentRect styleMask:(unsigned int)styleMask backing:(NSBackingStoreType)bufferingType defer:(BOOL)deferCreation
 {
-    if (!allWindowsRef)
-        allWindowsRef = CFArrayCreateMutable(NULL, 0, &NonRetainingArrayCallbacks);
+    if (!openWindowsRef)
+        openWindowsRef = CFArrayCreateMutable(NULL, 0, &NonRetainingArrayCallbacks);
 
-    CFArrayAppendValue(allWindowsRef, self);
+    CFArrayAppendValue(openWindowsRef, self);
             
     return [super initWithContentRect:contentRect styleMask:styleMask backing:bufferingType defer:deferCreation];
 }
 
-- (void)dealloc
+- (void)close
 {
-    CFRange arrayRange = CFRangeMake(0, CFArrayGetCount(allWindowsRef));
-    CFIndex i = CFArrayGetFirstIndexOfValue(allWindowsRef, arrayRange, self);
+    CFRange arrayRange = CFRangeMake(0, CFArrayGetCount(openWindowsRef));
+    CFIndex i = CFArrayGetFirstIndexOfValue(openWindowsRef, arrayRange, self);
     assert(i != -1);
-
-    CFArrayRemoveValueAtIndex(allWindowsRef, i);
-    [super dealloc];
+    CFArrayRemoveValueAtIndex(openWindowsRef, i);
+    [super close];
 }
 
 - (BOOL)isKeyWindow

diff --git a/WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm b/WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm
index 6c1efc1..70bd4bc 100644 (file)
--- a/WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm
+++ b/WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm
@@ -240,7 +240,7 @@ void LayoutTestController::setWaitToDump(bool waitUntilDone)
 
 int LayoutTestController::windowCount()
 {
-    return CFArrayGetCount(allWindowsRef);
+    return CFArrayGetCount(openWindowsRef);
 }
 
 void LayoutTestController::execCommand(JSStringRef name, JSStringRef value)