Comment 16 for bug 875266

Revision history for this message
In , Karlt (karlt) wrote :

Comment on attachment 577534
gsettings proxy patch v2

> FUNC(g_variant_get_string, const char *, (GVariant* value, gsize* length)) \
> FUNC(g_variant_is_of_type, gboolean, (GVariant* value, const GVariantType* type)) \
> FUNC(g_variant_new_int32, GVariant *, (gint32 value)) \
> FUNC(g_variant_new_boolean, GVariant *, (gboolean value)) \
> FUNC(g_variant_new_string, GVariant *, (const char* string)) \
>+ FUNC(g_variant_get_strv, GVariant *, (gsize* length)) \

These were roughly in an order.
Can you insert get_strv after get_string, please?

(In reply to jhorak from comment #10)
> Could you be more specific (the other things)?

Check the #define statements before and after GSETTIINGS_FUNCTIONS as any new
symbols need to be added here.

A #define is needed for g_variant_get_strv and G_VARIANT_TYPE_STRING_ARRAY.

>+ if (mGConf && IsProxyMode("auto")) {
>+ return mGConf->GetString(NS_LITERAL_CSTRING("/system/proxy/autoconfig_url"),
>+ aResult);
> }

>+ if (mGSettings) {
>+ // Check if mode is auto

I assume GSettings should override GConf settings.
Otherwise I assume those who upgrade to GNOME 3 will still be using their old
GConf settings, but the configuration utility will change the GSettings
values.

>+ nsCString proxyMode;
>+ nsCOMPtr<nsIGSettingsCollection> proxy_settings;
>+ mGSettings->GetCollectionForSchema(NS_LITERAL_CSTRING("org.gnome.system.proxy"),
>+ getter_AddRefs(proxy_settings));
>+ if (proxy_settings) {
>+ nsresult rv = proxy_settings->GetString(NS_LITERAL_CSTRING("mode"), proxyMode);

Move the proxyMode declaration to within the "if (proxy_setttings)" block
where it is used.

>+ return proxy_settings->GetString(NS_LITERAL_CSTRING("autoconfig-url"),
>+ aResult);

Alignment.

>+ PRInt32 port;
>+ rv = proxy_settings->GetInt(NS_LITERAL_CSTRING("port"), &port);
>+ NS_ENSURE_SUCCESS(rv, rv);
>+
>+ SetProxyResult(aType, host, port, aResult);
>+ return NS_OK;

Need to return NS_ERROR_FAILURE when the port is 0.

       'Each of the 4 proxy types is enabled if its "host" key is
        non-empty and its "port" key is non-0.'

http://git.gnome.org/browse/gsettings-desktop-schemas/tree/schemas/org.gnome.system.proxy.gschema.xml.in.in

>+ // Check if proxy is enabled, flag is only in schema org.gnome.system.proxy.http,
>+ // there is no separate flag for each schema.
>+ nsresult rv = mGSettings->GetCollectionForSchema(NS_LITERAL_CSTRING("org.gnome.system.proxy.http"),
>+ getter_AddRefs(proxy_settings));
>+ NS_ENSURE_SUCCESS(rv, rv);
>+ rv = proxy_settings->GetBoolean(NS_LITERAL_CSTRING("enabled"), &masterProxySwitch);
>+ NS_ENSURE_SUCCESS(rv, rv);

"enabled" is described as "Unused", so don't use this key.

>+ bool useHttpProxyForAll = false;
>+ // This setting sometimes doesn't exist, don't bail on failure
>+ proxy_settings->GetBoolean(NS_LITERAL_CSTRING("use-same-proxy"), &useHttpProxyForAll);
>+
>+ if (!useHttpProxyForAll) {
>+ rv = SetProxyResultFromGSettings("org.gnome.system.proxy.socks", "SOCKS", aResult);
>+ if (NS_SUCCEEDED(rv))
>+ return rv;
>+ }
>+

Similarly, "use-same-proxy" is also "Unused".

>+ if (aScheme.LowerCaseEqualsLiteral("http") || useHttpProxyForAll) {
>+ rv = SetProxyResultFromGSettings("org.gnome.system.proxy.http", "PROXY", aResult);
>+ } else if (aScheme.LowerCaseEqualsLiteral("https")) {
>+ rv = SetProxyResultFromGSettings("org.gnome.system.proxy.https", "PROXY", aResult);

Need to handle this case:

       "If an http proxy is configured, but an https proxy is not,
        then the http proxy is also used for https."