Summary: | [Efl][WebGL] Add better support to track and free XResources. | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kalyan <kalyan.kondapally> | ||||||||||||||||||||
Component: | WebKit EFL | Assignee: | Kalyan <kalyan.kondapally> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | kenneth, lucas.de.marchi, noam, webkit.review.bot, zeno | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | PC | ||||||||||||||||||||||
OS: | Linux | ||||||||||||||||||||||
Attachments: |
|
Description
Kalyan
2013-01-20 04:34:31 PST
Created attachment 183669 [details]
patch
Created attachment 183674 [details]
patchv2
removed usused member varibles in X11WindowResources
Comment on attachment 183674 [details] patchv2 View in context: https://bugs.webkit.org/attachment.cgi?id=183674&action=review > Source/WebCore/ChangeLog:8 > + Covered by existing WebGL tests. This is probably true but WebGL tests are not enabled for EFL on the bot so I think it is about time to hold on with re-factoring like this and enable the WebGL tests first. (In reply to comment #3) > (From update of attachment 183674 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183674&action=review > > > Source/WebCore/ChangeLog:8 > > + Covered by existing WebGL tests. > > This is probably true but WebGL tests are not enabled for EFL on the bot so I think it is about time to hold on with re-factoring like this and enable the WebGL tests first. I do agree with you but I don't see this as refactoring but rather part of effort in fixing things to enable the layout tests in our port. I was atleast having constant WebProcess crashes(doing double deletions sometimes) without these changes. (In reply to comment #3) > (From update of attachment 183674 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183674&action=review > > > Source/WebCore/ChangeLog:8 > > + Covered by existing WebGL tests. > > This is probably true but WebGL tests are not enabled for EFL on the bot so I think it is about time to hold on with re-factoring like this and enable the WebGL tests first. On side note we need 106878,107397,107178 fixed before trying to enable WebGL tests (atleast ones under fast/canvas/webgl). Atleast, I needed these before I could run the tests without any random crashes. I will create a separate issue to enable to the tests. Created attachment 183703 [details]
patchv3
updated the changelog
Created attachment 183837 [details]
patchv4
rebased patch. Not marking it for review yet. Layout tests with latest source seems stable even without this patch and don't see any de-allocation issues. The patch only fixes memory leaks and thus could be considered after enabling them.
Comment on attachment 183837 [details] patchv4 View in context: https://bugs.webkit.org/attachment.cgi?id=183837&action=review > Source/WebCore/platform/graphics/surfaces/glx/X11WindowResources.h:49 > +struct ScopedXResource { This is not needed. You can have OwnPtr with a specialized template, like GTK/Cairo does. Created attachment 184862 [details]
patchv5
Changed name of ScopedXResource to ScopedPtrX11 and moved it to a separate header. Cannot use OwnPtr here with GLXFBConfig, as it is a pointer.
i.e it is defined as typedef struct __GLXFBConfigRec *GLXFBConfig. One way would have been to handle this case separately but as we would need this change (ScopedPtrX11) for it, so I let it manage all the X Resources.
Created attachment 185079 [details]
patchv6
Comment on attachment 185079 [details] patchv6 View in context: https://bugs.webkit.org/attachment.cgi?id=185079&action=review > Source/WebCore/ChangeLog:8 > + We leak Memory during config selection as we dont free temp temp -> temporary > Source/WebCore/ChangeLog:35 > + Calls XFree on pointer to memory allocated by X when the class goes out of scope. Add some short explanation on why this was needed rather than using OwnPtr. Created attachment 185694 [details]
patchforlanding
Created attachment 185698 [details]
patchforlanding
Comment on attachment 185698 [details] patchforlanding Attachment 185698 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/16264129 Created attachment 185731 [details]
Patch
Comment on attachment 185731 [details] Patch Clearing flags on attachment: 185731 Committed r141404: <http://trac.webkit.org/changeset/141404> All reviewed patches have been landed. Closing bug. |