WakeLockListener - postpone dbus_bus_get() until we need it

RESOLVED FIXED in Firefox -esr60

Status

()

defect
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: stransky, Assigned: stransky)

Tracking

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox63 fixed, firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

8 months ago
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+
Assignee

Updated

8 months ago
Keywords: checkin-needed

Comment 4

8 months ago
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

Comment 5

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/526b8427a5bf
Status: NEW → RESOLVED
Last Resolved: 8 months 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)
Assignee

Comment 7

8 months ago
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)
Assignee

Comment 11

8 months ago
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.