[webkit-reviews] review requested: [Bug 40607] Implement the sync DB API in workers : [Attachment 61134] patch #3: implementing DatabaseSync::transaction() and changeVersion() + tests

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has asked	for review:
Bug 40607: Implement the sync DB API in workers
https://bugs.webkit.org/show_bug.cgi?id=40607

Attachment 61134: patch #3: implementing DatabaseSync::transaction() and
changeVersion() + tests
https://bugs.webkit.org/attachment.cgi?id=61134&action=review

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
(In reply to comment #70)
> (From update of attachment 61025 [details])
> I think the db logic looks correct and the new classes are nicely composed.
How the SQLTransactionClient class is used is kind of funky. A stateless object
but we're making global static instance.
> 
> 51 static SQLTransactionClient* transactionClient()
> 52 {
> 53	 DEFINE_STATIC_LOCAL(SQLTransactionClient, transactionClient, ());
> 54	 return &transactionClient;
> 55 }
> 
> I don't think DEFINE_STATIC_LOCAL construction macro is thread safe and this
getter will definitely be called on different threads. It may be better to just
add an SQLTransactionClient data member to SQLTransactionSync and call the
methods thru that instance.

done.