Summary: | [WK2][GTK] Frequent crashes when showing context menus in Debug builds | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||
Component: | WebKitGTK | Assignee: | Sergio Villar Senin <svillar> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cgarcia, gustavo, mrobinson, svillar, xan.lopez | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Sergio Villar Senin
2013-09-10 09:48:12 PDT
Created attachment 211208 [details]
Patch
Comment on attachment 211208 [details]
Patch
I wonder if we could use g_signal_connect_object and we don't need to keep a map of signal handlers.
(In reply to comment #2) > (From update of attachment 211208 [details]) > I wonder if we could use g_signal_connect_object and we don't need to keep a map of signal handlers. You mean using the GtkMenu as the object? Yeah I guess that could work as well... (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 211208 [details] [details]) > > I wonder if we could use g_signal_connect_object and we don't need to keep a map of signal handlers. > > You mean using the GtkMenu as the object? Yeah I guess that could work as well... I meant for the GtkAction, but doesn't make sense since it's the menu what is destroyed. Comment on attachment 211208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211208&action=review LGTM > Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:174 > + for (HashMap<unsigned long, GtkAction*>::const_iterator iter = m_signalHandlers.begin(); iter != m_signalHandlers.end(); ++iter) > + g_signal_handler_disconnect(iter->value, iter->key); I'm not C++ expert, but now that we are using C++ 11 features I wonder if we could use auto here. (In reply to comment #5) > (From update of attachment 211208 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211208&action=review > > LGTM > > > Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.cpp:174 > > + for (HashMap<unsigned long, GtkAction*>::const_iterator iter = m_signalHandlers.begin(); iter != m_signalHandlers.end(); ++iter) > > + g_signal_handler_disconnect(iter->value, iter->key); > > I'm not C++ expert, but now that we are using C++ 11 features I wonder if we could use auto here. I think you're right. See Source/JavaScriptCore/runtime/MapData.cpp for instance. Committed r155459: <http://trac.webkit.org/changeset/155459> |