[webkit-dev] Database in Worker context
dimich at chromium.org
Wed Dec 9 18:55:19 PST 2009
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
could be replaced by SEC::postTask() which is already implemented.
On Wed, Dec 9, 2009 at 5:21 PM, Eric Uhrhane <ericu at chromium.org> wrote:
> On Wed, Dec 9, 2009 at 4:11 PM, Dmitry Titov <dimich at chromium.org> wrote:
> > On Wed, Dec 9, 2009 at 12:58 PM, Eric Uhrhane <ericu at chromium.org>
> >> 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
> > 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the webkit-dev