We can simply use g_cclosure_marshal_generic instead.
Created attachment 312293 [details] Patch
Comment on attachment 312293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312293&action=review Informal r+ from me. There is a chance that the generic libffi-based marshaller is slower than the custom ones, but it should not be much of an issue because other parts of the WebKit code (e.g. rendering) have more weight on CPU usage. And it is nice to have code getting removed \o/ > Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp:87 > + g_cclosure_marshal_generic, You could as well pass “nullptr” here. According to the documentation for “g_signal_new()” (https://developer.gnome.org/gobject/stable/gobject-Signals.html#g-signal-new): > If c_marshaller is NULL, g_cclosure_marshal_generic() will be used as the > marshaller for this signal. And the same goes to the other changed locations. I have no strong opinion about whether passing “g_cclosure_marshal_generic” or “nullptr” should be preferred, so I'd leave it up to you.
(In reply to Adrian Perez from comment #2) > Comment on attachment 312293 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312293&action=review > > Informal r+ from me. Thanks. > There is a chance that the generic libffi-based marshaller is slower than the > custom ones, but it should not be much of an issue because other parts of the > WebKit code (e.g. rendering) have more weight on CPU usage. And it is nice to > have code getting removed \o/ Yes, I don't think it will be noticeable and we are using generic already in other signals. > > Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp:87 > > + g_cclosure_marshal_generic, > > You could as well pass “nullptr” here. According to the documentation for > “g_signal_new()” > (https://developer.gnome.org/gobject/stable/gobject-Signals.html#g-signal- > new): > > > If c_marshaller is NULL, g_cclosure_marshal_generic() will be used as the > > marshaller for this signal. > > And the same goes to the other changed locations. I have no strong opinion > about whether > passing “g_cclosure_marshal_generic” or “nullptr” should be preferred, so > I'd leave it > up to you. g_signal_new is already difficult to read with severl 0, nullptr, nullptr. In some cases we even add a comment explaining what the nullptr/0 is. So, I prefer to pass the marshaller for readability.
Comment on attachment 312293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312293&action=review >>> Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp:87 >>> + g_cclosure_marshal_generic, >> >> You could as well pass “nullptr” here. According to the documentation for “g_signal_new()” >> (https://developer.gnome.org/gobject/stable/gobject-Signals.html#g-signal-new): > > g_signal_new is already difficult to read with severl 0, nullptr, nullptr. In some cases we even add a comment explaining what the nullptr/0 is. So, I prefer to pass the marshaller for readability. SGTM. Let's be explicit where possible.
Good cleanup. (In reply to Carlos Garcia Campos from comment #3) > g_signal_new is already difficult to read with severl 0, nullptr, nullptr. > In some cases we even add a comment explaining what the nullptr/0 is. So, I > prefer to pass the marshaller for readability. Well it's up to you, but I prefer the NULL/nullptr style as well. It's simpler than writing out g_cclosure_marshal_generic, and I don't think it hurts readability. Here nullptr means "keep things as the default" and that's all you need to know. I converted Epiphany to use this style a year or two ago.
Committed r217960: <http://trac.webkit.org/changeset/217960>