[webkit-reviews] review requested: [Bug 29115] Allow multiple read transactions to run concurrently on the same DB thread : [Attachment 39725] patch

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has asked	for review:
Bug 29115: Allow multiple read transactions to run concurrently on the same DB
thread
https://bugs.webkit.org/show_bug.cgi?id=29115

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

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
(In reply to comment #6)
> on the new struct, add a default ctor that zero-inits
> numPendingWriteTransactions 

done.

> add a code comment that indicates the String identifies a database file

done.

> typedef HashMap<String, CoordinationInfo> CoordinationInfoMap;
> CoordinationInfoMap m_coordinationInfoMap;

done.

> 89	 CoordinationInfo& cInfo = it->second;
> 
> I think webkit style would have you call this either 'info' or
> 'coordinationInfo', the cJazz has a hungarian sounds to it.

changed to 'info'.

> 57	     CoordinationInfo cInfo;
> 58	     cInfo.numPendingWriteTransactions = 0;
> 59	     it = m_pendingTransactions.add(dbIdentifier, cInfo).first;
> 
> subtract two lines

done.

>  131		   if (!cInfo.pendingTransactions.first()->isReadOnly())
>  132		       cInfo.pendingTransactions.first()->lockAcquired();
>  133		   else {
>  134		       TransactionsQueue::iterator pt_it =
> cInfo.pendingTransactions.begin();
>  135		       while ((pt_it != cInfo.pendingTransactions.end()) &&
> (pt_it->get()->isReadOnly())) {
>  136			   pt_it->get()->lockAcquired();
>  137			   ++pt_it;
>  138		       }
>  139		   }
> 
> Since the else clause requires braces, put them around the if clause too.

i'm not sure the webkit style allows this. at the very least, they have this
example that is correct according to the webkit guide:

if (condition)
    doSomething();
else {
    ...
}