Closed
Bug 1001137
Opened 10 years ago
Closed 10 years ago
Enabling FifoWatcher via user pref does not work
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: erahm, Assigned: erahm)
References
Details
(Whiteboard: [MemShrink])
Attachments
(1 file)
4.74 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
We have two problems here: 1) Setting |memory_info_dumper.watch_fifo.enabled| to true via about:config has no effect. After tracing through it looks like the user pref doesn't get loaded until after nsDumpUtils is initialized. B2G seems to get around this by overriding the default value in it's build config (rather than using a user pref). 2) The default value libpref/src/init/all.js is actually set for |memory_info_dumper.watch_fifo|, it should be |memory_info_dumper.watch_fifo.enabled|
Assignee | ||
Comment 1•10 years ago
|
||
This patch renames the default pref to match what our code expects and registers an observer for the pref to handle the case where user prefs are loaded after initialization has taken place.
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8412206 [details] [diff] [review] Enabling FifoWatcher via user pref does not work Nathan can you take a look at this?
Attachment #8412206 -
Flags: review?(nfroyd)
Assignee | ||
Comment 3•10 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=fc56986c51b0
Comment 4•10 years ago
|
||
Comment on attachment 8412206 [details] [diff] [review] Enabling FifoWatcher via user pref does not work Review of attachment 8412206 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/base/nsMemoryInfoDumper.cpp @@ +169,5 @@ > } > > +bool SetupFifo() > +{ > + if (FifoWatcher::MaybeCreate()) { Since this is now a separate function that people might call from other places, WDYT about: - Inverting the logic so all the callback registration doesn't happen under the if, but in the main control flow of the function? - Adding MOZ_ASSERT or similar to enforce that we only run the callback registrations once? @@ +193,5 @@ > +{ > + LOG("memory_info_dumper.watch_fifo.enabled changed"); > + if (SetupFifo()) { > + Preferences::UnregisterCallback(OnFifoEnabledChange, > + "memory_info_dumper.watch_fifo.enabled", Maybe pull this out into a static const char[] somewhere and use it where you register the callback too?
Attachment #8412206 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Reordered logic, added assert on double-registering and made the pref name a static in FifoWatcher. remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f628c9d48728
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f628c9d48728 https://hg.mozilla.org/mozilla-central/rev/950fadd70f9e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•