IndexedDB: signal IDBFactoryBackendInterface destruction to embedder
Created attachment 73480 [details] Patch
LGTM
Why is this signal needed? The factory only goes away when webkit is shutting down. I'm surprised this is useful.
(In reply to comment #3) > Why is this signal needed? The factory only goes away when webkit is shutting down. That's what we want to detect. This seemed like a relatively clean way of piping it through to the BrowserWebKitClientImpl on the Chromium side, which is what we're trying to reach. Do you see a better way?
Why is chromiums factories lifetime related to that of a factory in the renderer? I mean the browsers factory likely outlives them all. Note also that we need to make sure that we handle lifetimes correctly when the renderer crashes (which we do not at the moment, iirc)--which I feel would make this unnecessary as well. Can you please explain the bigger picture here?
(In reply to comment #5) > Why is chromiums factories lifetime related to that of a factory in the renderer? I mean the browsers factory likely outlives them all. Note also that we need to make sure that we handle lifetimes correctly when the renderer crashes (which we do not at the moment, iirc)--which I feel would make this unnecessary as well. Can you please explain the bigger picture here? Umm, this is backend code so it runs in the browser not the renderer process. Renderer crashes don't really affect matter here, right? We just want to stop the utility process cleanly when the indexed database shuts down. Andrei
Oops, you're right...was mixed up. The object store objects don't hold a ref to the factory though, which means that it could go away before were done with the utility process, right?
(In reply to comment #7) > Oops, you're right...was mixed up. Adding a comment trying to make it less confusing. > The object store objects don't hold a ref to the factory though, which means that it could go away before were done with the utility process, right? The destruction of the factory happens after the IndexedDBDispatcherHost (in Chromium) is destructed, so I don't think any more IndexedDB activity could continue after that point - it should be fine to tell the utility process to stop at this point.
Created attachment 73604 [details] Patch
(In reply to comment #8) > (In reply to comment #7) > > Oops, you're right...was mixed up. > Adding a comment trying to make it less confusing. > > > The object store objects don't hold a ref to the factory though, which means that it could go away before were done with the utility process, right? > > The destruction of the factory happens after the IndexedDBDispatcherHost (in Chromium) is destructed, so I don't think any more IndexedDB activity could continue after that point - it should be fine to tell the utility process to stop at this point. Then why does Chromium require this signal? Can't we just shut it down when the dispatcher host shuts down? Also, this is true today, but in the future when we have background threads and such this assumption probably won't hold unless the dispatcher doesn't go away until all running transactions and such have ended.
(In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #7) > > > Oops, you're right...was mixed up. > > Adding a comment trying to make it less confusing. > > > > > The object store objects don't hold a ref to the factory though, which means that it could go away before were done with the utility process, right? > > > > The destruction of the factory happens after the IndexedDBDispatcherHost (in Chromium) is destructed, so I don't think any more IndexedDB activity could continue after that point - it should be fine to tell the utility process to stop at this point. > > Then why does Chromium require this signal? It's the cleanest solution to what we're trying to do in http://codereview.chromium.org/4678002/ > Can't we just shut it down when the dispatcher host shuts down? We tried that at first, but didn't see a good way of getting from the dispatcher host to the WebKitClient. This solution feels natural in the way that the WebKitClient (in this case BrowserWebKitClient impl on the Chromium side) gets a call from WebKit saying all the IDB stuff is done and shutting down so it can stop the utility process. Receiving calls from WebKit is what the WebKitClient does; trying to call it from somewhere else seems less clean. > Also, this is true today, but in the future when we have background threads and such this assumption probably won't hold unless the dispatcher doesn't go away until all running transactions and such have ended. The IDBTransactionCoordinator is destructed before the factory, so at this point I suppose all transactions should have finished.
Comment on attachment 73604 [details] Patch r=me Still not convinced this is the best way, but I can't think of a better one.
Comment on attachment 73604 [details] Patch Clearing flags on attachment: 73604 Committed r71829: <http://trac.webkit.org/changeset/71829>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/71829 might have broken Leopard Intel Release (Tests) The following tests are not passing: media/video-played-collapse.html media/video-played-ranges-1.html media/video-played-reset.html