Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Boris Zbarsky]

On 6/11/19 7:49 AM, Kartikaya Gupta wrote:

IIRC another difference between prefs in all.js and gfxPrefs was that if a
pref was not listed in all.js, you couldn't use it in the
{test-,ref-,}pref(...) annotations in reftest.list files.


That restriction is due to the code at 
https://searchfox.org/mozilla-central/rev/0da35261b6789eec65476dbdd4913df6e235af6d/layout/tools/reftest/reftest.jsm#675-699 
refusing to set the new value if it can't get the old 
(to-be-restored-later) value.  From that point of view, StaticPrefs and 
all.js are equivalent: both provide an existing value to work with.


That said, we could probably improve this code to restore 
no-value-set-before prefs via clearUserPref so it would not have this 
restriction at all.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jean-Yves Avenard]

Hi.

Thanks for the kind words.

> On 11 Jun 2019, at 9:49 pm, Kartikaya Gupta  wrote:
> 
> IIRC another difference between prefs in all.js and gfxPrefs was that if a 
> pref was not listed in all.js, you couldn't use it in the 
> {test-,ref-,}pref(...) annotations in reftest.list files. Can you confirm 
> that listing the pref in StaticPrefs but not all.js is not subject to this 
> restriction?

A gfxPref didn't set or create the related Preference to any value. It was only 
happening the other way round: the gfxPref would be initialised in that process 
to the value of the Preference at the time of the process creation but only if 
the preference existed.
Such that the value set in all.js would override the gfxPrefs default.

If you had only defined the gfxPref then the pref wouldn't show up in 
about:config either (which is why people for convenience also added an entry to 
all.js).

However, if you were to call the gfxPref setter method, it would then set the 
related Preference but only if called on the parent process and in the main 
thread. Otherwise, the change to the gfxPref was local to the current process 
only.

Setting a pref by listing all.js or in StaticPrefList.h initialise things in 
the same manner, they are fundamentally equivalent.

Now, while I'm not aware of the case you describe, I don't see how using 
StaticPref could break that.

I hope that none of the gfxpref flaws got carried into StaticPref, I believe 
they were all fixed following this transition.

Jean-Yves 

Get Firefox for iOS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Kartikaya Gupta]

On Tue., Jun. 11, 2019, 04:01 Jean-Yves Avenard, 
wrote:

>
> 1- If you define a StaticPref in StaticPrefList.h there is absolutely
> no-need to also defines the value in all.js. In fact this is strongly
> discouraged and there should almost never be a need for it. There will
> be no end-user visible difference if your pref is only defined in
> StaticPrefList.h : the pref value will still appear in about:config.
>

IIRC another difference between prefs in all.js and gfxPrefs was that if a
pref was not listed in all.js, you couldn't use it in the
{test-,ref-,}pref(...) annotations in reftest.list files. Can you confirm
that listing the pref in StaticPrefs but not all.js is not subject to this
restriction?

And thank you for making these changes!

Cheers,
kats
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jean-Yves Avenard]

Hi there.

So the changes have been lived for a couple of weeks now and some issues 
were raised or encountered since. So I would like to clarify some of those.


1- If you define a StaticPref in StaticPrefList.h there is absolutely 
no-need to also defines the value in all.js. In fact this is strongly 
discouraged and there should almost never be a need for it. There will 
be no end-user visible difference if your pref is only defined in 
StaticPrefList.h : the pref value will still appear in about:config.


Since Nick Nethercote cleaned up all.js in his first StaticPref 
iteration; 403 preferences have popped up with duplicated 
initialisation, often with different default value set between the two.


So if you add a StaticPref, remove the existing all.js entry if it exists.

2- Please read the StaticPrefList.h documentation present at the 
beginning of the file. Please keep its content organised within the 
right section and alphabetically ordered. Don't group preferences with 
different prefix together because they relate to the same topic. 
Reconside the prefix used instead.


3- Carefully consider when a StaticPref is defined with a Once policy: 
do you need to use such policy and are you appropriately testing it?


StaticPrefs with a Once policy will be initialised when one is read; all 
of them will now be frozen for the entire lifetime of the parent process 
(following bug 1554334)


If you create a test for that pref, and set that pref via means such as 
SpecialPower.pushPref(), via the web-platform-test etc; those will *not* 
update the value of the StaticPref. In order to prevent such misuse, in 
bug 1556131 at the request of Boris we created a test that is enabled 
during automation testing on debug build. Should anything attempts to 
modify the underlying preference of a `Once` StaticPref, it will crash 
with something like:


Assertion failure: staticPrefValue == preferenceValue (Preference 
'webgl.force-layers-readback' got modified since 
StaticPrefs::WebGLForceLayersReadback got initialized. Consider using a 
Live StaticPrefs instead), at 
/builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/StaticPrefList.h:6529


All the best

Jean-Yves

On 15/05/2019 11:02 pm, Jean-Yves Avenard wrote:

Dear all.

/TLDR; Wherever you used to use gfxPrefs, soon you will have to use 
StaticPrefs./


In a couple of days, once /Bug 1550422 
//lands I will 
be retiring gfxPrefs. All features originally found in gfxPrefs are 
now available in StaticPrefs with some extra bonuses./


//For the background, StaticPrefs gives you the ability to access a 
preference via a thread-safe accessor.

//

/StaticPrefs and Preferences will now be available on all processes 
(not just main and content, this includes GPU, VR and RDD process)/


//

/3 levels of update policies: Skip, Once and Live:/

/* Skip policy will ignore all user overrides.
* Once will read the preference once and will never be updated again
* Live is the original behaviour, the values will be updated across 
all processes whenever the preference change.

/

/Possibility to dynamically set a StaticPref on any threads (however, 
the changes aren't propagated to other processes; doing otherwise is 
certainly doable, I'm not convinced of the use case however)./


/There are few more options, to know more I invite you to read the 
StaticPrefList.h file

/

/The desire to gfxPrefs came from yet another misuse of StaticPrefs in 
the GPU process. In addition gfxPrefs turned out to not be thread-safe./


/It became rather tiring to always juggle between gfxPrefs and 
StaticPrefs depending on which process the code could run./


/And as Mr MacLeod used to say: There can be only one/

/Jean-Yves
/


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jean-Yves Avenard]

On 16/05/2019 9:02 am, Botond Ballo wrote:

Will SpecialPowers.pushPrefEnv(), which currently does propagate the
prefs at least between the content and parent processes, continue to
work? A lot of tests rely on this.


Yes of course.

The changes was to add changes so that process other than the content 
process are synced. I didn't check the necko process, but reading Kris 
message it seems that it does already.


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Kris Maglione]

On Wed, May 15, 2019 at 07:02:58PM -0400, Botond Ballo wrote:

On Wed, May 15, 2019 at 6:54 PM Jean-Yves Avenard  wrote:

It is then up to each process to handle preference modifications and
pass that change. There's no automatic or universal method on how this
is done unfortunately.


Will SpecialPowers.pushPrefEnv(), which currently does propagate the
prefs at least between the content and parent processes, continue to
work? A lot of tests rely on this.


Preference changes in the parent process are automatically 
propagated to any child processes that handle preference 
changes. Currently, I believe that does not include GPU and VR 
processes. It does include web content processes and the necko 
process.


But for processes that do propagate preference changes, it 
doesn't matter whether they use StaticPrefs or another 
preference access API.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Botond Ballo]

On Wed, May 15, 2019 at 6:54 PM Jean-Yves Avenard  wrote:
> It is then up to each process to handle preference modifications and
> pass that change. There's no automatic or universal method on how this
> is done unfortunately.

Will SpecialPowers.pushPrefEnv(), which currently does propagate the
prefs at least between the content and parent processes, continue to
work? A lot of tests rely on this.

Thanks,
Botond
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jean-Yves Avenard]

Hi

On 16/05/2019 1:54 am, Nicholas Alexander wrote:
Forgive my rank ignorance here, but as an outsider this surprises me: 
I expect "Gecko preferences" to be (eventually) consistent across 
processes.  Is this just not the case, and it's common for prefs to 
vary between the main and content/gfx processes?  Is there a `user.js` 
equivalent for main and child processes?



The whole handling of the preferences across processes is a bit fishy.

Today, preference are fully available only on the main and content 
processes with the limitation that modifying a preference must be done 
on the main thread, and if you modify one outside the main process, the 
change will be local to that process and won't propagate to the others.


On the RDD process (used for media decoding), preferences values are 
passed at the process creation only.


There's no preference support at all on the VR and GPU process.

If you modify a preference on the main thread, the change will propagate 
to process that specifically handle the update with custom code.


Following bug 1550422, Preference will be available on the other 
processes, and will be synced from the main process up. So the 
restrictions that you must modify the preference on the main process for 
them to propagate will remain.


When a new process is created, Preferences value are passed via command 
line arguments except on Windows. On Windows a shared memory object is 
created and the handle to that object is passed via the command line.


It is then up to each process to handle preference modifications and 
pass that change. There's no automatic or universal method on how this 
is done unfortunately.


Jean-Yves


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Nicholas Alexander]

On Wed, May 15, 2019 at 6:03 AM Jean-Yves Avenard 
wrote:

> Dear all.
>




> /Possibility to dynamically set a StaticPref on any threads (however,
> the changes aren't propagated to other processes; doing otherwise is
> certainly doable, I'm not convinced of the use case however)./
>

Forgive my rank ignorance here, but as an outsider this surprises me: I
expect "Gecko preferences" to be (eventually) consistent across processes.
Is this just not the case, and it's common for prefs to vary between the
main and content/gfx processes?  Is there a `user.js` equivalent for main
and child processes?

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jonathan Kew]

Great to see this consolidation happening; it's going to be a great 
improvement on the current situation.


Just to check, would it be correct to assume this won't be landing on 
central until after the current Fx68 soft-freeze period? It seems like 
the sort of wide-ranging change that probably shouldn't be hitting the 
tree just now.


Thanks for cleaning things up for us!

JK

On 15/05/2019 14:02, Jean-Yves Avenard wrote:

Dear all.

/TLDR; Wherever you used to use gfxPrefs, soon you will have to use 
StaticPrefs./


In a couple of days, once /Bug 1550422 
//lands I will be 
retiring gfxPrefs. All features originally found in gfxPrefs are now 
available in StaticPrefs with some extra bonuses./




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Change to preferences via StaticPrefs, tremoval of gfxPrefs

[Jean-Yves Avenard]

Dear all.

/TLDR; Wherever you used to use gfxPrefs, soon you will have to use 
StaticPrefs./


In a couple of days, once /Bug 1550422 
//lands I will be 
retiring gfxPrefs. All features originally found in gfxPrefs are now 
available in StaticPrefs with some extra bonuses./


//For the background, StaticPrefs gives you the ability to access a 
preference via a thread-safe accessor.

//

/StaticPrefs and Preferences will now be available on all processes (not 
just main and content, this includes GPU, VR and RDD process)/


//

/3 levels of update policies: Skip, Once and Live:/

/* Skip policy will ignore all user overrides.
* Once will read the preference once and will never be updated again
* Live is the original behaviour, the values will be updated across all 
processes whenever the preference change.

/

/Possibility to dynamically set a StaticPref on any threads (however, 
the changes aren't propagated to other processes; doing otherwise is 
certainly doable, I'm not convinced of the use case however)./


/There are few more options, to know more I invite you to read the 
StaticPrefList.h file

/

/The desire to gfxPrefs came from yet another misuse of StaticPrefs in 
the GPU process. In addition gfxPrefs turned out to not be thread-safe./


/It became rather tiring to always juggle between gfxPrefs and 
StaticPrefs depending on which process the code could run./


/And as Mr MacLeod used to say: There can be only one/

/Jean-Yves
/

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform