Closed Bug 1201394 Opened 10 years ago Closed 10 years ago

ProcessPriorityManager should use Prefs cache

Categories

(Core :: DOM: Content Processes, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: kanru, Assigned: kanru)

Details

Attachments

(2 files)

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: nobody → kchen
Status: NEW → ASSIGNED
Attachment #8656979 - Flags: review?(gsvelto)
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 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+
(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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: