[webkit-dev] Database in Worker context

[Eric Uhrhane]

On Thu, Dec 10, 2009 at 1:09 PM, Dmitry Titov <dimich at chromium.org> wrote:
> On Thu, Dec 10, 2009 at 11:09 AM, Eric Uhrhane <ericu at chromium.org> wrote:
>>
>> 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.
>
> It feels that there is a new class that does not have a specific lifetime or
> relationship to other existing classes, as it is a mechanical part of some
> other class rather...
> I look at how m_document is used by current Database and it feels that if
> SEC had a databaseThread() accessor,  then all other methods like
> addOpenDatabase etc could be part of DatabaseThread instead, with Database
> keeping a pointer to its DatabaseThread. What do you think about this?

You mean that DatabaseManager would merge into DatabaseThread, which
would then have DocumentDatabaseThread and WorkerDatabaseThread to
cover the DocumentDatabaseManager and WorkerDatabaseManager split?

That's doable.  It stretches the DT class a bit, but it's not too bad.
 It looks like there's some redundancy between the two
[m_openDatabaseSet in both] that I might be able to clear out, so
that's a plus.

>> > 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.
>
> I understand the reason for callOnJavascriptThread... This is exactly as
> SEC::postTask is defined - it posts a task from any thread to the one which
> executes JavaScript code for a given SEC. So for Document it is the main
> thread, for Workers - the thread associated with WorkerThread. I think they
> are the same thing - you can just use postTask in place of
> callOnJavascriptThread.

Ah, I see--thanks, I had missed that function when implementing this.
I've switched over to using it now.

     Eric