https://code.google.com/p/chromium/issues/detail?id=122249 Valgrind says: { bug_122249d Memcheck:Leak fun:_Znw* fun:_ZN6WebKit17IDBCallbacksProxy9onSuccessEN3WTF10PassRefPtrIN7WebCore27IDBDatabaseBackendInterfaceEEE fun:_ZN7WebCore22IDBDatabaseBackendImpl19processPendingCallsEv fun:_ZN7WebCore22IDBDatabaseBackendImpl35transactionFinishedAndCompleteFiredEN3WTF10PassRefPtrINS_25IDBTransactionBackendImplEEE fun:_ZN7WebCore25IDBTransactionBackendImpl6commitEv } Leak is pretty obvious, here's a speculative fix but I haven't run it through valgrind yet: void WebIDBCallbacksImpl::onSuccess(WebIDBDatabase* webKitInstance) { + OwnPtr<WebIDBDatabase> webDatabase = adoptPtr(webKitInstance); if (m_databaseProxy) { m_callbacks->onSuccess(m_databaseProxy.release()); return; } - m_callbacks->onSuccess(IDBDatabaseBackendProxy::create(adoptPtr(webKitInstance))); + m_callbacks->onSuccess(IDBDatabaseBackendProxy::create(webDatabase.release())); }
Created attachment 178674 [details] Patch
dgrogan@, alecflett@ - please take a look Verified this addresses the leak w/ valgrind, at least in basics.html
Comment on attachment 178674 [details] Patch LGTM
tony@ - r?
Comment on attachment 178674 [details] Patch Clearing flags on attachment: 178674 Committed r137330: <http://trac.webkit.org/changeset/137330>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 104719
Created attachment 179521 [details] Patch
The previous patch was on the receiving end, and didn't account for the "real" Chromium implementation which correct passed in the right objects. For the Chromium-DRT implementation, simply don't generate a new wrapper object if one was sent through in onUpgradeNeeded, and let it be ignored. dgrogan@ - another look?
In chromium, won't this cause IndexedDBCallbacksDatabase::onSuccess(WebKit::WebIDBDatabase* idb_object) to attempt to delete NULL? Also, to ensure I understand, this leak only happens in DRT, correct? Not chromium?
(In reply to comment #10) > In chromium, won't this cause > > IndexedDBCallbacksDatabase::onSuccess(WebKit::WebIDBDatabase* idb_object) > > to attempt to delete NULL? Correct (but delete null is a safe no-op in C++) > Also, to ensure I understand, this leak only happens in DRT, correct? Not chromium? Correct. (Well, Chromium's DRT to be precise, it wouldn't affect other ports.)
LGTM
tony@ - patch is completely different, so can you take another look?
Comment on attachment 179521 [details] Patch Clearing flags on attachment: 179521 Committed r137773: <http://trac.webkit.org/changeset/137773>