1882664 - Newly set prefs via --setpref do not seem to be activated from the command line in the js shell

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This may not be a bug in the usual sense, but I'm trying to figure out the right way to make use of bug 1877193's improvements.

On m-c rev 3facb0e94116 with debug js shell AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, I run the following:

$ ./js-dbg-64-linux-x86_64-3facb0e94116 --fuzzing-safe --list-prefs
array_grouping=true
arraybuffer_transfer=true
destructuring_fuse=true
experimental.arraybuffer_resizable=false
/snip

The default value of experimental.arraybuffer_resizable is false.

I can turn it on with --enable-arraybuffer-resizable on the same command line.

$ ./js-dbg-64-linux-x86_64-3facb0e94116 --fuzzing-safe --enable-arraybuffer-resizable --list-prefs
array_grouping=true
arraybuffer_transfer=true
destructuring_fuse=true
experimental.arraybuffer_resizable=true
/snip

However, with the new --setpref way of doing things, I cannot seem to get it to work:

$ ./js-dbg-64-linux-x86_64-3facb0e94116 --fuzzing-safe --setpref=experimental.arraybuffer_resizable=true --list-prefs
array_grouping=true
arraybuffer_transfer=true
destructuring_fuse=true
experimental.arraybuffer_resizable=false
/snip

experimental.arraybuffer_resizable is false when I wanted it to be true. Nor with -P either:

$ ./js-dbg-64-linux-x86_64-3facb0e94116 --fuzzing-safe -P experimental.arraybuffer_resizable=true --list-prefs
array_grouping=true
arraybuffer_transfer=true
destructuring_fuse=true
experimental.arraybuffer_resizable=false
/snip

I'm aware of bug 1882472 but that seems to involve whether I would put a = after --setpref or -P.

What am I doing wrong? Or is this intended? Or a bug?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1877193

Comment 2

[Jan de Mooij [:jandem]]

Hm good catch, it's caused by how we handle the pref aliases:

  JS::Prefs::setAtStartup_experimental_arraybuffer_resizable(
      op.getBoolOption("enable-arraybuffer-resizable"));

This happens after we process the --setpref options so it overwrites those pref values.

To fix this we could change the ordering, but the better fix might be to not set the pref value if the alias isn't used.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1882664 - Use default pref value in JS shell when alias isn't used. r?mgaudet!

This lets us use --setpref for prefs with a legacy alias.

It's also more consistent to use the default value from the YAML file if the alias isn't used.

clang-format added some unrelated (Wasm) changes.

Comment 4

[Jan de Mooij [:jandem]]

Thanks for reporting this, Gary!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8532c04bf9a8
Use default pref value in JS shell when alias isn't used. r=mgaudet

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8532c04bf9a8

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Jan de Mooij [:jandem] from comment #4)

Thanks for reporting this, Gary!

You're welcome! I've now verified this works as expected on m-c rev 76bfcd57b0cd.

More questions ahead:

1. For Jan: tests.uint32-pref=1 takes a number value instead of true/false, how does the 1 value work here? Can we synchronise everything on true/false values? It'll make testing easier as that pref is the only one with a number rather than a boolean.

2. I'm going to guess that any pref under the experimental section in --list-prefs is not yet turned on by default on at least nightly builds? And hence not ready for fuzzing to get bug bounties? Or maybe they're valid for bounties, just lower amounts due to them not being on by default yet.

Asking especially wrt. bug bounties, getting :dveditz on as well.

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #7)

1. For Jan: tests.uint32-pref=1 takes a number value instead of true/false, how does the 1 value work here? Can we synchronise everything on true/false values? It'll make testing easier as that pref is the only one with a number rather than a boolean.

Most prefs are booleans, but we also have to support numeric prefs. tests.uint32-pref is a test pref to let us that code - the value can be any uint32 value.

2. I'm going to guess that any pref under the experimental section in --list-prefs is not yet turned on by default on at least nightly builds?

This probably depends on the feature, also whether it's ready for fuzzing. Some of them are listed in fuzz-flags.txt I think.

Comment 9

[Daniel Veditz [:dveditz]]

Asking especially wrt. bug bounties, getting :dveditz on as well.

If they're not finished and stable enough to be turned on in nightly they are default non-eligible

Comment 10

[Dianna Smith [:diannaS]]

:jandem is this something you are looking to uplift to 124? if not, please set the status flag to wontfix

Comment 11

[Jan de Mooij [:jandem]]

This can ride the trains.