cleanup DrawingBufferChromium, correct comment
Created attachment 95794 [details] Patch
Ready for review
Comment on attachment 95794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95794&action=review > Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:46 > +class Canvas2DLayerChromium { }; I doubt this will compile. RefPtr requires certain methods.
Comment on attachment 95794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95794&action=review Patch looks great, but R- for the odd forward declaration and lack of verbosity in the ChangeLog. Thanks a bunch for cleaning this up. > Source/WebCore/ChangeLog:8 > + https://bugs.webkit.org/show_bug.cgi?id=61953 > + > + * platform/graphics/chromium/DrawingBufferChromium.cpp: please put a more verbose description here if what this patch does. > Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:46 > +class Canvas2DLayerChromium { }; To forward declare Canvas2DLayerChromium just do: class Canvas2DLayerChromium; I don't think you need to do this, however - do you? I think all the references to Canvas2DLayerChromium are guarded by #if USE(ACCELERATED_COMPOSITING), or at least they should be.
Comment on attachment 95794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95794&action=review >>> Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:46 >>> +class Canvas2DLayerChromium { }; >> >> I doubt this will compile. RefPtr requires certain methods. > > To forward declare Canvas2DLayerChromium just do: > > class Canvas2DLayerChromium; > > I don't think you need to do this, however - do you? I think all the references to Canvas2DLayerChromium are guarded by #if USE(ACCELERATED_COMPOSITING), or at least they should be. It wasn't a forward declare, it was a dirty attempt to avoid referring to USE(ACCELERATED_COMPOSITING) in DrawingBuffer.h. But, Ken's right, it won't compile since it doesn't have the RefPtr methods. I'll just use #if USE(ACCELERATED_COMPOSITING) in DrawingBuffer.h.
Created attachment 95804 [details] Patch
Comment on attachment 95804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95804&action=review > Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:83 > + , m_platformLayer(0) You don't need to 0-initialize RefPtr<>s, and in fact this'll stop compiling when we get stricter about RefPtr/PassRefPtr usage. Just leave the initialization out - the default c'tor for RefPtr does the right thing.
Comment on attachment 95794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95794&action=review >> Source/WebCore/ChangeLog:8 >> + * platform/graphics/chromium/DrawingBufferChromium.cpp: > > please put a more verbose description here if what this patch does. Done.
Created attachment 95823 [details] Patch
Comment on attachment 95804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95804&action=review >> Source/WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:83 >> + , m_platformLayer(0) > > You don't need to 0-initialize RefPtr<>s, and in fact this'll stop compiling when we get stricter about RefPtr/PassRefPtr usage. Just leave the initialization out - the default c'tor for RefPtr does the right thing. Done.
Comment on attachment 95823 [details] Patch Rock
Comment on attachment 95823 [details] Patch Clearing flags on attachment: 95823 Committed r87987: <http://trac.webkit.org/changeset/87987>
All reviewed patches have been landed. Closing bug.