|Summary:||Allow custom memory allocation control for IconDatabaseClient class|
|Product:||WebKit||Reporter:||Zoltan Horvath <zoltan>|
|Component:||WebCore Misc.||Assignee:||Zoltan Horvath <zoltan>|
|Severity:||Normal||CC:||darin, eric, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Zoltan Horvath 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
Comment 2 WebKit Review Bot 2010-01-06 04:36:29 PST
style-queue ran check-webkit-style on attachment 45960 [details] without any errors.
Comment 3 Eric Seidel (no email) 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?
Comment 4 Darin Adler 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.
Comment 5 Zoltan Horvath 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?
Comment 6 Darin Adler 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.
Comment 7 Zoltan Horvath 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.
Comment 8 Eric Seidel (no email) 2010-02-01 16:11:39 PST
Attachment 45960 [details] was posted by a committer and has review+, assigning to Zoltan Horvath for commit.
Comment 9 Zoltan Horvath 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>
Comment 10 Zoltan Horvath 2010-02-01 23:56:44 PST
All reviewed patches have been landed. Closing bug.