[webkit-dev] could someone try building with my patch to gtk and qt?

Alice Liu alice.liu at apple.com
Tue Oct 9 15:51:12 PDT 2007


Alice Liu wrote:
> 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
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
updated patch after Darin told me that I'm missing an adoptRef.


-------------- next part --------------
Index: WebCore/ChangeLog
===================================================================
--- WebCore/ChangeLog	(revision 26175)
+++ WebCore/ChangeLog	(working copy)
@@ -1,3 +1,17 @@
+2007-10-09  Alice Liu  <alice.liu at apple.com>
+
+        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  <sam at webkit.org>
 
         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  <alice.liu at apple.com>
+
+        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  <bdakin at apple.com>
 
         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  <alice.liu at apple.com>
+
+        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  <alp at atoker.com>
 
         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(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);
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  <alice.liu at apple.com>
+
+        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  <lars at trolltech.com>
 
         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  <alice.liu at apple.com>
+
+        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  <adachan at apple.com>
 
         <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);


More information about the webkit-dev mailing list