[Webkit-unassigned] [Bug 27966] Race condition in the database code

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27966


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34535|review?                     |review-
               Flag|                            |




--- Comment #21 from Eric Seidel <eric at webkit.org>  2009-08-12 11:04:21 PDT ---
(From update of attachment 34535)
For the future (this one being in pre-existing code), WebKit code tends to
favor early returns instead of long if-blocks:
544545     if (m_document->databaseThread()) {

Yes, our style is strange, but comments count as "lines" for multi-line if
blocks and shoudl have  { } (I used to get this confused myself):
 87     if (pendingTransactions.isEmpty())
 88         // No more pending transactions; delete dbIdentifier's queue
 89         m_pendingTransactions.remove(it);
 90     else
 91         // We have more pending transactions; start the next one
 92         pendingTransactions.first()->lockAcquired();

No argument names when they don't add clarity:
 46         void acquireLock(SQLTransaction* transaction);
 47         void releaseLock(SQLTransaction* transaction);

Why not add named functions for these:
 34                      // statement success callback
 35                      function(result) { log(dbName + " read statement
succeeded"); },
 36 
 37                      // statement failure callback
 38                      function(tx, error) { log(dbName + " read statement
failed: " + error.message); });

Then you dont' have to declare them twice, nor do you need the extra lines of
comments or spaces because the method names make it obvious what you're doing.

function successCallback(testName) {
return function(result) { log(dbName + " " + testName + " statement
succeeded"); }

successCallback("write")

If you wanted, you coudl even make them more general and cover the other places
you use success/error callbacks too.

I'm ready to r+ this, but I think we should go one more round (since iirc you
are not yet a committer).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.