1696498 - Startup notification is not properly sent on GNU/Linux with GTK <= 3.24.7

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Oleg Strikov]

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

Steps to reproduce:

Run one of the GNU/Linux distributions which uses GTK <= 3.24.7 (Debian 9 LTS, Debian 10, Ubuntu 18.04 LTS) with MATE/Xorg. Start Firefox browser (Trunk or 78ESR) from the applications menu.

Actual results:

Observe that the mouse pointer changed its form to "application is loading" spinner. This spinner keeps spinning even after main window of the browser gets fully loaded and displayed. Please note that spinner is observable only when mouse pointer is outside of the main browser window.

Expected results:

Spinner should be displayed for a small period of time (about a second on a regular machine) until main browser window is shown. Mouse pointer should change its form back to the regular arrow afterwards.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Oleg Strikov]

After closing Bug 726479 ("Change startup notification code to use GTK APIs") Firefox relies on GTK to send startup notification to the environment. Unfortunately, this approach doesn't properly work with GTK <= 3.24.7 (please find details below). Display server's "application is loading" spinner keeps spinning even after main window of the browser gets fully loaded and displayed. I observe the issue with MATE/Xorg but I believe that it impacts other DEs and display servers as well. At least three currently supported GNU/Linux distributions (Debian 9 in LTS mode, Debian 10, Ubuntu 18.04 LTS) rely on GTK <= 3.24.7 which makes the scope of this bug reasonably large. Bug can be reproduced with both 78esr and trunk.

Please note that the code and links below are for GTK 3.24.7 which is the last version impacted. Future versions of GTK changed the code significantly (see https://gitlab.gnome.org/GNOME/gtk/-/commit/7ce25293b12463142a777b457ce81e3fd9e1342e for details) to fix similar issue directly in GTK/GDK code.

Here is the procedure from GDK which reads DESKTOP_STARTUP_ID environment variable which is set by DE and contains an ID which needs to be included into the generated startup notification to help display server to distinguish startup notifications from different applications (see https://github.com/GNOME/gtk/blob/d9a382d689b9159daca09e33510ea229a17d37be/gdk/x11/gdkdisplay-x11.c#L2223 for details):

static void
gdk_x11_display_make_default (GdkDisplay *display)
{
  GdkX11Display *display_x11 = GDK_X11_DISPLAY (display);
  const gchar *startup_id;

  g_free (display_x11->startup_notification_id);
  display_x11->startup_notification_id = NULL;

  startup_id = g_getenv ("DESKTOP_STARTUP_ID");
  if (startup_id && *startup_id != '\0')
    {
      if (!g_utf8_validate (startup_id, -1, NULL))
        g_warning ("DESKTOP_STARTUP_ID contains invalid UTF-8");
      else
        gdk_x11_display_set_startup_notification_id (display, startup_id);

      /* Clear the environment variable so it won't be inherited by
       * child processes and confuse things.
       */
      g_unsetenv ("DESKTOP_STARTUP_ID");
    }
}

The issue with this code is that content of DESKTOP_STARTUP_ID gets lost if this procedure is called two or more times before generating startup notification. After reading DESKTOP_STARTUP_ID and setting display_x11->startup_notification_id for a first time DESKTOP_STARTUP_ID environment variable is unset. During next run of the procedure content of display_x11->startup_notification_id gets dropped but can't be read from the environment again because DESKTOP_STARTUP_ID has been unset during the first run.

That's exactly what happens with Firefox. Here is the code from XRE_mainStartup (see https://searchfox.org/mozilla-central/rev/002023eb262be9db3479142355e1675645d52d52/toolkit/xre/nsAppRunner.cpp#4385 for details):

mGdkDisplay = gdk_display_open(display_name); /* first indirect call to gdk_x11_display_make_default() */
if (!mGdkDisplay) {
  PR_fprintf(PR_STDERR, "Error: cannot open display: %s\n", display_name);
  return 1;
  }
gdk_display_manager_set_default_display(gdk_display_manager_get(),
                                              mGdkDisplay); /* second direct call to gdk_x11_display_make_default() */

By calling gdk_display_open() we indirectly call gdk_x11_display_make_default() for a first time because GDK automatically chooses first opened display as default display and calls dk_x11_display_make_default() accordingly (using callback for "opened" gsignal). Here is the backtrace:

#0  gdk_x11_display_make_default (...) at ././gdk/x11/gdkdisplay-x11.c:2035
#1  gdk_display_manager_set_default_display (...) at ././gdk/gdkdisplaymanager.c:392
#2  _gdk_display_manager_add_display (...) at ././gdk/gdkdisplaymanager.c:489
#3  g_closure_invoke (...) at ././gobject/gclosure.c:804
#4  signal_emit_unlocked_R (...) at ././gobject/gsignal.c:3673
#5  g_signal_emit_valist (...) at ././gobject/gsignal.c:3391
#6  g_signal_emit_by_name (..., detailed_signal=detailed_signal@entry=0x7ffff53a44ca "opened") at ././gobject/gsignal.c:3487
#7  _gdk_x11_display_open (...) at ././gdk/x11/gdkdisplay-x11.c:1609
#8  dk_display_manager_open_display (...) at ././gdk/gdkdisplaymanager.c:472
#9  XREMain::XRE_mainStartup(bool*) (...) at ../firefox/toolkit/xre/nsAppRunner.cpp:4411

And then gdk_x11_display_make_default() gets called again as a backend for gdk_display_manager_set_default_display(). Here is the backtrace:

#0  gdk_x11_display_make_default (...) at ././gdk/x11/gdkdisplay-x11.c:2035
#1  gdk_display_manager_set_default_display (...) at ././gdk/gdkdisplaymanager.c:392
#2  XREMain::XRE_mainStartup(bool*) (..) at ../firefox/toolkit/xre/nsAppRunner.cpp:4416

At this point content of DESKTOP_STARTUP_ID is lost and proper startup notification can't be generated when GTK later displays the main window of the browser.

Please note that the code of gdk_wayland_display_make_default() used for Wayland-based display servers looks exactly the same as gdk_x11_display_make_default() which I suppose leads to the same kind of misbehavior (see https://github.com/GNOME/gtk/blob/d9a382d689b9159daca09e33510ea229a17d37be/gdk/wayland/gdkdisplay-wayland.c#L799 for details).

Let's make things clear, this misbehavior is a bug in GTK <= 3.24.7 (now fixed). But I suppose that we can deal with it internally to fix startup notifications for people still using GTK <= 3.24.7. All we probably need to do is to remove excessive call to gdk_display_manager_set_default_display() in XRE_mainStartup(). First opened display becomes default one automatically and we don't need to additionally specify that by calling gdk_display_manager_set_default_display(). I tested this approach with both trunk and esr78, it seems to solve the issue without any observable drawbacks. But I'm not in any way expert in GTK/GDK and would be really happy to get some thoughts from people who knows this codebase better.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Oleg, you seems to be fully qualified for here, do you mind to create a patch for it?
Thanks.

Comment 4

[Oleg Strikov]

Attachment: Bug 1696498 - Fix startup notifications on Linux by removing excessive GDK call which triggers a bug in GTK <= 3.24.7. r=stransky

Backend procedure gdk_{x11,wayland}display_make_default() gets called twice by XREMain::XRE_mainStartup(). First time it gets called indirectly after a call to gdk_display_open() because GDK automatically chooses first opened display as default display and calls gdk{x11,wayland}display_make_default() accordingly. Second time it gets called directly as a backend for an excessive call to gdk_display_manager_set_default_display(). Two consecutive calls to gdk{x11,wayland}_display_make_default() trigger a bug in GTK <= 3.24.7 which causes content of DESKTOP_STARTUP_ID environment variable to get lost (GTK Bug 790963). In the absence of this information, startup notification can't be properly sent back to the environment when GTK later displays the main window.

Comment 5

[Oleg Strikov]

Hi Martin! I just submitted a patch to Phabricator (https://phabricator.services.mozilla.com/D108314) and put yourself as a reviewer. I don't have access to the try server so I'd appreciate if you do a test run for me. Thanks for helping!

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Sure, will do so.
Thanks.

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c989bb1e2d4638f81bf75af56421c24afa0aaf1c

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc30b031c5d9
Fix startup notifications on Linux by removing excessive GDK call which triggers a bug in GTK <= 3.24.7. r=stransky

Comment 9

[Oleg Strikov]

Hi Martin! Many thanks for all your kind help with the bug!

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/dc30b031c5d9