Bug 32106 – Allow custom memory allocation control for the other part of page directory in WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Zoltan Horvath]

Created attachment 44215 [details]
Patch

Inherits the following class from Noncopyable because it is instantiated
by 'new' and no need to be copyable:

class/struct name       - instantiated at: WebCore/'location'

class ContextMenuClient - (its child class) svg/graphics/SVGImage.cpp:232
class DragClient        - (its child class) svg/graphics/SVGImage.cpp:237
class ChromeClient      - (its child class) svg/graphics/SVGImage.cpp:243

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44215 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 44215 [details]
Patch

I'm not sure this is right.

 69     class ChromeClient : public Noncopyable {

Why should these Abstract Base Classes require that their implementors be non-copyable?

[Darin Adler]

(In reply to comment #2)
> I'm not sure this is right.
> 
>  69     class ChromeClient : public Noncopyable {
> 
> Why should these Abstract Base Classes require that their implementors be
> non-copyable?

They should not. I think it’s better to not add any base classes like Noncopyable or FastAllocBase to these abstract base classes.

[Zoltan Horvath]

(In reply to comment #2)
> (From update of attachment 44215 [details])
> I'm not sure this is right.
> 
>  69     class ChromeClient : public Noncopyable {
> 
> Why should these Abstract Base Classes require that their implementors be
> non-copyable?

In this case I can inherit SVGImageChromeClient Noncopyable, then ChromeClient don't need to be inherited.

At the case of DragClient the instantiation line is:
static DragClient* dummyDragClient = new EmptyDragClient;

DragClient is abstract and EmptyDragClient is abstract as well.


At the case of ContextMenuClient:
static ContextMenuClient* dummyContextMenuClient = new EmptyContextMenuClient;

The problem is the same as at DragClient.

[Darin Adler]

(In reply to comment #4)
> At the case of DragClient the instantiation line is:
> static DragClient* dummyDragClient = new EmptyDragClient;
> 
> DragClient is abstract and EmptyDragClient is abstract as well.

That's incorrect. EmptyDragClient is not abstract -- if it was, the new would not compile.

> At the case of ContextMenuClient:
> static ContextMenuClient* dummyContextMenuClient = new EmptyContextMenuClient;
> 
> The problem is the same as at DragClient.

Same comment.

[Zoltan Horvath]

Created attachment 44670 [details]
Patch

[Zoltan Horvath]

You're right! Sorry, I was on the QtWebKit summit, so I can the updated patch now.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44670 [details] without any errors.

[Zoltan Horvath]

Comment on attachment 44670 [details]
Patch

Clearing flags on attachment: 44670

Committed r51989: <http://trac.webkit.org/changeset/51989>

[Zoltan Horvath]

All reviewed patches have been landed.  Closing bug.