1421541 - Still more libpref refactorings

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Tweak some comments.

This includes removing a bunch of low-value ones.

Review commit: https://reviewboard.mozilla.org/r/203808/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203808/

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Make Preferences::HandleDirty() only work in the parent process.

pref_SetPref() is now the only function that runs in the content process and
calls HandleDirty(). So this patch moves the parent process check out of
HandleDirty() into pref_SetPref().

The patch also adds assertions to a couple of other parent-process-only
functions.

Review commit: https://reviewboard.mozilla.org/r/203810/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203810/

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Tweak GetCStringValue().

It's not possible for a string value to be nullptr.

Review commit: https://reviewboard.mozilla.org/r/203812/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203812/

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Inline and remove pref_LoadPrefsInDirList().

It has only one call site.

Review commit: https://reviewboard.mozilla.org/r/203814/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203814/

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Split Pref::SetValue() in two.

The default path and the user path are entirely disjoint, and some of the
arguments only apply to one of the paths, so splitting it into two functions
makes things a bit clearer. The aForceSet arg is also renamed aFromFile.

Review commit: https://reviewboard.mozilla.org/r/203816/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203816/

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: Bug 1421541 - Overhaul the prefs phase code.

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.)

Review commit: https://reviewboard.mozilla.org/r/203818/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/203818/

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8932759 [details]
Bug 1421541 - Tweak some comments.

https://reviewboard.mozilla.org/r/203808/#review210344

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8932760 [details]
Bug 1421541 - Make Preferences::HandleDirty() only work in the parent process.

https://reviewboard.mozilla.org/r/203810/#review210346

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8932761 [details]
Bug 1421541 - Tweak GetCStringValue().

https://reviewboard.mozilla.org/r/203812/#review210348

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8932762 [details]
Bug 1421541 - Inline and remove pref_LoadPrefsInDirList().

https://reviewboard.mozilla.org/r/203814/#review210350

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8932763 [details]
Bug 1421541 - Split Pref::SetValue() in two.

https://reviewboard.mozilla.org/r/203816/#review210352

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8932764 [details]
Bug 1421541 - Overhaul the prefs phase code.

https://reviewboard.mozilla.org/r/203818/#review210354

Comment 13

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43598bb5fe1ccf86d51229514db0e309a1d1aaa8
Bug 1421541 - Tweak some comments. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/0947c00a401e2404de97d5cdecce35dc0a6559a4
Bug 1421541 - Make Preferences::HandleDirty() only work in the parent process. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/c7c17d849b96792bad3ebd2c55ab923c51d4a0bb
Bug 1421541 - Tweak GetCStringValue(). r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e5c582b5f3e3303224795fab65130e8c915e441
Bug 1421541 - Inline and remove pref_LoadPrefsInDirList(). r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a6d4c45913ab577bf9ce2434efaf7e2ff26d359
Bug 1421541 - Split Pref::SetValue() in two. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d130a12ec98d05d0d1bdc10c135eb5510a3ab72
Bug 1421541 - Overhaul the prefs phase code. r=glandium

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/43598bb5fe1c
https://hg.mozilla.org/mozilla-central/rev/0947c00a401e
https://hg.mozilla.org/mozilla-central/rev/c7c17d849b96
https://hg.mozilla.org/mozilla-central/rev/1e5c582b5f3e
https://hg.mozilla.org/mozilla-central/rev/4a6d4c45913a
https://hg.mozilla.org/mozilla-central/rev/8d130a12ec98