On Thu, Dec 10, 2009 at 11:09 AM, Eric Uhrhane <[email protected]> wrote:
> On Wed, Dec 9, 2009 at 6:55 PM, Dmitry Titov <[email protected]> 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? > 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. Dmitry
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
