Bug 33252

Summary:

Allow custom memory allocation control for IconDatabaseClient class

Product:

WebKit

Reporter:

Zoltan Horvath <zoltan>

Component:

WebCore Misc.

Assignee:

Zoltan Horvath <zoltan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, eric, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
proposed patch	none

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	Details Formatted Diff Diff
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.