Bug 42161 - Removes cycles caused by "m_this" members in a few IndexedDB classes.
Summary: Removes cycles caused by "m_this" members in a few IndexedDB classes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-13 06:02 PDT by Marcus Bulach
Modified: 2010-07-13 09:12 PDT (History)
2 users (show)

See Also:


Attachments
Patch (10.77 KB, patch)
2010-07-13 06:06 PDT, Marcus Bulach
no flags Details | Formatted Diff | Diff
Patch (11.18 KB, patch)
2010-07-13 07:24 PDT, Marcus Bulach
no flags Details | Formatted Diff | Diff
Patch (10.90 KB, patch)
2010-07-13 08:08 PDT, Marcus Bulach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marcus Bulach 2010-07-13 06:02:42 PDT
Removes cycles caused by "m_this" members in a few IndexedDB classes.
Comment 1 Marcus Bulach 2010-07-13 06:06:45 PDT
Created attachment 61364 [details]
Patch
Comment 2 Marcus Bulach 2010-07-13 06:08:08 PDT
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!
Comment 3 Jeremy Orlow 2010-07-13 06:17:35 PDT
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?
Comment 4 Marcus Bulach 2010-07-13 07:24:02 PDT
Created attachment 61371 [details]
Patch
Comment 5 Marcus Bulach 2010-07-13 07:28:09 PDT
(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?
Comment 6 Jeremy Orlow 2010-07-13 07:40:54 PDT
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.
Comment 7 Marcus Bulach 2010-07-13 08:08:28 PDT
Created attachment 61376 [details]
Patch
Comment 8 Marcus Bulach 2010-07-13 08:09:15 PDT
(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.
Comment 9 Jeremy Orlow 2010-07-13 08:18:33 PDT
Comment on attachment 61376 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2010-07-13 09:12:18 PDT
Comment on attachment 61376 [details]
Patch

Clearing flags on attachment: 61376

Committed r63211: <http://trac.webkit.org/changeset/63211>
Comment 11 WebKit Commit Bot 2010-07-13 09:12:23 PDT
All reviewed patches have been landed.  Closing bug.