Re: [webkit-dev] Database in Worker context

2010-01-14 Thread Darin Adler
My main feedback on this plan would be to land and review the work in pieces that are as small as possible. For example, factoring out the database bits into a separate database manager class should be done in an initial first pass that does not make other changes. I’m also not sure I like

Re: [webkit-dev] Database in Worker context

2010-01-14 Thread Eric Uhrhane
Trying again; webkit-dev doesn't seem to like my @google.com address. On Thu, Jan 14, 2010 at 2:31 PM, Eric Uhrhane er...@google.com wrote: Dmitry requested that I not pull stuff out into DatabaseManager at all, so that step got skipped. The first changelist [currently in review] does a bunch

Re: [webkit-dev] Database in Worker context

2010-01-14 Thread Darin Adler
On Jan 14, 2010, at 2:35 PM, Eric Uhrhane wrote: I think it would have been hard to break this chunk any smaller. Any time you have a chunk and you’re wondering how it could be broken up smaller, please feel free to ask that question after cc'ing me on a bug. Maciej and I, in particular, have

Re: [webkit-dev] Database in Worker context

2010-01-14 Thread Dmitry Titov
+1 to small patches. I've reviewed a couple of iterations of the patch in https://bugs.webkit.org/show_bug.cgi?id=22725 and it was so far manageable, although it could be split in 2 at least (inheritance change and WorkerThread termination change). For future patches, definitely lets split into

Re: [webkit-dev] Database in Worker context

2009-12-10 Thread Eric Uhrhane
On Wed, Dec 9, 2009 at 6:55 PM, Dmitry Titov dim...@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

Re: [webkit-dev] Database in Worker context

2009-12-10 Thread Michael Nordman
not clear to me what benefit could be had by effectively splitting a class in two Perhaps more easily worked on by multiple hands in parallel with fewer conflicts. Various bits of database stuff are better encapsulated in database specific classes (so maybe easier to learn how the system works

Re: [webkit-dev] Database in Worker context

2009-12-10 Thread Dmitry Titov
On Thu, Dec 10, 2009 at 11:09 AM, Eric Uhrhane er...@chromium.org wrote: On Wed, Dec 9, 2009 at 6:55 PM, Dmitry Titov dim...@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

Re: [webkit-dev] Database in Worker context

2009-12-10 Thread Eric Uhrhane
On Thu, Dec 10, 2009 at 1:09 PM, Dmitry Titov dim...@chromium.org wrote: On Thu, Dec 10, 2009 at 11:09 AM, Eric Uhrhane er...@chromium.org wrote: On Wed, Dec 9, 2009 at 6:55 PM, Dmitry Titov dim...@chromium.org wrote: Thanks for the link to the prototype code! If I understand it right and

Re: [webkit-dev] Database in Worker context

2009-12-09 Thread Dmitry Titov
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

Re: [webkit-dev] Database in Worker context

2009-12-09 Thread Eric Uhrhane
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

Re: [webkit-dev] Database in Worker context

2009-12-09 Thread Dmitry Titov
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