Fix external application menu occasionally slowing down startup
ClosedPublic

Authored by rkflx on Jul 8 2018, 10:33 PM.

Details

Summary

If moving the menubar entries to a button in the window decoration is
enabled, the bug reporter experiences delays of up to 25 seconds in the
startup of Gwenview.

Bisecting points to 9631043c1, which added MPRIS support through
providing a D-Bus interface.

While the exact nature of the problem remains unclear due to not being
reproducible outside of the bug reporter's environment, moving the
initialization of the MPRIS support after createGUI is reported to
correct the issue. This may be due to avoiding conflicts or races with
the D-Bus connection, which is used for the MPRIS service, the
ScreenSaver service and the application menu integration.

Thanks to Duncan for reporting the bug and coming up with a fix.

BUG: 395925
FIXED-IN: 18.04.3

Test Plan

dbus-send --session --dest=org.mpris.MediaPlayer2.Gwenview --type=method_call /org/mpris/MediaPlayer2 org.mpris.MediaPlayer2.Player.Next still moves to the next image in Gwenview.
The external application menu is still working fine.

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx requested review of this revision.Jul 8 2018, 10:33 PM
rkflx created this revision.
broulik added a subscriber: broulik.Jul 9 2018, 7:58 AM

Probably doing blocking DBus calls/introspection which has a default timeout of 30 seconds

rkflx added a comment.Jul 9 2018, 10:19 AM

@kossebau Tagging of 18.04.3 is today at 23:59 UTC. As the author, are you okay with the change, or should we investigate more, rewrite if necessary and delay to 18.08?

Probably doing blocking DBus calls/introspection which has a default timeout of 30 seconds

Question is, though: Why does the code run into the timeout only when using the external app menu and an unknown second factor (which we lack compared to Duncan)?

kossebau added a comment.EditedJul 9 2018, 10:30 AM

@rkflx I cannot remember anything which would need the init to be done that very point at setup (EDIT: besides the objects passed as args to exist of course :) ), so if moving it to that later point and works for you, no objection from my side. Have not yet got around to testing things myself.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 9 2018, 7:50 PM
This revision was automatically updated to reflect the committed changes.