RESOLVED FIXED Bug 33252
Allow custom memory allocation control for IconDatabaseClient class 
https://bugs.webkit.org/show_bug.cgi?id=33252
Summary Allow custom memory allocation control for IconDatabaseClient class
Zoltan Horvath
Reported 2010-01-06 04:32:59 PST
Inherits the following struct from Noncopyable because it is instantiated by 'new' and no need to be copyable: class name - instantiated at: WebCore/'location' class IconDatabaseClient - loader/icon/IconDatabase.cpp:89
Attachments
proposed patch (1.46 KB, patch)
2010-01-06 04:34 PST, Zoltan Horvath 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Zoltan Horvath
Comment 1 2010-01-06 04:34:23 PST
Created attachment 45960 [details] proposed patch
WebKit Review Bot
Comment 2 2010-01-06 04:36:29 PST
style-queue ran check-webkit-style on attachment 45960 [details] without any errors.
Eric Seidel (no email)
Comment 3 2010-01-06 09:00:02 PST
Comment on attachment 45960 [details] proposed patch This is an Abstract Base Class. I don't think that this interface should require non-copyable. The implementors of this interface should inherit from NonCopyable or FastAllocBase instead, no?
Darin Adler
Comment 4 2010-01-06 09:38:22 PST
(In reply to comment #3) > This is an Abstract Base Class. It's not. It has no pure virtual functions and is instantiated, as the patch says, on line 89 of IconDatabase.cpp. However, I think it would make more sense if it was an abstract base class. The version that does nothing could be derived from it. I think there's no harm in making the change from this patch, but it's not great to have this client different from other clients.
Zoltan Horvath
Comment 5 2010-01-18 00:02:40 PST
So, what should be the best solution? Using fastNew in this case wouldn't be nice. Add an extra class? Or Inherit this client?
Darin Adler
Comment 6 2010-01-18 10:52:32 PST
(In reply to comment #5) > So, what should be the best solution? Using fastNew in this case wouldn't be > nice. Add an extra class? Or Inherit this client? I know you’re hoping I’ll decide, but both options seem OK to me.
Zoltan Horvath
Comment 7 2010-01-28 05:59:04 PST
Comment on attachment 45960 [details] proposed patch Okay, this change modifies fewest. I marked r? the patch again.
Eric Seidel (no email)
Comment 8 2010-02-01 16:11:39 PST
Attachment 45960 [details] was posted by a committer and has review+, assigning to Zoltan Horvath for commit.
Zoltan Horvath
Comment 9 2010-02-01 23:56:33 PST
Comment on attachment 45960 [details] proposed patch Clearing flags on attachment: 45960 Committed r54210: <http://trac.webkit.org/changeset/54210>
Zoltan Horvath
Comment 10 2010-02-01 23:56:44 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.