Preferences::RegisterCallback and variants will now do exact, not prefix, matches

[Boris Zbarsky]

I have just landed a change[1] which changes
Preferences::RegisterCallback/RegisterCallbackAndCall to do an exact,
not prefix, match on the passed-in string.

So if you do:

Preferences::RegisterCallback(MyFunc, "foo");

and the preference named "foobar" changes, MyFunc will no longer be
called. Most consumers did not expect this behavior, and a number were
buggy as a result.

The old behavior is still available via a new
RegisterPrefixCallback/RegisterPrefixCallbackAndCall/UnregisterPrefixCallback
API. These can be used to observe all preferences whose names start
with the given string.

One caveat: It's very important to correctly pair up the registration
and unregistration functions. If you RegisterPrefixCallback but then
UnregisterCallback, it will not actually unregister the callback.

-Boris

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1348331

[Eric Rahm]

This doesn't affect the behavior of |Preferences::AddStrongObserver| which
does prefix matching, correct?

-e

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

[zbran...@mozilla.com]

Is there a reason we should use RegisterCallback over AddStrongObserver?

I have a fresh API where I'm using AddStrongObserver at the moment, but would be happy to switch if that will be cheaper / more future-compatible.

zb.

[Boris Zbarsky]

On 3/21/17 4:34 PM, Eric Rahm wrote:
> This doesn't affect the behavior of |Preferences::AddStrongObserver| which
> does prefix matching, correct?

That's correct. Same for AddWeakObserver.

It's a little harder to misuse because it doesn't have a closure arg (so
you can't just associate the totally wrong value with the closure in
some way, which was the main failure mode for RegisterCallback).

But just for fun, start a debug build, open "about:config", create a new
preference called "intl.accept_languages_are_confusing" with any value
you want, and watch the nice MOZ_ASSERT failure in
nsGlobalWindow::Observe on this line:

MOZ_ASSERT(!NS_strcmp(aData, u"intl.accept_languages"));

So maybe we should push the exact/prefix match business down into the
pref observer API. :(

-Boris

[Boris Zbarsky]

On 3/21/17 7:18 PM, zbran...@mozilla.com wrote:
> Is there a reason we should use RegisterCallback over AddStrongObserver?

It doesn't require creating a separate object to act as an observer. Of
course it creates one under the hood for you, so this is mostly a matter
of whether calling code ends up simpler or not, not of efficiency.

> I have a fresh API where I'm using AddStrongObserver at the moment, but would be happy to switch if that will be cheaper / more future-compatible.

Are you properly handling the fact that AddStrongObserver watches all
prefs starting with the prefix you pass it? ;)

-Boris

[zbran...@mozilla.com]

On Tuesday, March 21, 2017 at 7:46:07 PM UTC-7, Boris Zbarsky wrote:
> Are you properly handling the fact that AddStrongObserver watches all
> prefs starting with the prefix you pass it? ;)

I don't, and I'd love not to. I know perfectly well this two strings I want to watch only them.

I don't think there's a high risk of someone adding a new string to "general.useragent.locale", but if I could narrow it down to just this string instead of treating it as a prefix, I'd like to.

zb.

[Boris Zbarsky]

On 3/22/17 6:17 PM, zbran...@mozilla.com wrote:
> On Tuesday, March 21, 2017 at 7:46:07 PM UTC-7, Boris Zbarsky wrote:
>> Are you properly handling the fact that AddStrongObserver watches all
>> prefs starting with the prefix you pass it? ;)
>
> I don't, and I'd love not to. I know perfectly well this two strings I want to watch only them.

Well, AddStrongObserver will only do prefix checking for you. Anything
else you have to do yourself at the moment....

-Boris

[Ehsan Akhgari]

Ugh. Yeah, that looks like a good idea. Also, thank you for fixing
RegisterCallback at least!