[webkit-dev] Database in Worker context

[Eric Uhrhane]

On Wed, Dec 9, 2009 at 6:55 PM, Dmitry Titov <dimich at chromium.org> wrote:
> 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?

That's there because the DatabaseThread [and therefore the Database]
can stick around for a bit after the ScriptExecutionContext is deleted
if I make it non-refcounted.  If I move the DatabaseManager parts into
the ScriptExecutionContext, then I have to make sure that the SEC
sticks around until after the Database and DatabaseThread have gone
away.

I can do that--the benefit I thought I was getting wasn't one of
object lifetime.  I just thought the classes were cleaner this way.
But if you really think it's better to have it all in together, I can
certainly move it.

> 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.

The point of CallOnJavascriptThread is that the JS thread is the Main
Thread in the Document context, but [in Chromium's implementation] NOT
in the Worker context.  I needed a virtual method to distinguish
between the two cases, since the Database code previously treated
isMainThread as synonymous with isJavascriptThread.  But of course
that virtual method can be moved to Document and SEC instead of DDM
and WDM.

> 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>
>> > 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 at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>