Hello,

I'm making a change to WebKit that will change the return type of FrameLoaderClient::createFrame() from Frame* to PassRefPtr<Frame>. In my patch I also included the necessary changes to gtk and qt frame loader classes to remain compatible with my change.

In an effort to not break the gtk and qt builds, I would really appreciate it if someone who is equipped to build those could try my patch and tell me if i'm missing any other changes.
Thanks.
-Alice Liu


Index: WebCore/ChangeLog
===================================================================
--- WebCore/ChangeLog   (revision 26175)
+++ WebCore/ChangeLog   (working copy)
@@ -1,3 +1,17 @@
+2007-10-09  Alice Liu  <[EMAIL PROTECTED]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        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-08  Sam Weinig  <[EMAIL PROTECTED]>
 
         Reviewed by Steve Falkenburg.
Index: WebCore/loader/FrameLoader.cpp
===================================================================
--- WebCore/loader/FrameLoader.cpp      (revision 26071)
+++ WebCore/loader/FrameLoader.cpp      (working copy)
@@ -449,7 +449,7 @@ Frame* FrameLoader::loadSubframe(HTMLFra
     }
 
     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(HTMLFra
         frame->loader()->checkCompleted();
     }
 
-    return frame;
+    return frame.get();
 }
 
 void FrameLoader::submitFormAgain()
Index: WebCore/loader/FrameLoaderClient.h
===================================================================
--- WebCore/loader/FrameLoaderClient.h  (revision 26071)
+++ WebCore/loader/FrameLoaderClient.h  (working copy)
@@ -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: WebCore/platform/graphics/svg/SVGImageEmptyClients.h
===================================================================
--- WebCore/platform/graphics/svg/SVGImageEmptyClients.h        (revision 26071)
+++ WebCore/platform/graphics/svg/SVGImageEmptyClients.h        (working copy)
@@ -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: WebKit/ChangeLog
===================================================================
--- WebKit/ChangeLog    (revision 26175)
+++ WebKit/ChangeLog    (working copy)
@@ -1,3 +1,15 @@
+2007-10-09  Alice Liu  <[EMAIL PROTECTED]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        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  <[EMAIL PROTECTED]>
 
         Reviewed by John Sullivan.
Index: WebKit/WebCoreSupport/WebFrameLoaderClient.h
===================================================================
--- WebKit/WebCoreSupport/WebFrameLoaderClient.h        (revision 26071)
+++ WebKit/WebCoreSupport/WebFrameLoaderClient.h        (working copy)
@@ -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: WebKit/WebCoreSupport/WebFrameLoaderClient.mm
===================================================================
--- WebKit/WebCoreSupport/WebFrameLoaderClient.mm       (revision 26071)
+++ WebKit/WebCoreSupport/WebFrameLoaderClient.mm       (working copy)
@@ -1140,7 +1140,7 @@ bool WebFrameLoaderClient::canCachePage(
     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: WebKit/gtk/ChangeLog
===================================================================
--- WebKit/gtk/ChangeLog        (revision 26175)
+++ WebKit/gtk/ChangeLog        (working copy)
@@ -1,3 +1,13 @@
+2007-10-09  Alice Liu  <[EMAIL PROTECTED]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        changes to keep the build from breaking
+
+        * WebCoreSupport/FrameLoaderClientGtk.cpp:
+        (WebKit::FrameLoaderClient::createFrame):
+        * WebCoreSupport/FrameLoaderClientGtk.h:
+
 2007-10-03  Alp Toker  <[EMAIL PROTECTED]>
 
         Reviewed by Adam.
Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
===================================================================
--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp  (revision 26071)
+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp  (working copy)
@@ -176,19 +176,17 @@ Widget* FrameLoaderClient::createPlugin(
     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 = 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);
Index: WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h
===================================================================
--- WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h    (revision 26071)
+++ WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h    (working copy)
@@ -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: WebKit/qt/ChangeLog
===================================================================
--- WebKit/qt/ChangeLog (revision 26175)
+++ WebKit/qt/ChangeLog (working copy)
@@ -1,3 +1,13 @@
+2007-10-09  Alice Liu  <[EMAIL PROTECTED]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        changes to keep the build from breaking
+
+        * WebCoreSupport/FrameLoaderClientQt.cpp:
+        (WebCore::FrameLoaderClientQt::createFrame):
+        * WebCoreSupport/FrameLoaderClientQt.h:
+
 2007-10-09  Lars Knoll  <[EMAIL PROTECTED]>
 
         Reviewed by Simon.
Index: WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp
===================================================================
--- WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp    (revision 26071)
+++ WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp    (working copy)
@@ -829,7 +829,7 @@ bool FrameLoaderClientQt::willUseArchive
     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;
@@ -860,7 +860,7 @@ Frame* FrameLoaderClientQt::createFrame(
     if (!childFrame->tree()->parent())
         return 0;
 
-    return childFrame.get();
+    return childFrame;
 }
 
 ObjectContentType FrameLoaderClientQt::objectContentType(const KURL& url, 
const String& _mimeType)
Index: WebKit/qt/WebCoreSupport/FrameLoaderClientQt.h
===================================================================
--- WebKit/qt/WebCoreSupport/FrameLoaderClientQt.h      (revision 26071)
+++ WebKit/qt/WebCoreSupport/FrameLoaderClientQt.h      (working copy)
@@ -202,7 +202,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: WebKit/win/ChangeLog
===================================================================
--- WebKit/win/ChangeLog        (revision 26175)
+++ WebKit/win/ChangeLog        (working copy)
@@ -1,3 +1,24 @@
+2007-10-09  Alice Liu  <[EMAIL PROTECTED]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        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  <[EMAIL PROTECTED]>
 
         <rdar://problem/5436617>
Index: WebKit/win/WebFrame.cpp
===================================================================
--- WebKit/win/WebFrame.cpp     (revision 26071)
+++ WebKit/win/WebFrame.cpp     (working copy)
@@ -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&
 
     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();
 
@@ -2231,10 +2230,10 @@ void WebFrame::dispatchDidCancelAuthenti
     }
 }
 
-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;
 
Index: WebKit/win/WebFrame.h
===================================================================
--- WebKit/win/WebFrame.h       (revision 26071)
+++ WebKit/win/WebFrame.h       (working copy)
@@ -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);
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to