779381 - [Settings API] mozSettings cannot set multiple values with a single set

Status: RESOLVED WORKSFORME

(Core :: DOM: Device Interfaces, defect)

Description

[Evelyn Hung]

it is a part of issue in bug 778027

Test case 2: http://pastie.org/4340028

Expected: 

I/GeckoDump( 3253): ==== set
I/GeckoDump( 3253): ==== observer 1
I/GeckoDump( 3253): ==== observer 2

Actual: observer 1 does not fire, observer 2 is being called twice.

I/GeckoDump( 3253): ==== set
I/GeckoDump( 3253): ==== observer 2
I/GeckoDump( 3253): ==== observer 2



Test case 3: http://pastie.org/4340633

Expected:

====foo.bar1
====foo1
====foo.bar
====foo

Actual:

====foo.bar
====foo
====foo.bar
====foo

Comment 1

[Gene Lian (I already quit Mozilla)]

Is this one still an issue after the bug 778027 is solved?

Comment 2

[Fabien Cazenave [:kaze]]

Gene, yes it’s still an issue.

Comment 3

[Gene Lian (I already quit Mozilla)]

Thanks Fabien for the clarification. Btw, is this an blocking issue for you guys? If yes, I think I can help to support this one. Please let me know.

Comment 4

[Fabien Cazenave [:kaze]]

It’s just nice to have, not a blocker.

Comment 5

[Gene Lian (I already quit Mozilla)]

Just taking a note here. Sometimes we need to update the multiple settings at the same time, because the values are considered as a single set (updating either one would make the whole set meaningless). Since the settings API is async, we could probably face some edge cases that the whole set is not completely read/written yet, thus using the wrong values.

Bug 777251 is one example, where the MMS proxy settings mmsc/mmsproxy/mmsport should always be updated at one shot. However, the edge case wouldn't happen that easily but it's still a potential deficiency. We need to revisit this issue in the future. Glad to take this one if needed.

Comment 6

[Gregor Wagner [:gwagner]]

You can update multiple setting at the same time. Just use multiple set on a single lock.

Comment 7

[Gene Lian (I already quit Mozilla)]

(In reply to Gregor Wagner [:gwagner] from comment #6)
> You can update multiple setting at the same time. Just use multiple set on a
> single lock.

Hi Gregor, I'd like to clarify one thing. Supposing we're listening to "setting-name-a" and "setting-name-b" and each of them will trigger a "mozsettings-changed" in the Chrome code. However, they're coming *separately*. How can we know whether the client is actually changing one setting value or both when we're handling the first "mozsettings-changed" observer event? That is, we don't know if we need to wait for the second "mozsettings-changed" to collect the final values of "setting-name-a" and "setting-name-b".

Comment 8

[Gene Lian (I already quit Mozilla)]

Just dropping a note: this bug is aimed for setting, bug 895183 is aimed for getting.

Comment 9

[Gene Lian (I already quit Mozilla)]

This one sounds a good-first-bug for newbie since it's not urgent and could be backward compatible.

Comment 10

[Gene Lian (I already quit Mozilla)]

Btw, I didn't find the need in the proposal [1]. Do we really want to support this feature?

[1] https://wiki.mozilla.org/WebAPI/SettingsAPI

Comment 11

[Arthur Chen [:arthurcc][inactive after 6/30]]

I've done the tests listed in the description and it behaved as expected. Gregor, should we close the bug?

Comment 12

[Gregor Wagner [:gwagner]]

We have to find out what we actually want to solve here.
The Bug summary "cannot set multiple values with a single set" is different from the description where the concern is that the observer notification is wrong for a single set.
If the problem described in the Description works, lets change the summary and close it. 
If we still want the feature described in the summary, lets open a new bug.

Comment 13

[Arthur Chen [:arthurcc][inactive after 6/30]]

Both test cases in the description set multiple values with a single set. It seems no need to change the bug summary before closing it.

Comment 14

[Gregor Wagner [:gwagner]]

(In reply to Arthur Chen [:arthurcc] from comment #13)
> Both test cases in the description set multiple values with a single set. It
> seems no need to change the bug summary before closing it.

Even better :)