RESOLVED FIXED 104113
[EFL][WK2] Context clients should unregister themselves when destroyed
https://bugs.webkit.org/show_bug.cgi?id=104113
Summary [EFL][WK2] Context clients should unregister themselves when destroyed
Chris Dumez
Reported 2012-12-05 04:14:52 PST
We get crashes after a Ewk_Context is destroyed because the corresponding client classes get destroyed but they don't unregister their callbacks. As a consequence, the client callbacks sometimes get called after the client objects have been destroyed and it causes crashes. We should make sure all our client classes unregister their callbacks in their class destructor.
Attachments
Patch (5.17 KB, patch)
2012-12-05 04:30 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2012-12-05 04:30:41 PST
Kenneth Rohde Christiansen
Comment 2 2012-12-05 04:33:17 PST
Comment on attachment 177726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > + m_context->initializeHistoryClient(0); hmm maybe we should rename to not be called initialize then, and use setHistoryClient
Chris Dumez
Comment 3 2012-12-05 05:00:24 PST
(In reply to comment #2) > (From update of attachment 177726 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > > > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > > + m_context->initializeHistoryClient(0); > > hmm maybe we should rename to not be called initialize then, and use setHistoryClient Well, the corresponding C API is WKContextSetHistoryClient() so I tend to agree with you. However, I believe this should be done in a separate patch and we will need to CC Andersca. For now, what matters is that we get the crashes fixed.
Chris Dumez
Comment 4 2012-12-06 04:57:44 PST
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 177726 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > > > > > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > > > + m_context->initializeHistoryClient(0); > > > > hmm maybe we should rename to not be called initialize then, and use setHistoryClient > > Well, the corresponding C API is WKContextSetHistoryClient() so I tend to agree with you. > > However, I believe this should be done in a separate patch and we will need to CC Andersca. > > For now, what matters is that we get the crashes fixed. Kenneth, do you mind if we land this patch like it is for now and propose the renaming afterwards? Or do you want to do the renaming beforehand?
Kenneth Rohde Christiansen
Comment 5 2012-12-06 05:04:59 PST
Comment on attachment 177726 [details] Patch no
WebKit Review Bot
Comment 6 2012-12-06 05:12:19 PST
Comment on attachment 177726 [details] Patch Clearing flags on attachment: 177726 Committed r136833: <http://trac.webkit.org/changeset/136833>
WebKit Review Bot
Comment 7 2012-12-06 05:12:24 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.