[webkit-reviews] review requested: [Bug 40607] Implement the sync DB API in workers : [Attachment 59713] patch #1: implement DatabaseSync::openDatabaseSync()

[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 59713: patch #1: implement DatabaseSync::openDatabaseSync()
https://bugs.webkit.org/attachment.cgi?id=59713&action=review

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
(In reply to comment #32)
> (From update of attachment 59696 [details])
> > +void DatabaseSync::closeImmediately()
> >  {
> > -	 ASSERT(m_scriptExecutionContext->isContextThread());
> > -	 return m_scriptExecutionContext.get();
> > +	 if (!opened())
> > +	     return;
> 
> I think you should move the call to opened() down so that the method is
called on the context thread.

done.

> > DatabaseSync::~DatabaseSync()
> > {
> >	ASSERT(m_scriptExecutionContext->isContextThread());
> > +	 closeImmediately();
> > }
> 
> Oh, thats a virtual method. Probably want to avoid calling that from the
dtor.
> http://www.artima.com/cppsource/nevercall.html

gotta love C++... i changed the call to closeImmediately() to:

if (opened()) {
    DatabaseTracker.tracker().removeOpenDatabase(this);
    closeDatabase();
}