1495404 - WakeLockListener - postpone dbus_bus_get() until we need it

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

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

Follow up from Bug 1487243:

According to documentation and some reports the dbus_bus_get() can hang if there's a problem with DBus connection. I may be related to actual DBus version on the test boxes, wrong permissions to DBus socket or something else.

I think we should postpone the dbus_bus_get() call until we really need it at WakeLockListener::Callback() - that will tile the dbus_bus_get() call with the code what actually uses it and makes the bug more clear and won't block whole browser startup.

Comment 1

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

Attachment: Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=59aca1f4c969ca03f92b765e823bfc9d6c1cf500

Comment 3

[Jan Horak [:jhorak]]

Comment on attachment 9013287 [details]
Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

Jan Horak [:jhorak] has approved the revision.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/526b8427a5bf
Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/526b8427a5bf

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Should we consider uplifting this patch to mozilla-beta? The test failures from bug 1487243 are also visible at this branch.

Comment 7

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

Comment on attachment 9013287 [details]
Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

Approval Request Comment
[Feature/Bug causing the regression]: Bug 811261
[User impact if declined]: Failed tests (Bug 1487243), browser hangs at startup.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low risk
[Why is the change risky/not risky?]: It delays dbus initialization until it's needed at screen saver inhibitor. If there's any problem with dbus, it won't hang browser at startup but media is played which can be more precisely diagnosed.
[String changes made/needed]: none

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9013287 [details]
Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

fix failing linux tests, uplift approved for 63 beta 12. Thanks.

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/95fd0bcb0f23

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR60 approval as well.

Comment 11

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

Comment on attachment 9013287 [details]
Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Failed tests (Bug 1487243), browser hangs at startup.

User impact if declined: It primary affects mozilla test infrastructure but can also affects users with wrong/non-functional DBus.

Fix Landed on Version: 64, 63 (beta)

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It delays dbus initialization until it's needed at screen saver inhibitor. If there's any problem with dbus, it won't hang browser at startup but media is played which can be more precisely diagnosed.

String or UUID changes made by this patch: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9013287 [details]
Bug 1495404 - Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak

Fixes a Linux startup hang, approved for ESR 60.3.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/3232bb3b622f