gtk:
authoraliceli1 <aliceli1@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2007 18:31:20 +0000 (18:31 +0000)
committeraliceli1 <aliceli1@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2007 18:31:20 +0000 (18:31 +0000)
        Reviewed by Geoff Garen.

        changes to keep the build from breaking

        * WebCoreSupport/FrameLoaderClientGtk.cpp:
        (WebKit::FrameLoaderClient::createFrame):
        * WebCoreSupport/FrameLoaderClientGtk.h:

qt:

        Reviewed by Geoff Garen.

        changes to keep the build from breaking

        * WebCoreSupport/FrameLoaderClientQt.cpp:
        (WebCore::FrameLoaderClientQt::createFrame):
        * WebCoreSupport/FrameLoaderClientQt.h:

WebCore:

        Reviewed by Geoff Garen.

        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT
        createFrame() now returns a RefPtr instead of a raw Frame pointer.
        Making this change improves the way we handle frames on Windows webkit.

        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::loadSubframe):
        * loader/FrameLoaderClient.h:
        * platform/graphics/svg/SVGImageEmptyClients.h:
        (WebCore::SVGEmptyFrameLoaderClient::createFrame):

WebKit:

        Reviewed by Geoff Garen.

        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT
        createFrame() now returns a RefPtr instead of a raw Frame pointer.
        Making this change improves the way we handle frames on Windows WebKit.

        * WebCoreSupport/WebFrameLoaderClient.h:
        * WebCoreSupport/WebFrameLoaderClient.mm:
        (WebFrameLoaderClient::createFrame):

win:

        Reviewed by Geoff Garen.

        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT

        * WebFrame.cpp:
        (WebFrame::createFrame):
        The crash was caused by the early destruction of the subframe.  To resolve this issue,
        the manual deref of the child frame that occurs in between being appended to the
        frametree and being used in loadURLIntoChild wasn't exactly incorrect, but just needed
        to be moved until after loadURLIntoChild. This hasn't been a problem for other uses of
        child frames because this test case involves removing a child frame immediately after
        loading it, all in an onload handler.  Even better than just moving the deref would be
        to change the signature of createFrame to use a RefPtr<Frame> so that a manual deref isn't
        necessary. This is what was done in this patch.
        * WebFrame.h:
        createFrame() now returns a RefPtr instead of a raw Frame pointer.
        Making this change improves the way we handle frames on Windows WebKit.

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

16 files changed:
WebCore/ChangeLog
WebCore/loader/FrameLoader.cpp
WebCore/loader/FrameLoaderClient.h
WebCore/platform/graphics/svg/SVGImageEmptyClients.h
WebKit/ChangeLog
WebKit/WebCoreSupport/WebFrameLoaderClient.h
WebKit/WebCoreSupport/WebFrameLoaderClient.mm
WebKit/gtk/ChangeLog
WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h
WebKit/qt/ChangeLog
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.h
WebKit/win/ChangeLog
WebKit/win/WebFrame.cpp
WebKit/win/WebFrame.h

index 5abaa7e675a71f054e7257c47666c9240d1bbb1b..7ae7c9dacd9ce7db38435f9c181e4a00e27dac23 100644 (file)
@@ -1,3 +1,17 @@
+2007-10-10  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT
+        createFrame() now returns a RefPtr instead of a raw Frame pointer. 
+        Making this change improves the way we handle frames on Windows webkit. 
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadSubframe):
+        * loader/FrameLoaderClient.h:
+        * platform/graphics/svg/SVGImageEmptyClients.h:
+        (WebCore::SVGEmptyFrameLoaderClient::createFrame):
+
 2007-10-10  Simon Hausmann  <hausmann@kde.org>
 
         Reviewed by Lars.
index 05532a8aa6287a1c632f4ae7007fe87fc47b3b7a..1d063eeb1edbfbbf687383b305808faec60348a4 100644 (file)
@@ -449,7 +449,7 @@ Frame* FrameLoader::loadSubframe(HTMLFrameOwnerElement* ownerElement, const KURL
     }
 
     bool hideReferrer = shouldHideReferrer(url, referrer);
-    Frame* frame = m_client->createFrame(url, name, ownerElement, hideReferrer ? String() : referrer,
+    RefPtr<Frame> frame = m_client->createFrame(url, name, ownerElement, hideReferrer ? String() : referrer,
                                          allowsScrolling, marginWidth, marginHeight);
 
     if (!frame)  {
@@ -475,7 +475,7 @@ Frame* FrameLoader::loadSubframe(HTMLFrameOwnerElement* ownerElement, const KURL
         frame->loader()->checkCompleted();
     }
 
-    return frame;
+    return frame.get();
 }
 
 void FrameLoader::submitFormAgain()
index 4b7c8c3e7fb1dbcc9874f82943f2f3044d6be7a9..f54507b6bea23e62be693c109c48cea85b24c2e1 100644 (file)
@@ -196,7 +196,7 @@ namespace WebCore {
         virtual bool canCachePage() const = 0;
         virtual void download(ResourceHandle*, const ResourceRequest&, const ResourceRequest&, const ResourceResponse&) = 0;
 
-        virtual Frame* createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+        virtual PassRefPtr<Frame> createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                    const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight) = 0;
         virtual Widget* createPlugin(const IntSize&, Element*, const KURL&, const Vector<String>&, const Vector<String>&, const String&, bool loadManually) = 0;
         virtual void redirectDataToPlugin(Widget* pluginWidget) = 0;
index d42f69f79d818f243eb09f1347ccd5aa25adf5d5..fb0cbf226b58d590bad038dc1b63764eef2afb5f 100644 (file)
@@ -262,7 +262,7 @@ public:
     virtual void saveDocumentViewToCachedPage(CachedPage*) { }
     virtual bool canCachePage() const { return false; }
 
-    virtual Frame* createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+    virtual PassRefPtr<Frame> createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight) { return 0; }
     virtual Widget* createPlugin(const IntSize&,Element*, const KURL&, const Vector<String>&, const Vector<String>&, const String&, bool) { return 0; }
     virtual Widget* createJavaAppletWidget(const IntSize&, Element*, const KURL&, const Vector<String>&, const Vector<String>&) { return 0; }
index ccce209035e1b2d3bcc0fb272736b0ffa0305f15..f107ac231650a2b7dec4bb4caa372277c5f2efcb 100644 (file)
@@ -1,3 +1,15 @@
+2007-10-10  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT
+        createFrame() now returns a RefPtr instead of a raw Frame pointer. 
+        Making this change improves the way we handle frames on Windows WebKit. 
+
+        * WebCoreSupport/WebFrameLoaderClient.h:
+        * WebCoreSupport/WebFrameLoaderClient.mm:
+        (WebFrameLoaderClient::createFrame):
+
 2007-10-04  Beth Dakin  <bdakin@apple.com>
 
         Reviewed by John Sullivan.
index c5e4a340ee25460048e0267c7343e13944cee510..a82f2aa9950f43ac8ffdd718209421020cd30988 100644 (file)
@@ -182,7 +182,7 @@ private:
 
     virtual void setTitle(const WebCore::String& title, const WebCore::KURL&);
 
-    virtual WebCore::Frame* createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement*,
+    virtual PassRefPtr<WebCore::Frame> createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement*,
                                         const WebCore::String& referrer, bool allowsScrolling, int marginWidth, int marginHeight);
     virtual WebCore::Widget* createPlugin(const WebCore::IntSize&, WebCore::Element*, const WebCore::KURL&, const Vector<WebCore::String>&,
                                           const Vector<WebCore::String>&, const WebCore::String&, bool);
index ea5958d25a1a1f1008bd76cb2abbcf4dfea5febf..8ba948c3c06fe8183c3c7a15330c793b59042e08 100644 (file)
@@ -1140,7 +1140,7 @@ bool WebFrameLoaderClient::canCachePage() const
     return [[[m_webFrame.get() _dataSource] representation] isKindOfClass:[WebHTMLRepresentation class]];
 }
 
-Frame* WebFrameLoaderClient::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+PassRefPtr<Frame> WebFrameLoaderClient::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                          const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight)
 {
     WebFrameBridge* bridge = m_webFrame->_private->bridge;
index 76a3da8d240d876e7d9db16fbd35f39efd7c6f86..2ffe7466fb8c580b2aecac1611dae3b573db8c2a 100644 (file)
@@ -1,3 +1,13 @@
+2007-10-10  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        changes to keep the build from breaking
+
+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
+        (WebKit::FrameLoaderClient::createFrame):
+        * WebCoreSupport/FrameLoaderClientGtk.h:
+
 2007-10-03  Alp Toker  <alp@atoker.com>
 
         Reviewed by Adam.
index bae005c4ab64a986ed2910d885c885fd8344282e..c930cee0b0841a24f16469faf60fc5800aa69eba 100644 (file)
@@ -176,19 +176,17 @@ Widget* FrameLoaderClient::createPlugin(const IntSize&, Element*, const KURL&, c
     return 0;
 }
 
-Frame* FrameLoaderClient::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+PassRefPtr<Frame> FrameLoaderClient::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                         const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight)
 {
     Frame* coreFrame = core(webFrame());
 
     ASSERT(core(getPageFromFrame(webFrame())) == coreFrame->page());
     WebKitFrame* gtkFrame = WEBKIT_FRAME(webkit_frame_init_with_page(getPageFromFrame(webFrame()), ownerElement));
-    Frame* childFrame = core(gtkFrame);
+    RefPtr<Frame> childFrame(adoptRef(core(gtkFrame)));
 
     coreFrame->tree()->appendChild(childFrame);
 
-    // Frames are created with a refcount of 1. Release this ref, since we've assigned it to d->frame.
-    childFrame->deref();
     childFrame->tree()->setName(name);
     childFrame->init();
     childFrame->loader()->load(url, referrer, FrameLoadTypeRedirectWithLockedHistory, String(), 0, 0);
@@ -210,7 +208,7 @@ Frame* FrameLoaderClient::createFrame(const KURL& url, const String& name, HTMLF
             childFrame->view()->setMarginHeight(marginHeight);
     }
 
-    return childFrame;
+    return childFrame.release();
 }
 
 void FrameLoaderClient::redirectDataToPlugin(Widget* pluginWidget)
index 5f8d23a1e0fd525691ad0287be1834c48a665f69..9a3945dc916a38f899fcdd4c0379c83232057dcb 100644 (file)
@@ -112,7 +112,7 @@ namespace WebKit {
         virtual void postProgressEstimateChangedNotification();
         virtual void postProgressFinishedNotification();
 
-        virtual WebCore::Frame* createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement* ownerElement,
+        virtual PassRefPtr<WebCore::Frame> createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement* ownerElement,
                                    const WebCore::String& referrer, bool allowsScrolling, int marginWidth, int marginHeight);
         virtual WebCore::Widget* createPlugin(const WebCore::IntSize&, WebCore::Element*, const WebCore::KURL&, const WTF::Vector<WebCore::String>&, const WTF::Vector<WebCore::String>&, const WebCore::String&, bool);
         virtual void redirectDataToPlugin(WebCore::Widget* pluginWidget);
index c8e11adad35ab8cea626f49b074f58b7bff89ad3..234219e123f963d02c18571ff98266067a7adce0 100644 (file)
@@ -1,3 +1,13 @@
+2007-10-10  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        changes to keep the build from breaking
+
+        * WebCoreSupport/FrameLoaderClientQt.cpp:
+        (WebCore::FrameLoaderClientQt::createFrame):
+        * WebCoreSupport/FrameLoaderClientQt.h:
+
 2007-10-09  Lars Knoll  <lars@trolltech.com>
 
         Reviewed by Simon.
index d748e7ae2537603a643b3f9aee90badb3c1fae6d..30986f06bbd254da22364d4ae2ee0ab575097ee8 100644 (file)
@@ -821,7 +821,7 @@ bool FrameLoaderClientQt::willUseArchive(WebCore::ResourceLoader*, const WebCore
     return false;
 }
 
-Frame* FrameLoaderClientQt::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+PassRefPtr<Frame> FrameLoaderClientQt::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                         const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight)
 {
     QWebFrameData frameData;
@@ -852,7 +852,7 @@ Frame* FrameLoaderClientQt::createFrame(const KURL& url, const String& name, HTM
     if (!childFrame->tree()->parent())
         return 0;
 
-    return childFrame.get();
+    return childFrame.release();
 }
 
 ObjectContentType FrameLoaderClientQt::objectContentType(const KURL& url, const String& _mimeType)
index 3e9895ed3f8792a84c157f56f566a0a8884038e0..739cddb30d9aa38544eedafe90e389299d441828 100644 (file)
@@ -201,7 +201,7 @@ namespace WebCore {
         virtual void postProgressEstimateChangedNotification();
         virtual void postProgressFinishedNotification();
 
-        virtual Frame* createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+        virtual PassRefPtr<Frame> createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                                    const String& referrer, bool allowsScrolling, int marginWidth, int marginHeight) ;
         virtual Widget* createPlugin(const IntSize&, Element*, const KURL&, const Vector<String>&, const Vector<String>&, const String&, bool);
         virtual void redirectDataToPlugin(Widget* pluginWidget);
index 93e62cd4bbc19678d323f5f0488094b8aa10d63b..d6e9533a4e9225a321720418fecf291b6ad84b0c 100644 (file)
@@ -1,3 +1,24 @@
+2007-10-10  Alice Liu  <alice.liu@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Fixed <rdar://5464402> Crash when running fast/frames/onload-remove-iframe-crash.html in DRT
+
+        * WebFrame.cpp:
+        (WebFrame::createFrame):
+        The crash was caused by the early destruction of the subframe.  To resolve this issue, 
+        the manual deref of the child frame that occurs in between being appended to the 
+        frametree and being used in loadURLIntoChild wasn't exactly incorrect, but just needed 
+        to be moved until after loadURLIntoChild. This hasn't been a problem for other uses of 
+        child frames because this test case involves removing a child frame immediately after 
+        loading it, all in an onload handler.  Even better than just moving the deref would be 
+        to change the signature of createFrame to use a RefPtr<Frame> so that a manual deref isn't 
+        necessary. This is what was done in this patch. 
+        * WebFrame.h:
+        createFrame() now returns a RefPtr instead of a raw Frame pointer. 
+        Making this change improves the way we handle frames on Windows WebKit. 
+
+
 2007-10-05  Ada Chan  <adachan@apple.com>
 
         <rdar://problem/5436617>
index 8ff2178dd0e1caac0fb529251d01b3fe43e0be33..991db4effc048aec108e51812eeb93d82253ca06 100644 (file)
@@ -1263,7 +1263,7 @@ void WebFrame::frameLoaderDestroyed()
     this->Release();
 }
 
-Frame* WebFrame::createFrame(const KURL& URL, const String& name, HTMLFrameOwnerElement* ownerElement, const String& referrer)
+PassRefPtr<Frame> WebFrame::createFrame(const KURL& URL, const String& name, HTMLFrameOwnerElement* ownerElement, const String& referrer)
 {
     Frame* coreFrame = core(this);
     ASSERT(coreFrame);
@@ -1273,11 +1273,10 @@ Frame* WebFrame::createFrame(const KURL& URL, const String& name, HTMLFrameOwner
 
     webFrame->initWithWebFrameView(0, d->webView, coreFrame->page(), ownerElement);
 
-    Frame* childFrame = core(webFrame.get());
+    RefPtr<Frame> childFrame(adoptRef(core(webFrame.get()))); // We have to adopt, because Frames start out with a refcount of 1.
     ASSERT(childFrame);
 
     coreFrame->tree()->appendChild(childFrame);
-    childFrame->deref(); // Frames are created with a refcount of 1. Release this ref, since we've assigned it to d->frame.
     childFrame->tree()->setName(name);
     childFrame->init();
 
@@ -1287,7 +1286,7 @@ Frame* WebFrame::createFrame(const KURL& URL, const String& name, HTMLFrameOwner
     if (!childFrame->tree()->parent())
         return 0;
 
-    return childFrame;
+    return childFrame.release();
 }
 
 void WebFrame::loadURLIntoChild(const KURL& originalURL, const String& referrer, WebFrame* childFrame)
@@ -2231,10 +2230,10 @@ void WebFrame::dispatchDidCancelAuthenticationChallenge(DocumentLoader* loader,
     }
 }
 
-Frame* WebFrame::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
+PassRefPtr<Frame> WebFrame::createFrame(const KURL& url, const String& name, HTMLFrameOwnerElement* ownerElement,
                             const String& referrer, bool /*allowsScrolling*/, int /*marginWidth*/, int /*marginHeight*/)
 {
-    Frame* result = createFrame(url, name, ownerElement, referrer);
+    RefPtr<Frame> result = createFrame(url, name, ownerElement, referrer);
     if (!result)
         return 0;
 
@@ -2251,7 +2250,7 @@ Frame* WebFrame::createFrame(const KURL& url, const String& name, HTMLFrameOwner
             result->view()->setMarginHeight(marginHeight);
     }
 
-    return result;
+    return result.release();
 }
 
 Widget* WebFrame::createPlugin(const IntSize& pluginSize, Element* element, const KURL& url, const Vector<String>& paramNames, const Vector<String>& paramValues, const String& mimeType, bool loadManually)
index 2d5bfbfaa64417735590fc9e6946737fae0422aa..64b3436a9fd2ea5f146f1d111010a15b2e43f947 100644 (file)
@@ -207,7 +207,7 @@ public:
     virtual void ref();
     virtual void deref();
 
-    virtual WebCore::Frame* createFrame(const WebCore::KURL&, const WebCore::String& name, WebCore::HTMLFrameOwnerElement*, const WebCore::String& referrer);
+    virtual PassRefPtr<WebCore::Frame> createFrame(const WebCore::KURL&, const WebCore::String& name, WebCore::HTMLFrameOwnerElement*, const WebCore::String& referrer);
     virtual void openURL(const WebCore::String& URL, const WebCore::Event* triggeringEvent, bool newWindow, bool lockHistory);
     virtual void windowScriptObjectAvailable(JSContextRef context, JSObjectRef windowObject);
     
@@ -309,7 +309,7 @@ public:
     virtual void postProgressEstimateChangedNotification();
     virtual void postProgressFinishedNotification();
 
-    virtual WebCore::Frame* createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement* ownerElement,
+    virtual PassRefPtr<WebCore::Frame> createFrame(const WebCore::KURL& url, const WebCore::String& name, WebCore::HTMLFrameOwnerElement* ownerElement,
                                const WebCore::String& referrer, bool allowsScrolling, int marginWidth, int marginHeight);
     virtual WebCore::Widget* createPlugin(const WebCore::IntSize&, WebCore::Element*, const WebCore::KURL&, const Vector<WebCore::String>&, const Vector<WebCore::String>&, const WebCore::String&, bool loadManually);
     virtual void redirectDataToPlugin(WebCore::Widget* pluginWidget);