Clean up resources when LayerRenderer changes
Created attachment 69545 [details] Patch
Comment on attachment 69545 [details] Patch Looks fine although it will collide with : https://bugs.webkit.org/show_bug.cgi?id=46765 Should we merge the two into a single checkin?
I'd let her checkin first and I'll merge later.
Comment on attachment 69545 [details] Patch This is copy-paste from ContentLayerChromium so we should move this logic up to the base class. Specifically, add a virtual cleanupResources() function to LayerChromium and have LayerChromium::setLayerRenderer() invoke it.
Created attachment 69875 [details] Patch
(In reply to comment #3) > I'd let her checkin first and I'll merge later. Alpha actually already committed his linked patch (http://trac.webkit.org/changeset/69163), so my patch here has been modified (slightly) to fit with the current code. (In reply to comment #4) > (From update of attachment 69545 [details]) > This is copy-paste from ContentLayerChromium so we should move this logic up to the base class. Specifically, add a virtual cleanupResources() function to LayerChromium and have LayerChromium::setLayerRenderer() invoke it. Thanks for the tip! I added this and updated the related files. Take another look?
Comment on attachment 69875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69875&action=review R=me. Please kill the extra ; in the header - you can either do that while landing, or upload another patch and I'll r+/cq+ > WebCore/platform/graphics/chromium/LayerChromium.h:208 > + virtual void cleanupResources() { }; no semicolon needed here
(In reply to comment #7) > > WebCore/platform/graphics/chromium/LayerChromium.h:208 > > + virtual void cleanupResources() { }; > > no semicolon needed here Removed. Thanks so much for reviewing, James! Committed by Alpha here: http://trac.webkit.org/changeset/69166
Comment on attachment 69875 [details] Patch Sorry I'm late to this. I like the idea of making cleanupResources a virtual. How about calling it from the LayerChromium destructor as well instead of having to call it from each derived class destructor?