[webkit-reviews] review denied: [Bug 27967] Decouple the code that deals with the main DB and quota management from the rest of the DB code : [Attachment 34296] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 7 14:21:06 PDT 2009


Eric Seidel <eric at webkit.org> has denied Dumitru Daniliuc <dumi at chromium.org>'s
request for review:
Bug 27967: Decouple the code that deals with the main DB and quota management
from the rest of the DB code
https://bugs.webkit.org/show_bug.cgi?id=27967

Attachment 34296: patch
https://bugs.webkit.org/attachment.cgi?id=34296&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I need more background in the ChangeLog here.  Why are we doing this?  So that
Chromium can have the TransactionClient in a separate process?

0 seems like the wrong default here:
 71						 PassRefPtr<VoidCallback>,
PassRefPtr<SQLTransactionWrapper>, SQLTransactionClient* transactionClient =
0);

I would think that we would want transactions to "work out of the box" for
webkit clients.  it seems possible here to construct a transaction which would
not work correctly out of the box!

Style:
 48 void SQLTransactionClient::databaseSizeChanged(Database* database) {

Why is a copy needed here?
 59	RefPtr<SecurityOrigin> origin = database->securityOriginCopy();

In general the chagnge looks fine.  I'm concerned about the =0 default.  I also
need more background in teh ChangeLog about why we're doing this (I'm not
opposed to it, but the ChangeLog needs to document our reasoning.)


More information about the webkit-reviews mailing list