RESOLVED FIXED46139
[chromium] LayerChromium and its derived types should use virtual destructors 
https://bugs.webkit.org/show_bug.cgi?id=46139
Summary [chromium] LayerChromium and its derived types should use virtual destructors
Vangelis Kokkevis
Reported 2010-09-20 17:02:26 PDT
Currently LayerChromium doesn't define it destructor as virtual which means that destructors for derived types don't execute when layers get removed from the layer tree.
Attachments
proposed patch (1.79 KB, patch)
2010-09-20 18:29 PDT, Vangelis Kokkevis 		jamesr: review+
DetailsFormatted DiffDiff
Proposed patch - Does proper ref counting for the compositor (11.31 KB, patch)
2010-09-27 15:26 PDT, Vangelis Kokkevis 		no flags
DetailsFormatted DiffDiff
proposed patch - fixed style issue (11.30 KB, patch)
2010-09-27 15:33 PDT, Vangelis Kokkevis 		jamesr: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Vangelis Kokkevis
Comment 1 2010-09-20 18:29:14 PDT
Created attachment 68170 [details] proposed patch
James Robinson
Comment 2 2010-09-20 18:32:21 PDT
Comment on attachment 68170 [details] proposed patch R=me Technically the virtual keyword on ContentLayerChromium is redundant, but I prefer to have it for clarity.
Vangelis Kokkevis
Comment 3 2010-09-20 18:36:17 PDT
Committed r67906: <http://trac.webkit.org/changeset/67906>
Vangelis Kokkevis
Comment 4 2010-09-20 19:13:16 PDT
Reverted r67906 for reason: Change causes chromium to crash when switching pages Committed r67912: <http://trac.webkit.org/changeset/67912>
Vangelis Kokkevis
Comment 5 2010-09-27 15:26:34 PDT
Created attachment 68973 [details] Proposed patch - Does proper ref counting for the compositor Corrected the reason why the previous patch was causing crashes which was that LayerChromium destructors could end up getting called after the layer compositor and its associated gl context were destroyed. This patch makes the LayerRendererChromium a ref counted class and the LayerChromium's used by it are now holding a reference to LayerRendererChromium they are used with. This ensures that the gl context used by the layers will outlive the layers themselves.
WebKit Review Bot
Comment 6 2010-09-27 15:28:47 PDT
Attachment 68973 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:138: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Vangelis Kokkevis
Comment 7 2010-09-27 15:33:37 PDT
Created attachment 68975 [details] proposed patch - fixed style issue
James Robinson
Comment 8 2010-09-27 15:52:24 PDT
Comment on attachment 68975 [details] proposed patch - fixed style issue Looks good!
Vangelis Kokkevis
Comment 9 2010-09-27 16:36:14 PDT
Committed r68442: <http://trac.webkit.org/changeset/68442>
Note You need to log in before you can comment on or make changes to this bug.