[webkit-reviews] review requested: [Bug 31388] Fix the ref-counting bug in Database.cpp : [Attachment 43025] patch

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has asked	for review:
Bug 31388: Fix the ref-counting bug in Database.cpp
https://bugs.webkit.org/show_bug.cgi?id=31388

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

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
(In reply to comment #2)
> (From update of attachment 43024 [details])
> r- for no explanation of fix in change log (and it looks equivalent to me,
> sorry that I don't see it).
> 
> 
> > Index: WebCore/ChangeLog
> > +2009-11-11  Dumitru Daniliuc  <dumi at chromium.org>
> > +
> > +	     Reviewed by NOBODY (OOPS!).
> > +
> > +	     Database.cpp does not correctly handle ref-counting in
> > +	Database::openDatabase().
> 
> Please remove the TAB here.

done.

> Also please explain the bug that is fixed. The code looks equivalent to me.

i'm still not very clear on PassRefPtr usages, so i might be wrong here, but i
think:

RefPtr<Foo> foo = adoptRef(new Foo()); // foo has a ref-count of 0

PassRefPtr<Foo> getFoo() { return adoptRef(new Foo()); }
RefPtr<Foo> foo = getFoo(); // foo has a ref-count of 1

(i'll ask dglazkov for more details, but there certainly _is_ a difference
between these two (i checked))

> > Index: WebCore/storage/Database.cpp
> > +PassRefPtr<Database> Database::create(Document* document, const String&
name, const String& expectedVersion, const String& displayName, unsigned long
estimatedSize) {
> 
> The open brace for the function needs to go on the next line.

done. also fixed the indentation to 4 spaces.

> > Index: WebCore/storage/Database.h
> > +	 static PassRefPtr<Database> create(Document* document, const String&
name, const String& expectedVersion, const String& displayName, unsigned long
estimatedSize);
> 
> The param name "document" shouldn't be here.

removed. also removed it from openDatabase().