Bug 31906 – Allow custom memory allocation control for classes of the rendering and storage directory in WebCore

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

[Zoltan Horvath]

Created attachment 43910 [details]
Patch

Reviewed by NOBODY (OOPS!).

Allow custom memory allocation control for classes of the rendering and storage directory in WebCore
https://bugs.webkit.org/show_bug.cgi?id=

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

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

class SQLTransactionClient      - storage/DatabaseThread.cpp:45
class SQLTransactionCoordinator - storage/DatabaseThread.cpp:46
class OriginUsageRecord         - storage/OriginQuotaManager.cpp:66
class DatabaseTracker           - storage/DatabaseTracker.cpp:62
class ScrollbarTheme            - (its child class) rendering/RenderScrollbarTheme.cpp:35
class RenderSelectionInfoBase   - (its child class) rendering/RenderView.cpp:310
class RenderOverflow            - rendering/RenderBox.cpp:2846
struct FillLayer                - css/CSSStyleSelector.cpp:197

Inherits the following classes from FastAllocBase because these are instantiated by 'new':

struct ShadowData               - rendering/style/ShadowData.cpp:35
class CounterContent            - css/CSSStyleSelector.cpp:4111

* platform/ScrollbarTheme.h:
* rendering/RenderOverflow.h:
* rendering/RenderSelectionInfo.h:
* rendering/style/CounterContent.h:
* rendering/style/FillLayer.h:
* rendering/style/ShadowData.h:
* storage/DatabaseTracker.h:
* storage/OriginUsageRecord.h:
* storage/SQLTransactionClient.h:
* storage/SQLTransactionCoordinator.h:

[Eric Seidel (no email)]

Comment on attachment 43910 [details]
Patch

Is ShadowData really copyable?  It seems the weak pointer would make it not.

[Zoltan Horvath]

There was a compile failed on MAC with Noncopyable ShadowData, compiler said that ShadowData is instantiated in an initialization list.

[Zoltan Horvath]

Comment on attachment 43910 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 51407)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,40 @@
> [phone]  Zoltan Horvath  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Allow custom memory allocation control for classes of the rendering and storage directory in WebCore
> +        https://bugs.webkit.org/show_bug.cgi?id=
> +
> +        Inherits the following classes from Noncopyable because these are instantiated
> +        by 'new' and no need to be copyable:
> +
> +        class/struct name               - instantiated at: WebCore/'location'
> +
> +        class SQLTransactionClient      - storage/DatabaseThread.cpp:45
> +        class SQLTransactionCoordinator - storage/DatabaseThread.cpp:46
> +        class OriginUsageRecord         - storage/OriginQuotaManager.cpp:66
> +        class DatabaseTracker           - storage/DatabaseTracker.cpp:62
> +        class ScrollbarTheme            - (its child class) rendering/RenderScrollbarTheme.cpp:35
> +        class RenderSelectionInfoBase   - (its child class) rendering/RenderView.cpp:310
> +        class RenderOverflow            - rendering/RenderBox.cpp:2846
> +        struct FillLayer                - css/CSSStyleSelector.cpp:197
> +
> +        Inherits the following classes from FastAllocBase because these are instantiated by 'new':
> +
> +        struct ShadowData               - rendering/style/ShadowData.cpp:35
> +        class CounterContent            - css/CSSStyleSelector.cpp:4111
> +
> +        * platform/ScrollbarTheme.h:
> +        * rendering/RenderOverflow.h:
> +        * rendering/RenderSelectionInfo.h:
> +        * rendering/style/CounterContent.h:
> +        * rendering/style/FillLayer.h:
> +        * rendering/style/ShadowData.h:
> +        * storage/DatabaseTracker.h:
> +        * storage/OriginUsageRecord.h:
> +        * storage/SQLTransactionClient.h:
> +        * storage/SQLTransactionCoordinator.h:
> +
>  2009-11-26  Søren Gjesse  <[e-mail]>
>  
>          Reviewed by Pavel Feldman.
> Index: WebCore/platform/ScrollbarTheme.h
> ===================================================================
> --- WebCore/platform/ScrollbarTheme.h	(revision 51407)
> +++ WebCore/platform/ScrollbarTheme.h	(working copy)
> @@ -36,7 +36,7 @@ class PlatformMouseEvent;
>  class Scrollbar;
>  class ScrollView;
>  
> -class ScrollbarTheme {
> +class ScrollbarTheme : public Noncopyable {
>  public:
>      virtual ~ScrollbarTheme() {};
>  
> Index: WebCore/rendering/RenderOverflow.h
> ===================================================================
> --- WebCore/rendering/RenderOverflow.h	(revision 51407)
> +++ WebCore/rendering/RenderOverflow.h	(working copy)
> @@ -37,7 +37,7 @@ namespace WebCore
>  // Examples of visual overflow are shadows, text stroke (and eventually outline and border-image).
>  
>  // This object is allocated only when some of these fields have non-default values in the owning box.
> -class RenderOverflow {
> +class RenderOverflow : public Noncopyable {
>  public:
>      RenderOverflow(const IntRect& defaultRect = IntRect()) 
>          : m_topLayoutOverflow(defaultRect.y())
> Index: WebCore/rendering/RenderSelectionInfo.h
> ===================================================================
> --- WebCore/rendering/RenderSelectionInfo.h	(revision 51407)
> +++ WebCore/rendering/RenderSelectionInfo.h	(working copy)
> @@ -30,7 +30,7 @@
>  
>  namespace WebCore {
>  
> -class RenderSelectionInfoBase {
> +class RenderSelectionInfoBase : public Noncopyable {
>  public:
>      RenderSelectionInfoBase()
>          : m_object(0)
> Index: WebCore/rendering/style/CounterContent.h
> ===================================================================
> --- WebCore/rendering/style/CounterContent.h	(revision 51407)
> +++ WebCore/rendering/style/CounterContent.h	(working copy)
> @@ -30,7 +30,7 @@
>  
>  namespace WebCore {
>  
> -class CounterContent {
> +class CounterContent : public FastAllocBase {
>  public:
>      CounterContent(const AtomicString& identifier, EListStyleType style, const AtomicString& separator)
>          : m_identifier(identifier)
> Index: WebCore/rendering/style/FillLayer.h
> ===================================================================
> --- WebCore/rendering/style/FillLayer.h	(revision 51407)
> +++ WebCore/rendering/style/FillLayer.h	(working copy)
> @@ -59,7 +59,7 @@ struct FillSize {
>      LengthSize size;
>  };
>  
> -struct FillLayer {
> +struct FillLayer : Noncopyable {
>  public:
>      FillLayer(EFillLayerType);
>      ~FillLayer();
> Index: WebCore/rendering/style/ShadowData.h
> ===================================================================
> --- WebCore/rendering/style/ShadowData.h	(revision 51407)
> +++ WebCore/rendering/style/ShadowData.h	(working copy)
> @@ -26,6 +26,7 @@
>  #define ShadowData_h
>  
>  #include "Color.h"
> +#include <wtf/FastAllocBase.h>
>  
>  namespace WebCore {
>  
> @@ -33,7 +34,7 @@ enum ShadowStyle { Normal, Inset };
>  
>  // This struct holds information about shadows for the text-shadow and box-shadow properties.
>  
> -struct ShadowData {
> +struct ShadowData : FastAllocBase {
>      ShadowData()
>          : x(0)
>          , y(0)
> Index: WebCore/storage/DatabaseTracker.h
> ===================================================================
> --- WebCore/storage/DatabaseTracker.h	(revision 51407)
> +++ WebCore/storage/DatabaseTracker.h	(working copy)
> @@ -55,7 +55,7 @@ struct SecurityOriginHash;
>  struct SecurityOriginTraits;
>  #endif // !PLATFORM(CHROMIUM)
>  
> -class DatabaseTracker {
> +class DatabaseTracker : public Noncopyable {
>  public:
>      static DatabaseTracker& tracker();
>  
> Index: WebCore/storage/OriginUsageRecord.h
> ===================================================================
> --- WebCore/storage/OriginUsageRecord.h	(revision 51407)
> +++ WebCore/storage/OriginUsageRecord.h	(working copy)
> @@ -40,7 +40,7 @@ namespace WebCore {
>  
>  // Objects of this class can be used from multiple threads with external synchronization.
>  // String arguments are also supposed to be deeply copied by the caller when necessary.
> -class OriginUsageRecord {
> +class OriginUsageRecord : public Noncopyable {
>  public:
>      OriginUsageRecord();
>  
> Index: WebCore/storage/SQLTransactionClient.h
> ===================================================================
> --- WebCore/storage/SQLTransactionClient.h	(revision 51407)
> +++ WebCore/storage/SQLTransactionClient.h	(working copy)
> @@ -31,13 +31,15 @@
>  #ifndef SQLTransactionClient_h
>  #define SQLTransactionClient_h
>  
> +#include <wtf/Noncopyable.h>
> +
>  namespace WebCore {
>  
>      class SQLTransaction;
>  
>      // A client to the SQLTransaction class. Allows SQLTransaction to notify interested
>      // parties that certain things have happened in a transaction.
> -    class SQLTransactionClient {
> +    class SQLTransactionClient : public Noncopyable {
>      public:
>          void didCommitTransaction(SQLTransaction*);
>          void didExecuteStatement(SQLTransaction*);
> Index: WebCore/storage/SQLTransactionCoordinator.h
> ===================================================================
> --- WebCore/storage/SQLTransactionCoordinator.h	(revision 51407)
> +++ WebCore/storage/SQLTransactionCoordinator.h	(working copy)
> @@ -42,7 +42,7 @@ namespace WebCore {
>  
>      class SQLTransaction;
>  
> -    class SQLTransactionCoordinator {
> +    class SQLTransactionCoordinator : public Noncopyable {
>      public:
>          void acquireLock(SQLTransaction*);
>          void releaseLock(SQLTransaction*);

[WebKit Commit Bot]

Comment on attachment 43910 [details]
Patch

Rejecting patch 43910 from commit-queue.

Failed to run "['WebKitTools/Scripts/build-webkit']" exit_code: 1
Last 500 characters of output:
CommitQueue/WebCore/svg/SVGSVGElement.cpp -o /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/SVGSVGElement.o
** BUILD FAILED **

The following build commands failed:
WebCore:
	Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/FillLayer.o /Users/eseidel/Projects/CommitQueue/WebCore/rendering/style/FillLayer.cpp normal i386 c++ com.apple.compilers.gcc.4_2
(1 failure)

[Zoltan Horvath]

Created attachment 43936 [details]
proposed patch

The problem with FillLayer is similar to the ShadowData problem. Unfortunately, my developer mac hardware failed, so I couldn't update the patch. Now i think it will be okay.

[Eric Seidel (no email)]

(In reply to comment #2)
> There was a compile failed on MAC with Noncopyable ShadowData, compiler said
> that ShadowData is instantiated in an initialization list.

Oh.  Well, that would be easy enough to fix using a constructor instead. :)  But OK.  In the future we could leave comments next to classes (FIXME's) where classes should be NonCopyable, but can't for reasons such as this.

[Eric Seidel (no email)]

Comment on attachment 43936 [details]
proposed patch

LGTM

[WebKit Commit Bot]

Comment on attachment 43936 [details]
proposed patch

Clearing flags on attachment: 43936

Committed r51466: <http://trac.webkit.org/changeset/51466>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.