GNOME Bugzilla – Bug 671249
GApplication: Allow a null application_id?
Last modified: 2012-05-28 13:35:36 UTC
It would be nice if GApplication did not demand a valid application_id until it actually needed it. This would allow, for instance, people to use gtk_application_new() and g_application_run() just as a better way to initialize GTK+ and then use it's quit-the-app-when-the-window-closes feature. At the moment, doing this requires us to think up some application ID that will never be used.
so basically, this would imply 'non-unique'. sounds like a pretty good idea, actually. it would also make subclassing gapplication easier.
(In reply to comment #1) > so basically, this would imply 'non-unique'. Yes, though I don't know what unique means when neither G_APPLICATION_HANDLES_OPEN or G_APPLICATION_IS_SERVICE are used. Apparently, when using G_APPLICATION_FLAGS_NONE, new instances are started, and kept alive, anyway. But right now I am only playing with our wrapper in gtkmm, and using just new(), add_window() and run(). (We have more involved examples working too.)
so this is slightly more complicated than i imagined... presumably these anonymous applications may want to put a menu into the shell. for that reason, we need to go onto dbus and register an object path for the action group and the menus. that object path has been assumed to be directly derived from the application id (x.y.z -> /x/y/z). that can't be the case if we have a NULL application id. we could say that we pick "org.gtk.AnonymousApplication" as the default ID, but that could have some unfortunate side-effects (ie: the shell would see all anonymous applications as if they were a single application). this will probably require changes to both glib and gtk...
Can't we just create some random ID and/or object path as a default, or discover some unused one with a common prefix? At the moment it feels like we are leaking an implementation detail (the use of D-Bus) into the API.
Created attachment 208906 [details] [review] GApplication: allow null application_id GApplication application ID is now permitted to be NULL for quick example applications. The application will implicitly have the 'non-unique' flag set at registration time if this is the case. Since we break the long-standing rule that the application object path can always be derived from the application ID, we now publish the object path (along with the session bus reference) as private qdata on the GApplication instance for Gtk+ to pick up for its own uses.
Created attachment 208907 [details] [review] GtkApplication: adjust to GApplication changes GApplication now makes the session bus and object path available as qdata on the application instance. Use that instead of trying to guess values for ourselves. This causes this version of Gtk to depend on git master GLib so bumping version dependency accordingly.
this change reduces the smallest permitted non-trivial GtkApplication to this: public class Application : Gtk.Application { protected override void activate () { new Gtk.ApplicationWindow (this).show_all (); } } int main (string[] args) { return new Application ().run (args); } which is nice.
Huge thanks for this.
Review of attachment 208906 [details] [review]: Looks like the documentation updates are missing.
Review of attachment 208906 [details] [review]: Whats the point in putting the session bus there ? Calling get_sync in gtk should pretty much have the same effect , no ?
i don't like that get_sync() is a blocking call that could theoretically fail...
Created attachment 208955 [details] [review] GApplication: allow null application_id With documentation changes, as requested.
Review of attachment 208955 [details] [review]: That is a good start. But I think the long description needs some updates as well - it is very much written from the 'uniqueness is the main feature' perspective, and says things like 'first you need to choose an application id'
(In reply to comment #12) > i don't like that get_sync() is a blocking call that could theoretically > fail... But it won't, since GApplication already called get_sync(). If this is a real concern, we should add api to GDBus that is guaranteed not to block, not do some backhanded object_data pseudo-api.
Bug 665561 for that. There is a slight concern that the call may fail the first time but succeed later for some reason and we don't want GApplication to be out of sync with itself as to whether or not it has the bus... I guess that's the main reason I like keeping track of it for ourselves.
Created attachment 213124 [details] [review] GApplication: add accessor for DBus information Provide public access to the GDBusConnect and object path that GApplication is using. Prevents others from having to guess these things for themselves based on the application ID.
Created attachment 213125 [details] [review] GApplication: allow null application_id GApplication application ID is now permitted to be NULL for quick example applications. The application will implicitly have the 'non-unique' flag set at registration time if this is the case. Since we break the long-standing rule that the application object path can always be derived from the application ID, we now publish the object path (along with the session bus reference) as private qdata on the GApplication instance for Gtk+ to pick up for its own uses.
Created attachment 213127 [details] [review] GtkApplication: fixes for NULL session bus We currently have a couple of cases where GtkApplication assumes that the session bus will be non-NULL causing critical error output or (in the case of trying to publish menus) an infinite loop. Three fixes: - if the session bus is NULL due to not having registered the GtkApplication yet then give a g_critical on the entry point to the menu setters instead of going into an infinite loop. Document this. - check for NULL session bus even when calling the menu setters at the right time in order to prevent the infinite loop for non-programer-error cases (ie: because we had trouble connecting to the session bus) - check for NULL session bus when publishing the X11 properties on the GtkApplicationWindow and skip publishing them if we're not on the bus
Created attachment 213128 [details] [review] GtkApplication: adjust to GApplication changes GApplication now makes the session bus and object path available as a public API on the application instance. Use that instead of trying to guess values for ourselves. This causes this version of Gtk+ to depend on GLib 2.32.2, so bumping version dependency accordingly.
Created attachment 213131 [details] [review] GtkApplication: fix for NULL application ID Deal with the possibility of a NULL application ID by updating our copied logic from GLib: use a path of /org/gtk/Application/anonymous in this case. This should go on the Gtk+ stable branch instead of the "adjust to GApplication changes" patch in order to avoid Gtk depending on a non-.0 GLib release. "fixes for NULL session bus" should go on both stable and master.
Review of attachment 213131 [details] [review]: This is for 3.4 ? Ok
Review of attachment 213124 [details] [review]: ::: gio/gapplication.c @@ +1152,3 @@ + * Returns: (transfer none): a #GDBusConnection, or %NULL + * + * Since: 2.32.2 2.34, please @@ +1184,3 @@ + * Returns: the object path, or %NULL + * + * Since: 2.32.2 Ditto ::: gio/gapplication.h @@ +107,3 @@ +GDBusConnection * g_application_get_dbus_connection (GApplication *application); +const gchar * g_application_get_dbus_object_path (GApplication *application); Need G_AVAILABLE_IN_2_34 decorations here
Review of attachment 213125 [details] [review]: ::: gio/gapplicationimpl-dbus.c @@ +392,3 @@ + g_object_set_data (G_OBJECT (application), "dbus-session-bus", impl->session_bus); + g_object_set_data (G_OBJECT (application), "dbus-object-path", impl->object_path); + This hunk is not needed anymore, right ?
Review of attachment 213127 [details] [review]: fine
Review of attachment 213128 [details] [review]: ::: configure.ac @@ +41,2 @@ # required versions of other packages +m4_define([glib_required_version], [2.32.2]) Make this 2.33.0
Review of attachment 213125 [details] [review]: ::: gio/gapplicationimpl-dbus.c @@ +392,3 @@ + g_object_set_data (G_OBJECT (application), "dbus-session-bus", impl->session_bus); + g_object_set_data (G_OBJECT (application), "dbus-object-path", impl->object_path); + Ya. Thought I deleted it -- not sure how it slipped back in.
Attachment 213127 [details] pushed as fa96610 - GtkApplication: fixes for NULL session bus Attachment 213131 [details] pushed as ab2ce66 - GtkApplication: fix for NULL application ID
Comment on attachment 213125 [details] [review] GApplication: allow null application_id Committed with fixes (qdata dropped). Attachment 213125 [details] pushed as 7e9306e - GApplication: allow null application_id
Okay. That's it for the stable branch. The rest for master.
Comment on attachment 213124 [details] [review] GApplication: add accessor for DBus information Attachment 213124 [details] pushed as eb5381b - GApplication: add accessor for DBus information Committed with decorators and modified annotations.
Attachment 213128 [details] pushed as af71917 - GtkApplication: adjust to GApplication changes Marking it as depending on 2.33.1, not .0 (since that's the version currently on git for GLib and the version I will be releasing shortly). We're done here.
So I may I document the application_id as being optional in gtk_application_new(), and similar, too?
Tricky question. I think we should do it, but only on master. There is a chance that the stable version of Gtk will be used with an older GLib for which it is not safe to have a null application ID. Patch welcome.
Created attachment 215117 [details] [review] 0001-gtk_application_new-Docs-application_id-may-now-be-n.patch > I think we should do it, but only on master. Yes, of course. > Patch welcome. Here it is.
I notice also that it complains about "" though NULL is OK. It's always a bit tedious when code distinguishes between NULL and "" unnecessarily.
Review of attachment 215117 [details] [review]: Thanks. Please commit.
(In reply to comment #36) > I notice also that it complains about "" though NULL is OK. It's always a bit > tedious when code distinguishes between NULL and "" unnecessarily. I very strongly disagree with that statement. NULL and "" are quite different things and we do no favour to anyone by allowing them to become conflated.