[haiku-webkit-commits] r297 - in webkit/trunk/WebKit/haiku: API WebCoreSupport
- From: webkit@xxxxxxxxxxxxxxx
- To: haiku-webkit-commits@xxxxxxxxxxxxx
- Date: Mon, 08 Mar 2010 13:19:03 +0000
Author: stippi
Date: Mon Mar 8 13:19:03 2010
New Revision: 297
URL: http://mmlr.dyndns.org/changeset/297
Log:
Documented a memory leaking problem with the shutdown process of pages. There
is one reference too many on the WebCore::Frame. Responsible for getting rid of
any BWebFrame instances is the FrameLoaderClientHaiku, as in other ports.
Added tracing which shows the problem.
Modified:
webkit/trunk/WebKit/haiku/API/WebFrame.cpp
webkit/trunk/WebKit/haiku/API/WebPage.cpp
webkit/trunk/WebKit/haiku/API/WebPage.h
webkit/trunk/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp
Modified: webkit/trunk/WebKit/haiku/API/WebFrame.cpp
==============================================================================
--- webkit/trunk/WebKit/haiku/API/WebFrame.cpp Sat Mar 6 16:42:03 2010
(r296)
+++ webkit/trunk/WebKit/haiku/API/WebFrame.cpp Mon Mar 8 13:19:03 2010
(r297)
@@ -56,12 +56,19 @@
using namespace WebCore;
+#if TRACE_SHUTDOWN_PROCESS
+#define TRACE_SHUTDOWN(x...) printf(x)
+#else
+#define TRACE_SHUTDOWN(x...) do {} while (false)
+#endif
+
BWebFrame::BWebFrame(BWebPage* webPage, BWebFrame* parentFrame,
WebFramePrivate* data)
: fTextMagnifier(1.0)
, fIsEditable(false)
, fTitle(0)
, fData(data)
{
+TRACE_SHUTDOWN("%p->BWebFrame::BWebFrame()\n", this);
fData->loaderClient = new WebCore::FrameLoaderClientHaiku(webPage,
this);
fData->frame = WebCore::Frame::create(fData->page, fData->ownerElement,
fData->loaderClient);
@@ -70,12 +77,21 @@
if (parentFrame)
parentFrame->Frame()->tree()->appendChild(fData->frame);
+ // TODO: We will be left with an extra reference to the WebCore::Frame,
but I have
+ // not yet fully understood how to get rid of it in the proper way.
+
fData->frame->init();
}
BWebFrame::~BWebFrame()
{
+TRACE_SHUTDOWN("%p->BWebFrame::~BWebFrame()\n", this);
+ // NOTE: This is currently never called, because we hage one
+ // reference too many on the WebCore::Frame. See above.
+ fData->frame->loader()->cancelAndClear();
+
delete fData;
+TRACE_SHUTDOWN("%p->BWebFrame::~BWebFrame() - done\n", this);
}
void BWebFrame::SetListener(const BMessenger& listener)
Modified: webkit/trunk/WebKit/haiku/API/WebPage.cpp
==============================================================================
--- webkit/trunk/WebKit/haiku/API/WebPage.cpp Sat Mar 6 16:42:03 2010
(r296)
+++ webkit/trunk/WebKit/haiku/API/WebPage.cpp Mon Mar 8 13:19:03 2010
(r297)
@@ -80,6 +80,12 @@
#include <wtf/Assertions.h>
#include <wtf/Threading.h>
+#if TRACE_SHUTDOWN_PROCESS
+#define TRACE_SHUTDOWN(x...) printf(x)
+#else
+#define TRACE_SHUTDOWN(x...) do {} while (false)
+#endif
+
/*
The basic idea here is to dispatch all public methods to the BLooper
to which the handler is attached (should be the be_app), such that
@@ -184,13 +190,19 @@
BWebPage::~BWebPage()
{
+TRACE_SHUTDOWN("%p->BWebPage::~BWebPage()\n", this);
// We need to make sure there are no more timers running, since those
// arrive to a different, global handler (the timer handler), and the
// timer functions would then operate on stale pointers.
+ // Calling detachFromParent() on the FrameLoader will recursively detach
+ // all child frames, as well as stop all loaders before doing that.
if (m_mainFrame && m_mainFrame->Frame())
- m_mainFrame->Frame()->loader()->stopAllLoaders();
+ m_mainFrame->Frame()->loader()->detachFromParent();
+
+// NOTE: The m_webFrame member should be deleted by the FrameLoaderClientHaiku,
+// but currently, we have an extra reference to the WebCore::Frame, which needs
+// to be fixed.
delete m_settings;
- delete m_mainFrame;
delete m_page;
// Deleting the BWebView is deferred here to keep it alive in
// case some timers still fired after the view calling Shutdown() but
@@ -198,6 +210,7 @@
// deleted itself before we reach the shutdown message, there would be
// a race condition and chance to operate on a stale BWebView pointer.
delete m_webView;
+TRACE_SHUTDOWN("%p->BWebPage::~BWebPage() - done\n", this);
}
// #pragma mark - public
Modified: webkit/trunk/WebKit/haiku/API/WebPage.h
==============================================================================
--- webkit/trunk/WebKit/haiku/API/WebPage.h Sat Mar 6 16:42:03 2010
(r296)
+++ webkit/trunk/WebKit/haiku/API/WebPage.h Mon Mar 8 13:19:03 2010
(r297)
@@ -65,6 +65,8 @@
B_DOWNLOAD_REMOVED = 'dwnr'
};
+#define TRACE_SHUTDOWN_PROCESS 0
+
typedef enum {
B_WEBKIT_CACHE_MODEL_DOCUMENT_VIEWER = 0,
B_WEBKIT_CACHE_MODEL_WEB_BROWSER
Modified: webkit/trunk/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp
==============================================================================
--- webkit/trunk/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp Sat Mar
6 16:42:03 2010 (r296)
+++ webkit/trunk/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp Mon Mar
8 13:19:03 2010 (r297)
@@ -72,6 +72,11 @@
#include <MimeType.h>
#include <String.h>
+#if TRACE_SHUTDOWN_PROCESS
+#define TRACE_SHUTDOWN(x...) printf(x)
+#else
+#define TRACE_SHUTDOWN(x...) do {} while (false)
+#endif
namespace WebCore {
@@ -81,6 +86,7 @@
, m_messenger()
, m_loadingErrorPage(false)
{
+TRACE_SHUTDOWN("%p->FrameLoaderClientHaiku::FrameLoaderClientHaiku(%p)\n",
this, webPage);
ASSERT(m_webPage);
ASSERT(m_webFrame);
}
@@ -92,9 +98,14 @@
void FrameLoaderClientHaiku::frameLoaderDestroyed()
{
- m_webFrame = 0;
- // deleted by BWebPage
+TRACE_SHUTDOWN("%p->FrameLoaderClientHaiku::frameLoaderDestroyed()\n", this);
+ // No one else feels responsible for the BWebFrame instance that created
us.
+ // Even cleaner would be introducing a BWebFrame method to forward this
+ // notification, so that BWebFrame deletes itself there, and us along,
+ // doesn't really matter though.
+ delete m_webFrame;
delete this;
+TRACE_SHUTDOWN("%p->FrameLoaderClientHaiku::frameLoaderDestroyed() - done\n",
this);
}
bool FrameLoaderClientHaiku::hasWebView() const
@@ -885,8 +896,6 @@
return 0;
}
-// frame->setListener(m_messenger);
-
childFrame->loader()->loadURLIntoChildFrame(url, referrer,
childFrame.get());
// The frame's onload handler may have removed it from the document.
Other related posts:
- » [haiku-webkit-commits] r297 - in webkit/trunk/WebKit/haiku: API WebCoreSupport - webkit
