Comment 12 for bug 875266

Comment on attachment 556542
gsettings proxy patch v 1

I looked at the GSettingsService changes.

There is a new symbol that needs to be added to the GSETTINGS_FUNCTIONS at the
top of the file for dynamic lookup, and some other things to touch up there so
that this compiles against old versions of GIO.

>+#include "nsISupports.h"
> #include "nsCOMPtr.h"
>+#include "nsIMutableArray.h"

I expect nsISupports.h does not need to be listed explicitly because
nsIMutableArray.h or nsISupportsPrimitives.h will pull it in.

>+ const gchar ** gs_strings = g_variant_get_strv(value, NULL);
>+ g_variant_unref(value);

gs_strings doesn't own the strings, only the list of pointers.
What owns the strings after value is released?

>+ if (!gs_strings) {
>+ // empty array
>+ return NS_OK;
>+ }

aResult needs to be set if returning NS_OK.

>+ nsCOMPtr<nsIMutableArray> items(do_CreateInstance(NS_ARRAY_CONTRACTID));
>+ if (!items)
>+ return NS_ERROR_OUT_OF_MEMORY;

Leaks gs_strings.

Probably tidiest to move this construction to before other allocations.

>+ nsCOMPtr<nsISupportsString> obj(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));

Would it make sense to instead use nsISupportsCString?
That would be more consistent with the AUTF8Strings returned by getString and
used in parameters, and it is probably the format more useful to the caller in
this case.

Think about whether using NS_NewAdoptingUTF8StringEnumerator instead of
nsIMutableArray would simplify things:
http://hg.mozilla.org/mozilla-central/annotate/84117219ded0/xpcom/ds/nsStringEnumerator.h#l90