Bug 32106 - Allow custom memory allocation control for the other part of page directory in WebCore
Summary: Allow custom memory allocation control for the other part of page directory i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-03 01:18 PST by Zoltan Horvath
Modified: 2009-12-11 08:18 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.79 KB, patch)
2009-12-03 01:18 PST, Zoltan Horvath
no flags Details | Formatted Diff | Diff
Patch (2.32 KB, patch)
2009-12-11 04:41 PST, Zoltan Horvath
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 2009-12-03 01:18:16 PST
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
Comment 1 WebKit Review Bot 2009-12-03 01:19:10 PST
style-queue ran check-webkit-style on attachment 44215 [details] without any errors.
Comment 2 Eric Seidel (no email) 2009-12-03 13:01:30 PST
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?
Comment 3 Darin Adler 2009-12-03 14:03:36 PST
(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.
Comment 4 Zoltan Horvath 2009-12-04 05:44:40 PST
(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.
Comment 5 Darin Adler 2009-12-04 09:17:28 PST
(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.
Comment 6 Zoltan Horvath 2009-12-11 04:41:28 PST
Created attachment 44670 [details]
Patch
Comment 7 Zoltan Horvath 2009-12-11 04:43:34 PST
You're right! Sorry, I was on the QtWebKit summit, so I can the updated patch now.
Comment 8 WebKit Review Bot 2009-12-11 04:44:59 PST
style-queue ran check-webkit-style on attachment 44670 [details] without any errors.
Comment 9 Zoltan Horvath 2009-12-11 08:18:17 PST
Comment on attachment 44670 [details]
Patch

Clearing flags on attachment: 44670

Committed r51989: <http://trac.webkit.org/changeset/51989>
Comment 10 Zoltan Horvath 2009-12-11 08:18:27 PST
All reviewed patches have been landed.  Closing bug.