Thanks for the link to the prototype code! If I understand it right and the DatabaseManager can not have any other relationship with ScriptExecutionContext but 1-1 and their lifetime is the same then it's not clear to me what benefit could be had by effectively splitting a class in two. The fact that DatabaseManager is RefCounted also hints that some other object could take ownership of it as well outside of lifetime of ScriptExecutionContext, but is this true?
ScriptExecutionContext collects functionality common for Workers and Documents, and as such is a home for a few 'groups' of methods, like a few methods to deal with MessagePorts. MessagePort, in turn, has a raw pointer back to ScriptExecutionContext - so it's clear that MessagePorts do not outlive the SEC. Same pattern could be used for ScriptExecutionContext/Database, for consistency. It also simplifies design a bit - no need for a special refcounted manager class, and things like callOnJavaScriptThread could be replaced by SEC::postTask() which is already implemented. Dmitry On Wed, Dec 9, 2009 at 5:21 PM, Eric Uhrhane <er...@chromium.org> wrote: > On Wed, Dec 9, 2009 at 4:11 PM, Dmitry Titov <dim...@chromium.org> wrote: > > On Wed, Dec 9, 2009 at 12:58 PM, Eric Uhrhane <er...@chromium.org> > wrote: > >> > >> I've pulled the database-related members out of Document and made a > >> new class for them, DatabaseManager. An instance of that is owned by > >> each ScriptExecutionContext. There are two flavors, > >> DocumentDatabaseManager and WorkerDatabaseManager. They're not very > >> different, but in a few cases it was necessary to distinguish between > >> them > > > > I don't see your code, just generic thought: If those classes are small, > > then perhaps ScriptExecutionContext could absorb a couple of methods to > deal > > with that? Some of them could be virtual and implemented differently on > > Document and WorkerContext. > > Dmitry > > You don't see the code because I sent this as an early > warning--there's nothing checked in yet. > The classes aren't huge, and in fact most of the code was in Document > before, but given that it's all to do with a separable concept [the > Database], I thought it nicer to pull it out. Let's > see...DatabaseManger.cpp is 172 lines [including copyright headers, > etc.] and the header is 107. Seems like a lot to sprinkle through > ScriptExecutionContext, Database, and WorkerContext. You can take a > look at my current code [where I'm backing it up--this won't actually > be sent for review from this repository] at > http://codereview.chromium.org/401016/show [mostly storage + v8 > bindings] and http://codereview.chromium.org/464018/show [mostly > Chrome browser process stuff] and see what you think. > > Thanks, > > Eric >
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev