User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

As of the 2021-03-03-09-41-10-mozilla-central night build, Firefox fails to start with

/tmp $ ./firefox/firefox -no-remote -profile /tmp/ff/
./firefox/firefox: symbol lookup error: /tmp/firefox/libxul.so: undefined symbol: gdk_wayland_display_get_type

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b2c9624b48f0dc756adcba7cd43a941f349720f6&tochange=478d2e18554dfcdde9876073614a2c4973cb6e1c

Possibly Bug 1622107 introducing a call to gfxPlatformGtk::GetPlatform()->IsWaylandDisplay()

This is running on Gentoo Linux with gtk+-3.24.24 build with wayland support disabled.

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::General' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Can you use mozregression tool to find exact match [1]?

I see the GDK_IS_WAYLAND_DISPLAY() macro only here:
https://searchfox.org/mozilla-central/rev/002023eb262be9db3479142355e1675645d52d52/widget/GfxInfoX11.cpp#465

Thanks.

[1] https://fedoraproject.org/wiki/How_to_debug_Firefox_problems?rd=Bug_info_Firefox#Use_Mozregression_tool

In theory, you should be able to reproduce if you run firefox in X11 session in gdb and break on gdk_wayland_display_get_type, the expectation being that when not running on wayland, it shouldn't be called.

There's a case to be made whether Firefox should work with LD_BINDNOW=1 on a system with gtk without wayland support... (it currently doesn't)

5:54.81 INFO: Narrowed integration regression window from [66ff1258, 49a61253] (4 builds) to [d4a0013b, 49a61253] (2 builds) (~1 steps left)
5:54.81 INFO: No more integration revisions, bisection finished.
5:54.81 INFO: Last good revision: d4a0013b717fdd3818eb8d3a2783976c56b30a57
5:54.81 INFO: First bad revision: 49a6125392f5d6ce92443676d6297c46980d4a29
5:54.81 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d4a0013b717fdd3818eb8d3a2783976c56b30a57&tochange=49a6125392f5d6ce92443676d6297c46980d4a29

The only commit in this range is from Bug 1695453, so maybe this is causing Firefox to incorrectly enable a wayland session.

https://hg.mozilla.org/integration/autoland/rev/49a6125392f5d6ce92443676d6297c46980d4a29#l7.12

This does swap !GDK_IS_X11_DISPLAY for GDK_IS_WAYLAND_DISPLAY.

Seems like GfxInfoX11 and nsAppRunner have overlaping, but unique criteria for what constitutes a wayland session. Maybe both should validate the environment before interrogating the toolkit?

Using GDK_IS_WAYLAND_DISPLAY is arguably more correct, but seems to rely on gtk+ being built with wayland support. GDK_IS_X11_DISPLAY works for now, but I expect at some point gtk+ build with X disable will also start to show up.

(In reply to Matthew Turnbull [Bluefang] from comment #5)

https://hg.mozilla.org/integration/autoland/rev/49a6125392f5d6ce92443676d6297c46980d4a29#l7.12

This does swap !GDK_IS_X11_DISPLAY for GDK_IS_WAYLAND_DISPLAY.

Yes, we should use !GDK_IS_X11_DISPLAY instead of GDK_IS_WAYLAND_DISPLAY. Robert, do you mind to fix that?
Thanks.

(In reply to Martin Stránský [:stransky] from comment #6)

(In reply to Matthew Turnbull [Bluefang] from comment #5)

https://hg.mozilla.org/integration/autoland/rev/49a6125392f5d6ce92443676d6297c46980d4a29#l7.12

This does swap !GDK_IS_X11_DISPLAY for GDK_IS_WAYLAND_DISPLAY.

Yes, we should use !GDK_IS_X11_DISPLAY instead of GDK_IS_WAYLAND_DISPLAY. Robert, do you mind to fix that?
Thanks.

Using !GDK_IS_X11_DISPLAY prevents us from building without X11 support eventually. The part in question is wrapped in #ifdef MOZ_WAYLAND so Matthew, apparently you build with MOZ_WAYLAND enabled. Could you double check that?

(In reply to Robert Mader [:rmader] from comment #7)

Using !GDK_IS_X11_DISPLAY prevents us from building without X11 support eventually. The part in question is wrapped in #ifdef MOZ_WAYLAND so Matthew, apparently you build with MOZ_WAYLAND enabled. Could you double check that?

I am not building Firefox my self. I'm using what ever is provided by Mozilla's nightly build channel (i.e. http://archive.mozilla.org/pub/firefox/nightly/2021/03/).

Ah ok, sorry, now I understand issue. I'll revert that part, but I guess we need another solution in the future as using !GDK_IS_X11_DISPLAY breaks things for people building GTK or FF without X11 support.

IIUC we don't specify the required GTK backends needed ATM, so the X11 backend is probably assumed. Ideally we should support all possible setups (X11 only/Wayland only/both available) - but favoring X11 or Wayland is getting odd these days.

Yeah, that all make sense, which is why I was wondering if it made sense to add some environment validation (i.e. WAYLAND_DISPLAY / MOZ_ENABLE_WAYLAND ) before checking gdk (assuming symbol look-ups happen at instruction execution and not linking) or by providing a stub for gdk_wayland_display_get_type when it's not available from gtk+ at runtime.

Unfortunately this is getting into areas of c[++] that I have little knowledge of, but I would definitely support a solution that keeps using GDK_IS_WAYLAND_DISPLAY as long as it doesn't crash when wayland support isn't available. But switching back to !GDK_IS_X11_DISPLAY might be the expedient short-term solution.

We can use dlsym() to read gdk_wayland_display_get_type() / gdk_x11_display_get_type() directly and avoid GDK_IS_X11_DISPLAY/GDK_IS_WAYLAND_DISPLAY macros.

In D106726 this was changed to GDK_IS_WAYLAND_DISPLAY as a drive-by
cleanup, in order to eventually support compiling FF without X11 support.
Turns out that was misguided as it breaks compatability with
setups that have GTK compiled without Wayland support.

Stick to GDK_IS_X11_DISPLAY until we have a better solution.

https://hg.mozilla.org/mozilla-central/rev/95b6c826ad97

(In reply to Martin Stránský [:stransky] from comment #11)

We can use dlsym() to read gdk_wayland_display_get_type() / gdk_x11_display_get_type() directly and avoid GDK_IS_X11_DISPLAY/GDK_IS_WAYLAND_DISPLAY macros.

I guess we can then do something like:

bool GdkIsWaylandDisplay(GdkDisplay * display) {
    static auto sGdkWaylandDisplayGetType =
        (GType(*)())dlsym(RTLD_DEFAULT, "gdk_wayland_display_get_type");
    GType type = sGdkWaylandDisplayGetType();
    return type != 0 && G_TYPE_CHECK_INSTANCE_TYPE(display, type);
}

bool GdkIsX11Display(GdkDisplay * display) {
    static auto sGdkX11DisplayGetType =
        (GType(*)())dlsym(RTLD_DEFAULT, "gdk_x11_display_get_type");
    GType type = sGdkX11DisplayGetType();
    return type != 0 && G_TYPE_CHECK_INSTANCE_TYPE(display, type);
}

(In reply to Jan Alexander Steffens [:heftig] from comment #15)

(In reply to Martin Stránský [:stransky] from comment #11)

We can use dlsym() to read gdk_wayland_display_get_type() / gdk_x11_display_get_type() directly and avoid GDK_IS_X11_DISPLAY/GDK_IS_WAYLAND_DISPLAY macros.

I guess we can then do something like:

bool GdkIsWaylandDisplay(GdkDisplay * display) {
    static auto sGdkWaylandDisplayGetType =
        (GType(*)())dlsym(RTLD_DEFAULT, "gdk_wayland_display_get_type");
    GType type = sGdkWaylandDisplayGetType();
    return type != 0 && G_TYPE_CHECK_INSTANCE_TYPE(display, type);
}

bool GdkIsX11Display(GdkDisplay * display) {
    static auto sGdkX11DisplayGetType =
        (GType(*)())dlsym(RTLD_DEFAULT, "gdk_x11_display_get_type");
    GType type = sGdkX11DisplayGetType();
    return type != 0 && G_TYPE_CHECK_INSTANCE_TYPE(display, type);
}

You're right Jan. If you feel so please go ahead and do the patch.
Thanks.

Should I just reopen this bug for that?

I'm not sure where to put those functions. They would be needed outside of widget/gtk.

Set release status flags based on info from the regressing bug 1695453