[Webkit-unassigned] [Bug 27966] Race condition in the database code
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 12 15:40:30 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=27966
--- Comment #22 from Dumitru Daniliuc <dumi at chromium.org> 2009-08-12 15:40:27 PDT ---
(In reply to comment #21)
> (From update of attachment 34535 [details])
> 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()) {
fixed.
> 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();
fixed.
> No argument names when they don't add clarity:
> 46 void acquireLock(SQLTransaction* transaction);
> 47 void releaseLock(SQLTransaction* transaction);
removed.
> 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")
done.
> If you wanted, you coudl even make them more general and cover the other places
> you use success/error callbacks too.
we could do this, but since the messages are quite different (read vs. write,
statement vs. transaction, and read/write doesn't even apply in case of a
transaction callback), i feel like this would only lead to more complicated and
unintuitive implementations for successCallback() and errorCallback(), as well
as some pretty cryptic calls to these functions.
> I'm ready to r+ this, but I think we should go one more round (since iirc you
> are not yet a committer).
i'm working my way there. :)
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list