1416613 - Three small libpref tweaks

Status: RESOLVED FIXED

(Core :: Preferences: Backend, enhancement)

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Bug 1416613 - Improve comments about pref IPC.

These would have saved me some time last week when I was figuring out how this
stuff works.

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

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: Bug 1416613 - Make Preferences::InitStaticMembers() slightly faster.

InitStaticMembers() is the hottest function within libpref, because it is
called from all the getters and setters. This patch tweaks it so the common
case is handled with a single comparison, instead of two. This is valid because
sPreferences is nulled at the same time that sShutdown is set.

The patch also tweaks some other conditions to take advantage of this fact.

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

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1416613 - Replace use of std::string and std::vector with Mozilla equivalents.

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

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 8927709 [details]
Bug 1416613 - Replace use of std::string and std::vector with Mozilla equivalents.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198982/diff/1-2/

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8927701 [details]
Bug 1416613 - Improve comments about pref IPC.

https://reviewboard.mozilla.org/r/198974/#review204404

I'm wondering if the pref handling code from dom/ipc shouldn't be in modules/libpref.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8927702 [details]
Bug 1416613 - Make Preferences::InitStaticMembers() slightly faster.

https://reviewboard.mozilla.org/r/198976/#review204406

Technically speaking, sShutdown and sPreferences are not guaranteed to be set to resp. true and nullptr in Preferences::Shutdown in the order it's written there. But the API, as of writing, is main thread only (and InitStaticMembers asserts it is), we can be lenient for now.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8927709 [details]
Bug 1416613 - Replace use of std::string and std::vector with Mozilla equivalents.

https://reviewboard.mozilla.org/r/198982/#review204408

Comment 8

[Nicholas Nethercote [inactive]]

> I'm wondering if the pref handling code from dom/ipc shouldn't be in
> modules/libpref.

Possibly. I suspect it's not because of the big "a DOM peer must review additions to this list" warning in ContentPrefs.cpp.

Comment 9

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea1df4688f4ba7c6f16292d6758201c0862e74bc
Bug 1416613 - Improve comments about pref IPC. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef86c22e6f1fecc38d80fd6144fcef4f8828982b
Bug 1416613 - Make Preferences::InitStaticMembers() slightly faster. r=glandium

https://hg.mozilla.org/integration/mozilla-inbound/rev/e281e797c85d6c24961f504c6f27e364210bb0a7
Bug 1416613 - Replace use of std::string and std::vector with Mozilla equivalents. r=glandium

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/ea1df4688f4b
https://hg.mozilla.org/mozilla-central/rev/ef86c22e6f1f
https://hg.mozilla.org/mozilla-central/rev/e281e797c85d

Comment 11

[Gerry Chang [:gchang]]

Won't fix for 58. Let it ride the train.