GNOME Bugzilla – Bug 694465
Allow backends to fail during initialisation
Last modified: 2018-04-15 00:30:29 UTC
Check if the GdkDisplayManager implementation also implements GInitable and use it to determine if the backend is initialised successfully. If a backend fails, then attempt to load the next available backend. This allows backends to fail gracefully and for GDK to try another backend if possible.
Created attachment 237200 [details] [review] gdk: check if backends implement GInitable Check if the GdkDisplayManager implementation also implements GInitable and use it to determine if the backend is initialised successfully. If a backend fails, then attempt to load the next available backend. Since backends may need to read any relevant options (such as the display flag) to determine if they can be created successfully, this patch also removes calls that attempt to create the display manager before the options have been parsed.
Created attachment 237201 [details] [review] wayland: implement GInitable and check the connection to the display server Check that a connection to the default display server is possible during initialisation and fail the initialisation if it is not possible. This commit also allows GDK to prefer the Wayland backend over X11, but fall back to X11 if Wayland is unavailable.
Review of attachment 237200 [details] [review]: ::: gdk/gdkdisplaymanager.c @@ +215,3 @@ + + if (g_type_is_a (backend_type, G_TYPE_INITABLE)) + manager = g_initable_new (backend_type, NULL, NULL, NULL); Currently, any error message from the backend is ignored, since the next backend may be initialised successfully. Perhaps any error messages should be stored and printed later if none of the backends could be initialised.
If we think that initable backends are a good idea, we should just make all backends implement GInitable, instead of checking in the frontend.
I think a two phase approach would be good here. Initially lets check for GInitable and use it if is present whilst encouraging the backends to start implementing the interface. Once they've all moved over to that then we can remove the check and make implementing GInitable a mandatory part of implementing a backend.
It might be possible to add a default implementation of GInitable to GdkDisplayManager which would simply return TRUE. This would then be overridden in the backend implementations.
Created attachment 237623 [details] [review] Add a default implementation of GInitable on GdkDisplayManager, that always succeeds initialisation.
Created attachment 237624 [details] [review] Updated patch for the Wayland backend to add GInitable implementation
Review of attachment 237624 [details] [review]: ::: gdk/gdkdisplaymanager.c @@ +267,3 @@ + if (!manager && (backend == NULL || strcmp (backend, "x11") == 0)) + manager = g_initable_new (gdk_x11_display_manager_get_type (), NULL, NULL, NULL); +#endif Does this reordering make wayland the preferred backend ? I don't think thats right...
Review of attachment 237623 [details] [review]: This looks reasonable to me, if Benjamin is ok with it
(In reply to comment #9) > Review of attachment 237624 [details] [review]: > > ::: gdk/gdkdisplaymanager.c > @@ +267,3 @@ > + if (!manager && (backend == NULL || strcmp (backend, "x11") == 0)) > + manager = g_initable_new (gdk_x11_display_manager_get_type (), NULL, > NULL, NULL); > +#endif > > Does this reordering make wayland the preferred backend ? I don't think thats > right... Yes (as mentioned in the commit message). The reason for giving Wayland a higher priority is that if XWayland is being used, the user would presumably prefer applications to use Wayland if possible, even though an X server is available.
So if I'm playing around with XWayland and then double-click a PDF in Nautilus, that evince instance goes missing inside XWayland?
Pretty sure this work caused https://bugzilla.gnome.org/show_bug.cgi?id=696457
I've reverted this on my quest to only one GdkDisplayManager class. Reopening this so we're sure stuff works after I'm done with refactoring.
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new