Closed Bug 1495404 Opened 2 years ago Closed 2 years ago

WakeLockListener - postpone dbus_bus_get() until we need it

Categories

(Core :: Widget: Gtk, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- fixed
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

Details

Attachments

(1 file)

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 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.
Attachment #9013287 - Flags: review+
Keywords: checkin-needed
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/526b8427a5bf
Postpone dbus_bus_get() call until we need it at WakeLockListener::Callback(), r=jhorak
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/526b8427a5bf
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Should we consider uplifting this patch to mozilla-beta? The test failures from bug 1487243 are also visible at this branch.
Flags: needinfo?(stransky)
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
Flags: needinfo?(stransky)
Attachment #9013287 - Flags: approval-mozilla-beta?
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.
Attachment #9013287 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Please nominate this for ESR60 approval as well.
Flags: needinfo?(stransky)
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
Flags: needinfo?(stransky)
Attachment #9013287 - Flags: approval-mozilla-esr60?
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.
Attachment #9013287 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.