IndexedDB: IDBTransaction should manage lifetime of IDBRequests
Created attachment 182368 [details] Patch
Speculative fix for possible source of crashes. In IDBObjectStore.cpp it is assumed that the request is kept alive by the owning transaction, but transactions didn't actually do that.
Comment on attachment 182368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182368&action=review > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:82 > + relaxAdoptionRequirement(); Is this because of the iterator, or removal from the m_requestList? Is there any way to call some of the helper methods like removeFirst or remove(iterator it) ?
(In reply to comment #3) > (From update of attachment 182368 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=182368&action=review > > > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:82 > > + relaxAdoptionRequirement(); > > Is this because of the iterator, or removal from the m_requestList? Is there any way to call some of the helper methods like removeFirst or remove(iterator it) ? It on insertion into m_requestList c/o IDBTransaction::registerRequest(), which creates the first RefPtr<> to the request before the IDBRequest's constructor has completed.
ah, I see it. LGTM (seems like the proper ownership anyway)
tony@ - can you take a look?
Comment on attachment 182368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182368&action=review >>> Source/WebCore/Modules/indexeddb/IDBRequest.cpp:82 >>> + relaxAdoptionRequirement(); >> >> Is this because of the iterator, or removal from the m_requestList? Is there any way to call some of the helper methods like removeFirst or remove(iterator it) ? > > It on insertion into m_requestList c/o IDBTransaction::registerRequest(), which creates the first RefPtr<> to the request before the IDBRequest's constructor has completed. Rather than using relaxAdoptionRequirement, could we move the call to transaction->registerRequest() into the IDBRequest::create methods? I think in the long term, we want to remove relaxAdoptionRequirement() from RefCounted.h and it looks like IDB is the only place that uses relaxAdoptionRequirement().
Created attachment 182423 [details] Patch
(In reply to comment #7) > Rather than using relaxAdoptionRequirement, could we move the call to transaction->registerRequest() into the IDBRequest::create methods? Good suggestion - patch updated. (as you noticed...) > I think in the long term, we want to remove relaxAdoptionRequirement() from RefCounted.h and it looks like IDB is the only place that uses relaxAdoptionRequirement(). Once alecflett@'s current refactor is done we can tackle removing the other uses.
Comment on attachment 182423 [details] Patch Clearing flags on attachment: 182423 Committed r139518: <http://trac.webkit.org/changeset/139518>
All reviewed patches have been landed. Closing bug.