Closed
Bug 1201394
Opened 9 years ago
Closed 9 years ago
ProcessPriorityManager should use Prefs cache
Categories
(Core :: DOM: Content Processes, defect)
Core
DOM: Content Processes
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: kanru, Assigned: kanru)
Details
Attachments
(2 files)
13.57 KB,
patch
|
gsvelto
:
review+
|
Details | Diff | Splinter Review |
1.87 KB,
patch
|
gsvelto
:
review+
|
Details | Diff | Splinter Review |
To avoid having to call the static Preferences API and automatically get updated value when pref changes. I found this when I wrote a test that uses ProcessPriorityManager test mode but didn't set any LRUPoolLevels, which broke the test_BackgroundLRU.html test because prefs were only read once.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8656981 -
Flags: review?(gsvelto)
Comment 3•9 years ago
|
||
Comment on attachment 8656981 [details] [diff] [review] Remove unused mLRUPoolSize member variable. LGTM, I'll have a look at the other patch ASAP.
Attachment #8656981 -
Flags: review?(gsvelto) → review+
Comment 4•9 years ago
|
||
Comment on attachment 8656979 [details] [diff] [review] Use cached preferences value in ProcessPriorityManager Review of attachment 8656979 [details] [diff] [review]: ----------------------------------------------------------------- I like these changes quite a bit, thanks for doing this! ::: dom/ipc/ProcessPriorityManager.cpp @@ +204,5 @@ > */ > void Unfreeze(); > > + /** > + * Call ShutDown before destroy ProcessPriorityManager. nit: "Call ShutDown before destroying the ProcessPriorityManager", it would also be nice to describe why this is needed. @@ +392,5 @@ > void* aClosure) > { > StaticInit(); > + if (!PrefsEnabled() && sSingleton) { > + sSingleton->ShutDown(); Do we really need to call ShutDown() explicitly? Doesn't setting the StaticRefPtr<> to nullptr do that for us already?
Attachment #8656979 -
Flags: review?(gsvelto) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Gabriele Svelto [:gsvelto] from comment #4) > Comment on attachment 8656979 [details] [diff] [review] > Use cached preferences value in ProcessPriorityManager > > Review of attachment 8656979 [details] [diff] [review]: > ----------------------------------------------------------------- > > I like these changes quite a bit, thanks for doing this! > > ::: dom/ipc/ProcessPriorityManager.cpp > @@ +204,5 @@ > > */ > > void Unfreeze(); > > > > + /** > > + * Call ShutDown before destroy ProcessPriorityManager. > > nit: "Call ShutDown before destroying the ProcessPriorityManager", it would > also be nice to describe why this is needed. > > @@ +392,5 @@ > > void* aClosure) > > { > > StaticInit(); > > + if (!PrefsEnabled() && sSingleton) { > > + sSingleton->ShutDown(); > > Do we really need to call ShutDown() explicitly? Doesn't setting the > StaticRefPtr<> to nullptr do that for us already? It's needed because WakeLockObserver holds a strong reference to us.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5047a4f41b4 https://hg.mozilla.org/integration/mozilla-inbound/rev/16a9a6385720
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f5047a4f41b4 https://hg.mozilla.org/mozilla-central/rev/16a9a6385720
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•