Closed Bug 838097 Opened 7 years ago Closed 6 years ago

Clarify nsContentPrefService behavior and modify nsContentPrefService2 to match it

Categories

(Toolkit :: Preferences, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: gsvelto, Unassigned)

References

Details

Currently nsContentPrefService disallows content processes from getting or setting preferences except for two that have been explicitly whitelisted: browser.upload.lastDir and spellcheck.lang. nsContentPrefService2 on the other hand has been implemented in a way that prevents content processes from accessing any preference. According to bug 777196 the behavior of nsContentPrefService should be removed; if this is the case then nsContentPrefService2 should be modified not to propagate notifications of changes to content processes, if it is not then it should be modified to be usable from content processes according to the whitelist provided in nsContentPrefService.
I never really understood the reasoning behind these content process limitations - seems like if we have an exploited content process we have worse problems to worry about than them setting content prefs.

But I haven't thought about this very much. Since b2g is the only potential multi-process consumer in the near term, someone from that team may have stronger/more informed opinions about the best way forward here.
Hi Gabriele, then it sounds like the right thing to do is modify nsContentPrefService2 so that it doesn't broadcast notifications to content processes.  Since you made the original modification in bug 835352 to broadcast to content processes, would you like to take this bug as well?
it is actually not broadcasting, it's just the name of the method, since it can't even get there.
that said, did we already reach consensus CPS2 won't support that? what's the migration code path for B2G code if we remove CPS?
I've spent some time investigating this and here's what I found. The preferences whitelist was originally added to be able to set 'browser.upload.lastDir' from content processes (bug 575556 and bug 584842). From the discussion it seems that both Fennec and the (now dead?) Maemo port were affected which suggests that preventing nsContentPrefsService2 from accessing white-listed entries will break something there.

Similarly the entry for 'spellcheck.lang' was added as part of bug 678842 however bug 678842 comment 7 doesn't shed much light as to why. AFAIK spell checking is not available in Fennec so I don't know where else this preference would be set from a content process.

As an extra test I tried emptying the white list and running tests, the only things that break are unit tests:

https://tbpl.mozilla.org/?tree=Try&rev=9e1fec494801

For the B2G browser it seems to me that the general consensus was that this kind of things should be handled via mozbrowser which sounds like a saner solution to me. With the data at hand for other consumers I don't know if turning off the white list entirely would break something or not.
To clarify, the scope of this bug is to track bug 777196, if that will require changes in cps2, they will happen here, otherwise we are done.
Now to proceed here we need action in bug 777196.
Now that bug we fixed bug 777196 this can also be closed as no further actions are needed here.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.