GNOME Bugzilla – Bug 622903
Migrate from dbus-glib to glib's GDBus
Last modified: 2011-12-15 10:10:49 UTC
For GLib 2.25.5 GDBus D-Bus support was merged, providing an API to replace dbus-glib. See http://library.gnome.org/devel/gio/unstable/gdbus.html and http://library.gnome.org/devel/gio/unstable/ch28.html . According to a quick grep this module seems to use dbus-glib: ./epiphany/configure.ac:PKG_CHECK_MODULES([DBUS],[dbus-glib-1 >= $DBUS_GLIB_REQUIRED])
Created attachment 189231 [details] [review] ephy-dbus: half-port to gdbus Work in progress. Works: - gets the session name - installs the org.gnome.Epiphany interface for our remote methods - calling remote methods work Doesn't work: - remote methods don't return until Epiphany quits Needs cleanup, of course. Bug #622903
Why do we need this at all, if we use GtkApplication we get this for granted.
*** Bug 646777 has been marked as a duplicate of this bug. ***
So my understanding is that behind this "move to GDBus" thing there are three sub-goals: - Unique app - Remote actions - The NetworkManager bits I thought that only the first one was sort of duplicated with GtkApplication, but apparently the two first are. So I guess it's better to just port over the NM stuff and leave the rest for the GtkApplication bug?
Indeed. The only bit that needs clarification, though, is regarding the exposure of the remote actions in the shell, since we don't want to expose all of them in the UI. So far I haven't found any info about this, should probably ask the shell guys.
Created attachment 190534 [details] [review] Use a gdbus-codegen generated GDBusProxy to monitor NM state Get rid of the EphyNetMonitor code and instead generate a GDBusProxy subclass that monitors the StateChanged signal and State property in org.gnome.NetworkManager. This proxy exports these features as standard GObject signal and property, respectively, so we can use it directly through the relevant accessors.
Review of attachment 190534 [details] [review]: Wow, amazing :)
Attachment 190534 [details] pushed as 3af15cd - Use a gdbus-codegen generated GDBusProxy to monitor NM state
Thanks for reviewing so quickly. I think that now we only need to land the GtkApplication patch and the dbus-glib code can be dropped.
Created attachment 190655 [details] [review] Drop all dbus-glib code We still need to provide equivalent ways to activate epiphany, but we will use GActions instead.
Created attachment 190656 [details] [review] Ensure startup_context is not NULL before freeing it
Created attachment 190657 [details] [review] Mark string parameters in ephy_session_queue_command() as const ephy_session_queue_command() doesn't take over the strings it receives, so mark these as const for clarity.
Created attachment 190658 [details] [review] Bring back dbus activation in the form of GSimpleActions. This adds some actions that can be activated through org.gtk.Actions.Activate calls on /org/gnome/Epiphany. The interface is of course different than the previously exposed through DBus, since it is now defined by GTK+.
I am not 100% sure that we want/need the last patch; anyone wanting to activate epiphany can use the command-line and the effect will be the same. otoh, please note that the open-uri-list opens everything in new windows, as there is no way to specify otherwise, but this can be added if we really want this.
Review of attachment 190655 [details] [review]: Ok.
Review of attachment 190656 [details] [review]: Ok.
Review of attachment 190657 [details] [review]: Ok.
Attachment 190655 [details] pushed as d40bac5 - Drop all dbus-glib code Attachment 190656 [details] pushed as 74d64e0 - Ensure startup_context is not NULL before freeing it Attachment 190657 [details] pushed as 016b699 - Mark string parameters in ephy_session_queue_command() as const
Created attachment 191289 [details] [review] ephy_history_service_get_url_row: don't leak the url strings
Created attachment 191290 [details] [review] EphyHistoryService: don't leak the history-filename string property
Created attachment 191291 [details] [review] tests/ephy-history: plug a couple of leaks
Created attachment 191292 [details] [review] EphyHistoryService: fix the disposal of EphyHistoryJobMethod data The GDestroyNotify for the argument data was not even stored in the job details, leaking the argument data for every single job.
Created attachment 191293 [details] [review] ephy_history_service_execute_set_url_title: don't leak the title
Created attachment 191294 [details] [review] Add tests for EphyHistoryService::set_url_title()
Created attachment 191295 [details] [review] Add tests for EphyHistoryService::get_url()
I think all this was done.