Summary: | Allow custom memory allocation control for classes of the rendering and storage directory in WebCore | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Horvath <zoltan> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Zoltan Horvath
2009-11-26 01:50:00 PST
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. |