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:
Comment on attachment 43910 [details] Patch Is ShadowData really copyable? It seems the weak pointer would make it not.
There was a compile failed on MAC with Noncopyable ShadowData, compiler said that ShadowData is instantiated in an initialization list.
Comment on attachment 43910 [details] Patch > Index: WebCore/ChangeLog > =================================================================== > --- WebCore/ChangeLog (revision 51407) > +++ WebCore/ChangeLog (working copy) > @@ -1,3 +1,40 @@ > +2009-11-26 Zoltan Horvath <zoltan@webkit.org> > + > + 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 <sgjesse@chromium.org> > > 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*);
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)
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.
(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.
Comment on attachment 43936 [details] proposed patch LGTM
Comment on attachment 43936 [details] proposed patch Clearing flags on attachment: 43936 Committed r51466: <http://trac.webkit.org/changeset/51466>
All reviewed patches have been landed. Closing bug.