Settings API: Need to notify content processes about the settings changes when calling SettingsService.set() from chrome process

RESOLVED FIXED in mozilla18



6 years ago
5 years ago


(Reporter: Gene Lian (I already quit Mozilla), Assigned: Gene Lian (I already quit Mozilla))


Bug Flags:
in-testsuite -

Firefox Tracking Flags



(Whiteboard: [LOE:S])


(1 attachment, 1 obsolete attachment)

3.76 KB, patch
Gene Lian (I already quit Mozilla)
: review+
Details | Diff | Splinter Review


6 years ago
Please see for the discussion thread. In summary, Gecko should be responsible for notifying all the content processes about the settings changes even if they're changed by the chrome process.
Some Background: we implemented the SettingsService to read settings during startup where we don't have any windows or content processes.


5 years ago
Blocks: 784220

Comment 2

5 years ago
According to :hsinyi's saying, bug 784220 (telephony) recently needs this one to be fixed, because the content process has to keep track of the settings status that could be changed from chrome process. So... we got some business here. ;) I think I can support to upload a patch for solving this issue in a couple of days.

As mentioned by :sicking, this should be a basecamp-blocker now.
Assignee: nobody → clian
blocking-basecamp: --- → ?

Comment 3

5 years ago
Created attachment 659147 [details] [diff] [review]

Hi :gwagner :)

I'd like to invite you to take a look on this patch. Some notes:

1. I directly use the existing SettingsChangeNotifier.jsm to listen to the "mozsettings-changed" observer event fired from the chrome process.

2. After receiving the "mozsettings-changed" observer event, it will broadcast "Settings:Change:Return:OK" messages to all the content processes.

3. Note that to avoid redundantly dealing with the "mozsettings-changed" observer event that could be requested from content processes, I use a marker to "fromSettingsChangeNotifier" to distinguish that.
Attachment #659147 - Flags: review?(anygregor)
blocking-basecamp: ? → +

Comment 4

5 years ago
Hi Gregor, may I ping on this one for review? Thanks a lot!
Whiteboard: [LOE:S]

Comment 5

5 years ago
Hi Gregor, sorry for bugging. Ping a bit on this one. Thanks!
Comment on attachment 659147 [details] [diff] [review]

>+      case kMozSettingsChangedObserverTopic:
>+      {
>+        let setting = JSON.parse(aData);
>+        if (setting.message && setting.message === kFromSettingsChangeNotifier)
>+          return;

Please add a comment here explaining when this happens: "setting.message === kFromSettingsChangeNotifier"
Attachment #659147 - Flags: review?(anygregor) → review+

Comment 7

5 years ago
Created attachment 662151 [details] [diff] [review]
Patch 1.1

:gwagner already had review+ at comment #6.
Attachment #659147 - Attachment is obsolete: true
Attachment #662151 - Flags: review+


5 years ago
Keywords: checkin-needed


5 years ago
Flags: in-testsuite-
(In reply to Gene Lian [:gene] from comment #8)
> Waiting for try server:

Green on Try.
Keywords: checkin-needed
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.