Bug 42161 – Removes cycles caused by "m_this" members in a few IndexedDB classes.

• Component: New Bugs
• Status: RESOLVED FIXED

[Marcus Bulach]

Removes cycles caused by "m_this" members in a few IndexedDB classes.

[Marcus Bulach]

Created attachment 61364 [details]
Patch

[Marcus Bulach]

Hi Jeremy,

Following your suggestion on https://bugs.webkit.org/show_bug.cgi?id=41888, this patch contains only the "m_this" / "IDBAny" cleanup. Would you mind taking a look please?

Thanks!

[Jeremy Orlow]

Comment on attachment 61364 [details]
Patch

WebCore/storage/IDBAny.h:46
 +      static PassRefPtr<IDBAny> create();
Do we still need this?

WebCore/storage/IDBAny.cpp:40
 +  static PassRefPtr<IDBAny> createIDBAny(PassRefPtr<T> idbType)
Hm...it seems like you could have just added the template function to the IDBAny.h if you wanted...right?

[Marcus Bulach]

Created attachment 61371 [details]
Patch

[Marcus Bulach]

(In reply to comment #3)
> (From update of attachment 61364 [details])
> WebCore/storage/IDBAny.h:46
>  +      static PassRefPtr<IDBAny> create();
> Do we still need this?
yep, http://trac.webkit.org/browser/trunk/WebCore/storage/IDBRequest.cpp still uses it..

> 
> WebCore/storage/IDBAny.cpp:40
>  +  static PassRefPtr<IDBAny> createIDBAny(PassRefPtr<T> idbType)
> Hm...it seems like you could have just added the template function to the IDBAny.h if you wanted...right?

indeed, done. note though that due to the automatic type conversion from Foo* to PassRefPtr<Foo>, the caller needs to specify the type on the templated call site.

another look please?

[Jeremy Orlow]

Comment on attachment 61371 [details]
Patch

WebCore/storage/IDBAny.h:47
 +      // Only valid with T accepted by set().
I'd lean towards leaving this comment out.  I think it's pretty clear from the code.

WebCore/storage/IDBAny.h:49
 +      static PassRefPtr<IDBAny> create(PassRefPtr<T> idbObject)
I think it'd be slightly cleaner if you took in a raw pointer and created the passRefPtr in this method.  That way we don't have all the redundant data scattered through the rest of the file.

[Marcus Bulach]

Created attachment 61376 [details]
Patch

[Marcus Bulach]

(In reply to comment #6)
> (From update of attachment 61371 [details])
> WebCore/storage/IDBAny.h:47
>  +      // Only valid with T accepted by set().
> I'd lean towards leaving this comment out.  I think it's pretty clear from the code.
removed.

> 
> WebCore/storage/IDBAny.h:49
>  +      static PassRefPtr<IDBAny> create(PassRefPtr<T> idbObject)
> I think it'd be slightly cleaner if you took in a raw pointer and created the passRefPtr in this method.  That way we don't have all the redundant data scattered through the rest of the file.

agreed, done.

[Jeremy Orlow]

Comment on attachment 61376 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 61376 [details]
Patch

Clearing flags on attachment: 61376

Committed r63211: <http://trac.webkit.org/changeset/63211>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.