- fix <rdar://problem/
5310848> WebDataSource lifetime problem -- may be cause of the leaks seen on the buildbot
* WebView/WebDataSource.mm:
(-[WebDataSourcePrivate dealloc]): Added a call to the new detachDataSource function.
(-[WebDataSourcePrivate finalize]): Ditto.
* WebView/WebDocumentLoaderMac.h: Added detachDataSource function to be used when the
WebDataSource is deallocated. Added retain/releaseDataSource helper functions to be
used to retain and release the data source object. Replaced the m_hasEverBeenDetached
boolean with a more primitive and hence easier to understand m_isDataSourceRetained boolean.
* WebView/WebDocumentLoaderMac.mm:
(WebDocumentLoaderMac::WebDocumentLoaderMac): Initialize m_isDataSourceRetained to false.
(WebDocumentLoaderMac::setDataSource): Call retainDataSource instead of calling HardRetain
on the dataSource parameter. Also updated a comment.
(WebDocumentLoaderMac::attachToFrame): Call retainDataSource unconditionally rather than
trying to use m_hasEverBeenDetached to decide if a retain is needed. Also got rid of an
assertion that m_loadingResources is empty -- not important any more.
(WebDocumentLoaderMac::detachFromFrame): Call releaseDataSource instead of using
HardRelease, but only if m_loadingResources is empty. If it's non-empty, then we'll
do the releaseDataSource later in decreaseLoadCount.
(WebDocumentLoaderMac::increaseLoadCount): Call retainDataSource unconditionally
rather than calling HardRetain only if the old set of resources was empty.
(WebDocumentLoaderMac::decreaseLoadCount): Call releaseDataSource if m_loadingResources
is empty and we're not attached to a frame. If we are attached to a frame, then
we'll do the releaseDataSource later in detachFromFrame.
(WebDocumentLoaderMac::retainDataSource): Added. Calls CFRetain, but only if the data
source is not already retained (according to the boolean).
(WebDocumentLoaderMac::releaseDataSource): Added. Calls CFRelease, but only if the data
source is currently retained (according to the boolean).
(WebDocumentLoaderMac::detachDataSource): Added. Sets m_dataSource to nil. Since this
is only called from WebDataSource's dealloc and finalize methods, it won't ever be called
when the m_isDataSourceRetained boolean is true.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@23957
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-07-03 Darin Adler <darin@apple.com>
+
+ Reviewed by Maciej.
+
+ - fix <rdar://problem/5310848> WebDataSource lifetime problem -- may be cause of the leaks seen on the buildbot
+
+ * WebView/WebDataSource.mm:
+ (-[WebDataSourcePrivate dealloc]): Added a call to the new detachDataSource function.
+ (-[WebDataSourcePrivate finalize]): Ditto.
+
+ * WebView/WebDocumentLoaderMac.h: Added detachDataSource function to be used when the
+ WebDataSource is deallocated. Added retain/releaseDataSource helper functions to be
+ used to retain and release the data source object. Replaced the m_hasEverBeenDetached
+ boolean with a more primitive and hence easier to understand m_isDataSourceRetained boolean.
+
+ * WebView/WebDocumentLoaderMac.mm:
+ (WebDocumentLoaderMac::WebDocumentLoaderMac): Initialize m_isDataSourceRetained to false.
+ (WebDocumentLoaderMac::setDataSource): Call retainDataSource instead of calling HardRetain
+ on the dataSource parameter. Also updated a comment.
+ (WebDocumentLoaderMac::attachToFrame): Call retainDataSource unconditionally rather than
+ trying to use m_hasEverBeenDetached to decide if a retain is needed. Also got rid of an
+ assertion that m_loadingResources is empty -- not important any more.
+ (WebDocumentLoaderMac::detachFromFrame): Call releaseDataSource instead of using
+ HardRelease, but only if m_loadingResources is empty. If it's non-empty, then we'll
+ do the releaseDataSource later in decreaseLoadCount.
+ (WebDocumentLoaderMac::increaseLoadCount): Call retainDataSource unconditionally
+ rather than calling HardRetain only if the old set of resources was empty.
+ (WebDocumentLoaderMac::decreaseLoadCount): Call releaseDataSource if m_loadingResources
+ is empty and we're not attached to a frame. If we are attached to a frame, then
+ we'll do the releaseDataSource later in detachFromFrame.
+ (WebDocumentLoaderMac::retainDataSource): Added. Calls CFRetain, but only if the data
+ source is not already retained (according to the boolean).
+ (WebDocumentLoaderMac::releaseDataSource): Added. Calls CFRelease, but only if the data
+ source is currently retained (according to the boolean).
+ (WebDocumentLoaderMac::detachDataSource): Added. Sets m_dataSource to nil. Since this
+ is only called from WebDataSource's dealloc and finalize methods, it won't ever be called
+ when the m_isDataSourceRetained boolean is true.
+
2007-07-03 Darin Adler <darin@apple.com>
- forgot to check in one file in the fix for <rdar://problem/5307880>
using namespace WebCore;
-@interface WebDataSourcePrivate : NSObject
-{
- @public
-
- WebDocumentLoaderMac *loader;
+@interface WebDataSourcePrivate : NSObject {
+@public
+ WebDocumentLoaderMac* loader;
id <WebDocumentRepresentation> representation;
WebUnarchivingState *unarchivingState;
BOOL representationFinishedLoading;
}
-
@end
@implementation WebDataSourcePrivate
- (void)dealloc
{
ASSERT(!loader->isLoading());
-
+ loader->detachDataSource();
loader->deref();
[representation release];
- (void)finalize
{
- ASSERT(!loader->isLoading());
+ ASSERT_MAIN_THREAD();
+ ASSERT(!loader->isLoading());
+ loader->detachDataSource();
loader->deref();
[super finalize];
/*
- * Copyright (C) 2006 Apple Computer, Inc. All rights reserved.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
class ResourceRequest;
}
-class WebDocumentLoaderMac : public WebCore::DocumentLoader
-{
+class WebDocumentLoaderMac : public WebCore::DocumentLoader {
public:
WebDocumentLoaderMac(const WebCore::ResourceRequest&, const WebCore::SubstituteData&);
void setDataSource(WebDataSource *, WebView*);
+ void detachDataSource();
WebDataSource *dataSource() const;
virtual void attachToFrame();
void increaseLoadCount(unsigned long identifier);
void decreaseLoadCount(unsigned long identifier);
+
private:
+ void retainDataSource();
+ void releaseDataSource();
+
WebDataSource *m_dataSource;
+ bool m_isDataSourceRetained;
RetainPtr<id> m_resourceLoadDelegate;
RetainPtr<id> m_downloadDelegate;
- bool m_hasEverBeenDetached;
HashSet<unsigned long> m_loadingResources;
};
/*
- * Copyright (C) 2006 Apple Computer, Inc. All rights reserved.
+ * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
#import "WebDocumentLoaderMac.h"
-#import <JavaScriptCore/Assertions.h>
-#import <WebCore/SubstituteData.h>
-#import <WebCore/FoundationExtras.h>
-
#import "WebView.h"
using namespace WebCore;
WebDocumentLoaderMac::WebDocumentLoaderMac(const ResourceRequest& request, const SubstituteData& substituteData)
: DocumentLoader(request, substituteData)
, m_dataSource(nil)
- , m_hasEverBeenDetached(false)
+ , m_isDataSourceRetained(false)
{
}
void WebDocumentLoaderMac::setDataSource(WebDataSource *dataSource, WebView *webView)
{
ASSERT(!m_dataSource);
- HardRetain(dataSource);
+ ASSERT(!m_isDataSourceRetained);
+
m_dataSource = dataSource;
-
+ retainDataSource();
+
m_resourceLoadDelegate = [webView resourceLoadDelegate];
m_downloadDelegate = [webView downloadDelegate];
- // Possibly work around a bug in Tiger AppKit where timers do not fire sometimes
+ // Work around a bug in Tiger AppKit's use of WebKit. The particular idiom used
+ // won't allow the timer to run, so deferring the main resource load with a timer
+ // causes a delay until something else wakes the run loop.
// See <rdar://problem/5266289>
if (needsAppKitWorkaround(webView))
m_deferMainResourceDataLoad = false;
void WebDocumentLoaderMac::attachToFrame()
{
DocumentLoader::attachToFrame();
- ASSERT(m_loadingResources.isEmpty());
- if (m_hasEverBeenDetached)
- HardRetain(m_dataSource);
+ retainDataSource();
}
void WebDocumentLoaderMac::detachFromFrame()
{
DocumentLoader::detachFromFrame();
-
- m_hasEverBeenDetached = true;
- HardRelease(m_dataSource);
+
+ if (m_loadingResources.isEmpty())
+ releaseDataSource();
+
+ // FIXME: What prevents the data source from getting deallocated while the
+ // frame is not attached?
}
void WebDocumentLoaderMac::increaseLoadCount(unsigned long identifier)
{
ASSERT(m_dataSource);
-
+
if (m_loadingResources.contains(identifier))
return;
-
- if (m_loadingResources.isEmpty())
- HardRetain(m_dataSource);
-
m_loadingResources.add(identifier);
+
+ retainDataSource();
}
void WebDocumentLoaderMac::decreaseLoadCount(unsigned long identifier)
{
ASSERT(m_loadingResources.contains(identifier));
-
m_loadingResources.remove(identifier);
if (m_loadingResources.isEmpty()) {
m_resourceLoadDelegate = 0;
m_downloadDelegate = 0;
- HardRelease(m_dataSource);
+ if (!frame())
+ releaseDataSource();
}
}
+
+void WebDocumentLoaderMac::retainDataSource()
+{
+ if (m_isDataSourceRetained || !m_dataSource)
+ return;
+ m_isDataSourceRetained = true;
+ CFRetain(m_dataSource);
+}
+
+void WebDocumentLoaderMac::releaseDataSource()
+{
+ if (!m_isDataSourceRetained)
+ return;
+ ASSERT(m_dataSource);
+ m_isDataSourceRetained = false;
+ CFRelease(m_dataSource);
+}
+
+void WebDocumentLoaderMac::detachDataSource()
+{
+ ASSERT(!m_isDataSourceRetained);
+ m_dataSource = nil;
+}