1433685 - Remove nsGConfService

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Cedric]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:40.0) Gecko/20180101 Firefox/52.0

Steps to reproduce:

gconf, being a long deprecated GNOME 2 component, should not be dependend upon. Under normal circumstances, GSettings should be used, which allows for any backend. See also the removal of gtk code in bug 1278282

Comment 1

[Gregory Szorc [:gps]]

glandium: could you please triage this request? I'm not sure how to even estimate the scope of this. I see some C/C++ code references to gconf APIs. So scope seems beyond the build system.

Comment 2

[Mike Hommey [:glandium]]

gconf is not a hard dependency. We don't link against it, and we only use it if it's present at run time. In fact, we probably don't even need the gconf headers (would need a couple simple changes). It might be a good thing to do that.

There are essentially three pieces of code in Firefox that rely on gconf: accessibility, shell service and system proxy settings. (there's also a #include of nsIGConfService.h in nsWindow.cpp, but it doesn't seem used).

The first uses it to check whether a11y should be enabled, as a fallback from checking some dbus things. Maybe this should have a gsettings equivalent, I don't know, but there currently isn't one.

The second uses it to set/get a desktop wallpaper, set Firefox as the default browser or check whether it is, or drive the opening of applications for mailto: or news: links. The use of alternatives from gsettings/gio is actually not consistent here. We sometimes use gconf first, sometimes last. Maybe there's something to do there.

The third uses it to get system proxy configuration for when Firefox is configured to use the system proxy (which IIRC is the default). gconf is only used there if gsettings is not available.

With all that being said, I don't think the minimum version of glib we require lacks gsettings or giovfs, so except for accessibility, practically speaking, we're mostly not using gconf (except when the code tried it first, and it's installed). So it would seem it's a good candidate for removal. The a11y situation should be checked, though.

Karl, do you have additional thoughts?

Comment 3

[Mike Hommey [:glandium]]

Relatedly, we're still dlopen()ing e.g. libgio in nsGSettingsService.cpp, we can probably simplify all that by using gio directly too.

Comment 4

[Mike Hommey [:glandium]]

Bwarf, we already have a hard dependency on the gsettings symbols through intl/locale/gtk/OSPreferences_gtk.cpp, which kind of duplicates nsGSettingsService.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to Mike Hommey [:glandium] from comment #2)
> The first uses it to check whether a11y should be enabled, as a fallback
> from checking some dbus things. Maybe this should have a gsettings
> equivalent, I don't know, but there currently isn't one.

IIRC the dbus things were to check a server which would read the appropriate
gsettings, and so there is no need for a gsettings fallback.

> With all that being said, I don't think the minimum version of glib we
> require lacks gsettings or giovfs, so except for accessibility, practically
> speaking, we're mostly not using gconf (except when the code tried it first,
> and it's installed). So it would seem it's a good candidate for removal. The
> a11y situation should be checked, though.
> 
> Karl, do you have additional thoughts?

Yes, I think we already have a hard dependency on a sufficient GIO version
through GTK libraries.

Even MATE uses GSettings now according to
http://wiki.mate-desktop.org/docs:gsettings

And https://packages.debian.org/oldstable/cinnamon implies that Cinnamon also
uses GSettings.

So, I also expect that GConf code can be removed.
I expect it can be removed in ally even.

Comment 7

[Mike Hommey [:glandium]]

Martin, are you going to do a11y too?

Comment 8

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

(In reply to Mike Hommey [:glandium] from comment #7)

Martin, are you going to do a11y too?

Yes, I will.

Comment 9

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

Attachment: Bug 1433685 - Remove nsGConfService, r=glandium

Comment 10

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a7bc9379d2baf5f0ca2591ec617bfc1e78f7644

Comment 11

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

Filed a follow up for crashreporter - Bug 1543351

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e18f4ceca46b
Remove nsGConfService, r=glandium

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e18f4ceca46b