Bug 31473

Summary: Allow custom memory allocation control for 10 classes of the platform directory in WebCore
Product: WebKit Reporter: Zoltan Horvath <zoltan>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed patch
none
proposed patch darin: review+

Description Zoltan Horvath 2009-11-13 07:23:20 PST
Inherits the following classes from FastAllocBase because these are instantiated by 'new':

class  AnimationList         - instantiated at WebCore/rendering/style/StyleRareNonInheritedData.cpp:85
class  Color                 - instantiated at WebCore/rendering/RenderTheme.cpp:48
struct Length                - instantiated at WebCore/platform/Length.cpp:103
class  PlatformKeyboardEvent - instantiated at WebCore/dom/KeyboardEvent.cpp:63
class  ContextMenuItem       - instantiated at WebCore/platform/ContextMenu.cpp:70
class  DeprecatedPtrList     - instantiated at WebCore/rendering/RenderBlock.cpp:2284

Inherits the following classes from Noncopyable because these are instantiated by 'new' and no need to be copyable:

class  GraphicsContextPrivate - instantiated at WebCore/platform/graphics/GraphicsContext.cpp:78
class  FontCache              - instantiated at WebCore/platform/graphics/qt/FontCacheQt.cpp:43
struct MediaPlayerFactory     - instantiated at WebCore/platform/graphics/MediaPlayer.cpp:163
Comment 1 Zoltan Horvath 2009-11-13 07:24:24 PST
Created attachment 43155 [details]
proposed patch
Comment 2 Zoltan Horvath 2009-11-13 07:29:11 PST
Created attachment 43156 [details]
proposed patch
Comment 3 Darin Adler 2009-11-13 08:53:09 PST
Comment on attachment 43156 [details]
proposed patch

> -    class ContextMenuItem {
> +    class ContextMenuItem : public FastAllocBase {

Should be Noncopyable instead.

> -    class PlatformKeyboardEvent {
> +    class PlatformKeyboardEvent : public FastAllocBase {

I think this should be Noncopyable instead. Needs testing to be sure.

review- because we'd like keep direct FastAllocBase use to a minimum and use Noncopyable whenever appropriate.
Comment 4 Zoltan Horvath 2009-11-13 09:47:16 PST
(In reply to comment #3)
> (From update of attachment 43156 [details])
> > -    class ContextMenuItem {
> > +    class ContextMenuItem : public FastAllocBase {
> 
> Should be Noncopyable instead.

It can't be Noncopyable because WebCore/platform/qt/ContextMenuQt.cpp:51 m_items.append(item); copies a ContextMenuItem instance.


> > -    class PlatformKeyboardEvent {
> > +    class PlatformKeyboardEvent : public FastAllocBase {
> 
> I think this should be Noncopyable instead. Needs testing to be sure.
> 
> review- because we'd like keep direct FastAllocBase use to a minimum and use
> Noncopyable whenever appropriate.

The PlatformKeyboardEvent's problem comes from WebCore/dom/KeyboardEvent.cpp:63 it would use its copy constructor.
Comment 5 Zoltan Horvath 2009-11-13 10:01:10 PST
Btw, I analyzed all classes in the patch, and if a class is inherited from FastAllocBase than it won't compile with Noncopyable some reason.
Comment 6 Zoltan Horvath 2009-11-16 23:10:32 PST
Comment on attachment 43156 [details]
proposed patch

I mark it to r? again.
Comment 7 Zoltan Horvath 2009-11-18 15:49:40 PST
Landed in 51149.
http://trac.webkit.org/changeset/51149