Rather than attempting to determine when the Once policy StaticPrefs should be set we initialize them when one of the getter gets called. They become immutable after that.
In a future change we will prevent those values to ever be changed once they have been initialized.
Differential Revision: https://phabricator.services.mozilla.com/D33440
--HG--
extra : moz-landing-system : lando
I also tried to avoid this change but, again, given the number of times I was
repeating the same pattern of defining a static method just to forward a
callback to an instance method, decided it was probably necessary. Without an
easy way to do this, people are more likely to register observers rather than
callbacks, for which we'll wind up paying a continued memory and performance
penalty.
This patch adds a helper which creates a type-safe preference callback
function which forwards calls to an instance method on its closure object.
The implementation is somewhat complicated, mainly due to the constraint that
unregistering a callback requires passing the exact same function pointer that
was used to register it. The patch achieves this by creating the callback
function as a template, with the method pointer as a template parameter. As
long as the Register and Unregister calls happen in the same translation unit,
the same template instance is guaranteed to be used for both.
The main difficulty is that, until C++ 17, there's no way match a value as a
template parameter unless you know its complete type, or can at least compute
its complete type based on earlier template parameters. That means that we
need a macro to extract the type of the method, and construct the template
with the full set of explicit parameters.
MozReview-Commit-ID: 10N3R2SRtPc
--HG--
extra : rebase_source : 7d0a8ddeb77e01d4a6f421459514e93bc0875598
I initially tried to avoid this, but decided it was necessary given the number
of times I had to repeat the same pattern of casting a variable to void*, and
then casting it back in a part of code far distant from the original type.
This changes our preference callback registration functions to match the type
of the callback's closure argument to the actual type of the closure pointer
passed, and then casting it to the type of our generic callback function. This
ensures that the callback function always gets an argument of the type it's
actually expecting without adding any additional runtime memory or
QueryInterface overhead for tracking it.
MozReview-Commit-ID: 9tLKBe10ddP
--HG--
extra : rebase_source : 7524fa8dcd5585f5a31fdeb37d95714f1bb94922
Preference callbacks consume a non-trivial amount of memory, which adds up
fast given the number of callbacks we register.
Many of our consumers, however, register a large number of callbacks with the
same function and closure object, but different preference names or prefixes.
Since our callback matching is nothing more complicated than iteration over
all of our registered callbacks, there's nothing that prevents us from
combining all of these into a single callback, with an array of preferences
rather than a single string.
And since we already allocate an extra 8 bytes for each callback object, we
can add a variant tag without increasing our allocation size.
MozReview-Commit-ID: I497lWfMUp3
--HG--
extra : rebase_source : bac374d9a590c325728551d1669ebc6ef8ca3884
This adds an additional file descriptor to the set sent at content process
startup, for the read-only preference map that we share between all content
processes. This forms the base set of preferences. The other preferences FD
contains changes that the content process will need to apply on top of the
snapshot.
MozReview-Commit-ID: 6hc0HIxFmHg
--HG--
extra : intermediate-source : 46a6f9d0773ba2d756d8801cee79bfa994165d44
extra : absorb_source : a725a095946946797fd4f3abe6461e810b15d899
extra : source : e3bbc87b71af2f2ce1fa8bdf2cf26857c071ba5e
extra : histedit_source : 958212e804df7f9e1be6af182e73edc956b9a576
This is based on the SharedStringMap that's currently used for shared memory
string bundles.
When the parent process is ready to launch its first content process, it
creates a snapshot of the current state of the preference database, maps that
as read-only, and shares it with each content process. Look-ups in the
snapshotted map are done entirely using data in the shared memory region. It
doesn't require any additional per-process state data.
MozReview-Commit-ID: BdTUhak7dmS
--HG--
extra : intermediate-source : 434106f1b75e3ba900912f261bd22a1b7f5c931d
extra : absorb_source : 647ad37590448ad3c1eb8eb512bf671f262fa96e
extra : source : 68bb03c63b3cee1d47cbddfd3abf919f5783c04b
extra : histedit_source : 2228a9f8395929f5072a3c5ebda6ae3221e4a62d
This adds an additional file descriptor to the set sent at content process
startup, for the read-only preference map that we share between all content
processes. This forms the base set of preferences. The other preferences FD
contains changes that the content process will need to apply on top of the
snapshot.
MozReview-Commit-ID: 6hc0HIxFmHg
--HG--
extra : source : e3bbc87b71af2f2ce1fa8bdf2cf26857c071ba5e
extra : histedit_source : dc1c7c8015e0443eed218f826fda9f5b38b3e062%2C4cfa2f90104ca3e907607d884ac86f73ad4995bf
This is based on the SharedStringMap that's currently used for shared memory
string bundles.
When the parent process is ready to launch its first content process, it
creates a snapshot of the current state of the preference database, maps that
as read-only, and shares it with each content process. Look-ups in the
snapshotted map are done entirely using data in the shared memory region. It
doesn't require any additional per-process state data.
MozReview-Commit-ID: BdTUhak7dmS
--HG--
extra : source : 68bb03c63b3cee1d47cbddfd3abf919f5783c04b
This adds an additional file descriptor to the set sent at content process
startup, for the read-only preference map that we share between all content
processes. This forms the base set of preferences. The other preferences FD
contains changes that the content process will need to apply on top of the
snapshot.
MozReview-Commit-ID: 6hc0HIxFmHg
--HG--
extra : rebase_source : f5de7462438e33cf4983be3fcef3c781c25ec492
extra : absorb_source : 9f9861546fc0b11333e575bb3c164dfe07575d99
This is based on the SharedStringMap that's currently used for shared memory
string bundles.
When the parent process is ready to launch its first content process, it
creates a snapshot of the current state of the preference database, maps that
as read-only, and shares it with each content process. Look-ups in the
snapshotted map are done entirely using data in the shared memory region. It
doesn't require any additional per-process state data.
MozReview-Commit-ID: BdTUhak7dmS
--HG--
extra : rebase_source : e7cb96dd52380f2ed2fbd79b4e157e4efab65cb0
extra : absorb_source : ed95ed85388875353458eb65e41727e606ebf097
Most preference callbacks use literal strings for their domain filters, which
means that there's no need to make copies of them at all. Currently, however,
every preference observer node makes a separate heap-allocated copy of its
domain string.
This patch switches the domain string storage to nsCString instances, which
dramatically reduces the amount of unnecessary copies, at the expense of
making the callback nodes slightly larger.
MozReview-Commit-ID: 8NA3t2JS2UI
--HG--
extra : rebase_source : 628ad9af65cec16fb8be0c8dddc608b5ee5602e2
Currently VarCache prefs are setup in two parts:
- The vanilla pref part, installed via a data file such as all.js, or via an
API call.
- The VarCache variable part, setup by an Add*VarCache() call.
Both parts are needed for the pref to actually operate as a proper VarCache
pref. (There are various prefs for which we do one but not the other unless a
certain condition is met.)
This patch introduces a new way of doing things. There is a new file,
modules/libpref/init/StaticPrefList.h, which defines prefs like this:
> VARCACHE_PREF(
> "layout.accessiblecaret.width",
> layout_accessiblecaret_width,
> float, 34.0
> )
This replaces both the existing parts.
The preprocessor is used to generate multiple things from this single
definition:
- A global variable (the VarCache itself).
- A getter for that global variable.
- A call to an init function that unconditionally installs the pref in the
prefs hash table at startup.
C++ files can include the new StaticPrefs.h file to access the getter.
Rust code cannot use the getter, but can access the global variable directly
via structs.rs. This is similar to how things currently work for Rust code.
Non-VarCache prefs can also be declared in StaticPrefList.h, using PREF instead
of the VARCACHE_PREF.
The new approach has the following advantages.
+ It eliminates the duplication (in all.js and the Add*VarCache() call) of the
pref name and default value, preventing potential mismatches. (This is a real
problem in practice!)
+ There is now a single initialization point for these VarCache prefs.
+ This avoids need to find a place to insert the Add*VarCache() calls, which
are currently spread all over the place.
+ It also eliminates the common pattern whereby these calls are wrapped in a
execute-once block protected by a static boolean (see bug 1346224).
+ It's no longer possible to have a VarCache pref for which only one of the
pieces has been setup.
+ It encapsulates the VarCache global variable, so there is no need to declare
it separately.
+ VarCache reads are done via a getter (e.g. StaticPrefs::foo_bar_baz())
instead of a raw global variable read.
+ This makes it clearer that you're reading a pref value, and easier to
search for uses.
+ This prevents accidental writes to the global variable.
+ This prevents accidental mistyping of the pref name.
+ This provides a single chokepoint in the code for such accesses, which make
adding checking and instrumentation feasible.
+ It subsumes MediaPrefs, and will allow that class to be removed. (gfxPrefs is
a harder lift, unfortunately.)
+ Once all VarCache prefs are migrated to the new approach, the VarCache
mechanism will be better encapsulated, with fewer details publicly visible.
+ (Future work) This will allow the pref names to be stored statically, saving
memory in every process.
The main downside of the new approach is that all of these prefs are in a
single header that is included in quite a few places, so any changes to this
header will cause a fair amount of recompilation.
Another minor downside is that all VarCache prefs are defined and visible from
start-up. For test-only prefs like network.predictor.doing-tests, having them
show in about:config isn't particularly useful.
The patch also moves three network VarCache prefs to the new mechanism as a
basic demonstration. (And note the inconsistencies in the multiple initial
values that were provided for
network.auth.subresource-img-cross-origin-http-auth-allow!) There will be
numerous follow-up bugs to convert the remaining VarCache prefs.
MozReview-Commit-ID: 9ABNpOR16uW
* * *
[mq]: fixup
MozReview-Commit-ID: 6ToT9dQjIAq
All pref-modifying operations now only occur in the parent process. Hooray!
MozReview-Commit-ID: GDVsda4rw5f
--HG--
extra : rebase_source : 4f8484f0751212120078b3ba1a32930bc9c5ed8a
All prefs that need to be sent to a new content process are now put into the
shared memory segment, and they are identified by the pref name instead of an
index into a list. The old IPC used at process startup (in XPCOMInitData) is
removed.
Benefits:
- It removes the need for the early prefs list
(dom/ipc/ContentProcesses.{h,cpp}) and the associated checking, which is ugly
and often trips people up (e.g. bug 1432979, bug 1439406).
- Using prefnames instead of indices fixes some fragility (fixing bug 1419432).
- It fixes the problem of early prefs being installed as unlocked default
values even if they are locked and/or have user values.
MozReview-Commit-ID: FRIzHF8Tjd
This patch doesn't change the functionality, it just splits out the code into
separate functions that are easier to read.
MozReview-Commit-ID: Gx05YCxGgve
--HG--
extra : rebase_source : 3b7250cea630bebf35992bb69e651509c863c1c6
This patch replaces the large -intPrefs/-boolPrefs/-stringPrefs flags with
a short-lived, anonymous, shared memory segment that is used to pass the early
prefs.
Removing the bloat from the command line is nice, but more important is the
fact that this will let us pass more prefs at content process start-up, which
will allow us to remove the early/late prefs split (bug 1436911).
Although this mechanism is only used for prefs, it's conceivable that it could
be used for other data that must be received very early by children, and for
which the command line isn't ideal.
Notable details:
- Much of the patch deals with the various platform-specific ways of passing
handles/fds to children.
- Linux and Mac: we use a fixed fd (8) in combination with the new
GeckoChildProcessHost::AddFdToRemap() function (which ensures the child
won't close the fd).
- Android: like Linux and Mac, but the handles get passed via "parcels" and
we use the new SetPrefsFd() function instead of the fixed fd.
- Windows: there is no need to duplicate the handle because Windows handles
are system-wide. But we do use the new
GeckoChildProcessHost::AddHandleToShare() function to add it to the list of
inheritable handles. We also ensure that list is processed on all paths
(MOZ_SANDBOX with sandbox, MOZ_SANDBOX without sandbox, non-MOZ_SANDBOX) so
that the handles are marked as inheritable. The handle is passed via the
-prefsHandle flag.
The -prefsLen flag is used on all platforms to indicate the size of the
shared memory segment.
- The patch also moves the serialization/deserialization of the prefs in/out of
the shared memory into libpref, which is a better spot for it. (This means
Preferences::MustSendToContentProcesses() can be removed.)
MozReview-Commit-ID: 8fREEBiYFvc
--HG--
extra : rebase_source : 7e4c8ebdbcd7d74d6bd2ab3c9e75a6a17dbd8dfe
The meaning of "possibly-changed" is provided by the big comment above
MustSendToContentProcesses.
On a new profile this reduces the number of prefs sent like so:
- Command-line: 222 --> 3
- IPC: 3129 --> 130
On an older profile:
- Command-line: 222 --> 3
- IPC: 3165 --> 180
MozReview-Commit-ID: DcgedhXhZd8
--HG--
extra : rebase_source : acef424fab5031347cbcbd5c3e6a24ee66895ef9
They currently fail to pass on `aKind`, always getting the user value (when
possible). There are three callsites that are affected:
- nsSHistory::Startup, docshell/shistory/nsSHistory.cpp.
- FeatureState::SetDefaultFromPref(), in gfx/config/gfxFeature.cpp.
- gfxPlatform::InitOMTPConfig(), in gfx/thebes/gfxPlatform.cpp.
The patch also adds a gtest that would have failed prior to the fix.
MozReview-Commit-ID: L0U1XQmPUFc
--HG--
extra : rebase_source : d51d09836609c5a45d0b9f20570427681d8b3309
This patch rearranges these functions so that nsPrefBranch::GetPrefType() calls
into Preferences::GetType(), because all other nsPrefBranch methods depend on
Preferences methods.
The patch also removes the `aKind` argument from GetType(), because it has no
effect -- a pref only has one type, regardless of whether it has a default
value, a user value, or both.
MozReview-Commit-ID: J3vxFPaP8S3
The prefs parser has two significant problems.
- It doesn't separate tokenizing from parsing.
- It is implemented as a loop around a big switch on a "current state"
variable.
As a result, it is hard to understand and modify, slower than it could be, and
in obscure cases (involving comments and whitespace) it fails to parse what
should be valid input.
This patch replaces it with a recursive descent parser (albeit one without any
recursion!) that has separate tokenization. The new parser is easier to
understand and modify, more correct, and has better error messages. It doesn't
do error recovery, but that would be much easier to add than in the old parser.
The new parser also runs about 1.9x faster than the existing parser. (As
measured by parsing greprefs.js's contents from memory 1000 times in
succession, omitting the prefs hash table construction. If the table
construction is included, it's about 1.6x faster.)
The new parser is slightly stricter than the old parser in a few ways.
- Disconcertingly, the old parser allowed arbitrary junk between prefs
(including at the start and end of the prefs file) so long as that junk
didn't include any of the following chars: '/', '#', 'u', 's', 'p'. I.e.
lines like these:
!foo@bar&pref("prefname", true);
ticky_pref("prefname", true); // missing 's' at start
User_pref("prefname", true); // should be 'u' at start
would all be treated the same as this:
pref("prefname", true);
The new parser disallows such junk because it isn't necessary and seems like
an unintentional botch by the old parser.
- The old parser allowed character 0x1a (SUB) between tokens and treated it
like '\n'.
The new parser does not allow this character. SUB was used to indicate
end-of-file (*not* end-of-line) in some old operating systems such as MS-DOS,
but this doesn't seem necessary today.
- The old parser tolerated (with a warning) invalid escape sequences within
string literals -- such as "\q" (not a valid escape) and "\x1" and "\u12"
(both of which have insufficient hex digits) -- accepting them literally.
The new parser does not tolerate invalid escape sequences because it doesn't
seem necessary and would complicate things.
- The old parser tolerated character 0x00 (NUL) within string literals; this is
dangerous because C++ code that manipulates string values with embedded NULs
will almost certainly consider those chars as end-of-string markers.
The new parser treats NUL chars as end-of-file, to avoid this danger and
because it facilitates a significant optimization (described within the
code).
- The old parser allowed integer literals to overflow, silently wrapping them.
The new parser treats integer overflow as a parse error. This seems better,
and it caught existing overflows of places.database.lastMaintenance, in
testing/profiles/prefs_general.js (bug 1424030) and
testing/talos/talos/config.py (bug 1434813).
The first of these changes meant that a couple of existing prefs with ";;" at
the end had to be changed (done in the preceding patch).
The minor increase in strictness shouldn't be a problem for default pref files
such as greprefs.js within the application (which we can modify), nor for
app-written prefs files such as prefs.js. It could affect user-written prefs
files such as user.js; the experience above suggests that integer overflow and
";;" are the most likely problems in practice. In my opinion, the risk here is
acceptable.
The new parser also does a better job of tracking line numbers because it (a)
treats "\r\n" sequences as a single end-of-line marker, and (a) pays attention
to end-of-line sequences within string literals.
Finally, the patch adds thorough tests of both valid and invalid syntax.
MozReview-Commit-ID: JD3beOQl4AJ
The prefs parser has two significant problems.
- It doesn't separate tokenizing from parsing.
- It is implemented as a loop around a big switch on a "current state"
variable.
As a result, it is hard to understand and modify, slower than it could be, and
in obscure cases (involving comments and whitespace) it fails to parse what
should be valid input.
This patch replaces it with a recursive descent parser (albeit one without any
recursion!) that has separate tokenization. The new parser is easier to
understand and modify, more correct, and has better error messages. It doesn't
do error recovery, but that would be much easier to add than in the old parser.
The new parser also runs about 1.9x faster than the existing parser. (As
measured by parsing greprefs.js's contents from memory 1000 times in
succession, omitting the prefs hash table construction. If the table
construction is included, it's about 1.6x faster.)
The new parser is slightly stricter than the old parser in a few ways.
- Disconcertingly, the old parser allowed arbitrary junk between prefs
(including at the start and end of the prefs file) so long as that junk
didn't include any of the following chars: '/', '#', 'u', 's', 'p'. I.e.
lines like these:
!foo@bar&pref("prefname", true);
ticky_pref("prefname", true); // missing 's' at start
User_pref("prefname", true); // should be 'u' at start
would all be treated the same as this:
pref("prefname", true);
The new parser disallows such junk because it isn't necessary and seems like
an unintentional botch by the old parser.
- The old parser allowed character 0x1a (SUB) between tokens and treated it
like '\n'.
The new parser does not allow this character. SUB was used to indicate
end-of-file (*not* end-of-line) in some old operating systems such as MS-DOS,
but this doesn't seem necessary today.
- The old parser tolerated (with a warning) invalid escape sequences within
string literals -- such as "\q" (not a valid escape) and "\x1" and "\u12"
(both of which have insufficient hex digits) -- accepting them literally.
The new parser does not tolerate invalid escape sequences because it doesn't
seem necessary and would complicate things.
- The old parser tolerated character 0x00 (NUL) within string literals; this is
dangerous because C++ code that manipulates string values with embedded NULs
will almost certainly consider those chars as end-of-string markers.
The new parser treats NUL chars as end-of-file, to avoid this danger and
because it facilitates a significant optimization (described within the
code).
- The old parser allowed integer literals to overflow, silently wrapping them.
The new parser treats integer overflow as a parse error. This seems better,
and it caught an existing overflow in testing/profiles/prefs_general.js, for
places.database.lastMaintenance (see bug 1424030).
The first of these changes meant that a couple of existing prefs with ";;" at
the end had to be changed (done in the preceding patch).
The minor increase in strictness shouldn't be a problem for default pref files
such as greprefs.js within the application (which we can modify), nor for
app-written prefs files such as prefs.js. It could affect user-written prefs
files such as user.js; the experience above suggests that ";;" is the most
likely problem in practice. In my opinion, the risk here is acceptable.
The new parser also does a better job of tracking line numbers because it (a)
treats "\r\n" sequences as a single end-of-line marker, and (a) pays attention
to end-of-line sequences within string literals.
Finally, the patch adds thorough tests of both valid and invalid syntax.
MozReview-Commit-ID: 8EYWH7KxGG
* * *
[mq]: win-fix
MozReview-Commit-ID: 91Bxjfghqfw
--HG--
extra : rebase_source : a8773413e5d68c33e4329df6819b6e1f82c22b85
This code is used to detect too-early accesses of prefs in content processes.
The patch makes the following changes.
- New terminology: "early" prefs are those sent via the command line; "late"
prefs are those sent via IPC. Previously the former were "init" prefs and the
latter didn't have a clear name.
- The phase tracking and checking is now almost completely encapsulated within
Preferences.cpp. The only exposure to outside code is via the
AreAllPrefsSetInContentProcess() method, which has a single use.
- The number of states tracked drops from 5 to 3. There's no need to track the
beginning of the pref-setting operations, because we only need to know if
they've finished. (This also avoids the weirdness where we could transition
from END_INIT_PREFS back to BEGIN_INIT_PREFS because of the way -intPrefs,
-boolPrefs and -stringPrefs were parsed separately.)
MozReview-Commit-ID: IVJWiDxdsDV
--HG--
extra : rebase_source : 8cee1dcbd40847bf052ca9e2b759dd550350e5a1
Preferences::SetPreference() is used when setting prefs in the content process
from dom::Pref values that are passed from the parent process. Currently we
use the high-level Set*InAnyProcess() methods to do this -- basically the same
code path as sets done via the API -- but this has several problems.
- It is subtly broken. If the content process already has a pref of type T with
a default value and then we get a SetPreference() call that tries to change
it to type U, it will erroneously fail. In practice this never(?) happens,
but it shows that things aren't arranged very well.
- SetPreference() also looks up the hashtable twice to get the same pref if
both a default value and a user value are present in the dom::Pref.
- This happens in content processes, while all other pref set operations occur
in the parent process. This is the main reason we have the Set*InAnyProcess()
functions.
In short, the setting of prefs via IPC is quite different to the setting of
prefs via other means -- because it happens in content processes, and it's more
of a clobber that can set both values at once -- and so should be handled
differently.
The solution is to make Preferences::SetPreference() use lower-level operations
to do the update. It now does the hash table lookup/add itself, and then calls
the new Pref::FromDomPref() method. That method then possibly calls the new
PrefValue::FromDomPrefValue() method for both kinds of value, and overwrites
them as necessary. SetValueFromDom() is no longer used and the patch removes it.
MozReview-Commit-ID: 2Rg8VMOc0Cl
--HG--
extra : rebase_source : 0eddc3a4b694a79af3e173aefa7758f8e2ae776b
It represents a pref, so `Pref` is a better name. Within Preferences.cpp the
patch uses domPref/aDomPref to distinguish it from PrefHashEntry values.
MozReview-Commit-ID: HXTl0GX4BtO
--HG--
extra : rebase_source : c1e0726c55e7577720f669f0ed2dbc38627d853e
This requires adding an aPriority argument (defaulting to false) to
Preferences::RegisterCallback(). And RegisterVarCacheCallback() is no longer
necessary.
MozReview-Commit-ID: BMDk3HuaQVV
--HG--
extra : rebase_source : 17a61cfd9a82f24854162fc993223691041ea46d
Jean-Yves Avenard
Brindusan Cristian
Gurzau Raul
Kris Maglione
arthur.iakab
Ehsan Akhgari
Sylvestre Ledru
Gabriele Svelto
Bogdan Tara
Nicholas Nethercote
Cosmin Sabou