Register DBus connection to gmain loop

VERIFIED FIXED in Firefox -esr60

Status

()

defect
VERIFIED FIXED
8 months ago
7 months ago

People

(Reporter: stransky, Assigned: stransky)

Tracking

({regression})

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr6064+ verified, firefox62 unaffected, firefox63 wontfix, firefox64 verified)

Details

Attachments

(1 attachment)

Assignee

Description

8 months ago
After Bug 1495404 the Firefox main DBus connection is no longer registered do gmain as we miss the dbus_connection_setup_with_g_main() calls here.

We should register new connections to dbus_connection_setup_with_g_main() as we don't know which connection is the new.
Assignee

Updated

8 months ago
Keywords: checkin-needed

Comment 2

8 months ago
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ffed4de7dc0
register all DBus connection to be handled by gmain loop, r=jhorak
Keywords: checkin-needed

Comment 3

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ffed4de7dc0
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Assignee

Updated

8 months ago
Blocks: 1500850
I'm guessing we're at least going to want this on ESR60 at some point.
Jim, is this something you think we should uplift for an upcoming ESR?
Flags: needinfo?(jmathies)
Assignee

Comment 6

7 months ago
Yes, we should uplift it for the WifiScanner - it may break geolocation.
Assignee

Comment 7

7 months ago
Comment on attachment 9018560 [details]
Bug 1500366 - register all DBus connection to be handled by gmain loop, r=jhorak

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch registers a newly created DBus connection to gmain loop, we're missing that in our DBus handling code so the connection is not served by gmain and we're missing DBus messages.

User impact if declined: This may cause nonfunctional wifi scanner used for geolocation as it uses DBus.

Fix Landed on Version: 64

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Linux only, used on other places already.

String or UUID changes made by this patch: none
Attachment #9018560 - Flags: approval-mozilla-esr60?
Flags: needinfo?(jmathies)
Some testing of geolocation on Linux only once this patch lands on ESR60 would be great.
Flags: qe-verify+
Comment on attachment 9018560 [details]
Bug 1500366 - register all DBus connection to be handled by gmain loop, r=jhorak

Let's uplift this patch to avoid breaking geolocation for ESR users.
Attachment #9018560 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
I have tested some websites using geolocation on Ubuntu 16.04 x64 LTS using Fx 64.0b11 and Fx60.3.1ESR buildID: 20181119134246.
As part of the testing activity I used websites listed here:
- https://css-tricks.com/forums/topic/list-of-sites-using-geolocation/
I have also tested the most common maps that use geolocation like https://www.google.com/maps, https://www.bing.com/maps and https://whatismyipaddress.com.

The Geolocation was used while on a mobile WiFi hotspot, as well as a WiFi network connection. Everything worked without any issues.
Based on these results I will close the issue as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+

Updated

7 months ago
Depends on: 1512377

Updated

7 months ago
No longer blocks: 1500850
Depends on: 1500850
No longer depends on: 1512377

Comment 12

7 months ago
Can you backport bug 1500850 to esr60? Otherwise, Wayland build will be broken in 60.4.0.
Flags: needinfo?(stransky)
Assignee

Comment 13

7 months ago
(In reply to Jan Beich from comment #12)
> Can you backport bug 1500850 to esr60? Otherwise, Wayland build will be
> broken in 60.4.0.

It's already there.
Flags: needinfo?(stransky)

Comment 14

7 months ago
(In reply to Martin Stránský [:stransky] from comment #13)
> (In reply to Jan Beich from comment #12)
> > Can you backport bug 1500850 to esr60? Otherwise, Wayland build will be
> > broken in 60.4.0.
> 
> It's already there.

dbus_connection_setup_with_g_main is declared by dbus/dbus-glib-lowlevel.h but still not included by DBusRemoteClient.cpp

https://hg.mozilla.org/releases/mozilla-esr60/file/tip/widget/xremoteclient/DBusRemoteClient.cpp
Flags: needinfo?(stransky)
Assignee

Comment 15

7 months ago
It's at bug 1500850.
Flags: needinfo?(stransky)
You need to log in before you can comment on or make changes to this bug.