[webkit-dev] Database in Worker context

Dmitry Titov dimich at chromium.org
Thu Jan 14 15:50:07 PST 2010


+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 smaller ones, 60kb is on a biggish side.

There is no DatabaseManager class anymore since the database-supporting
methods are being moved to ScriptExecutionContext from which both Document
and WorkerContext are derived from. Having a 'Context' to own another
'Manager' seemed like too much layers.

I lean to r+ the first patch after latest set of notes is addressed, more
eyes are always welcome though :-)

Dmitry

On Thu, Jan 14, 2010 at 3:06 PM, Darin Adler <darin at apple.com> wrote:

> 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 a lot of experience with this, much of
> dating from the first year of the Safari project. I made changes that were
> too large and learned “no apologies” strategies to break them up into much
> smaller pieces that were easier to get right and to read and review.
>
>    -- Darin
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100114/f12c8dc0/attachment.html>


More information about the webkit-dev mailing list