844970 - Give a grace period for all process-priority decreases

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

Right now we give a grace period for all process-priority downgrades /except/ priority downgrades between two BACKGROUND* priorities.

This is unnecessarily inconsistent.  But it's also problematic because when e.g. a music player app switches tracks, there's likely to be a brief pause, and we don't want to downgrade its priority at that time.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 718073 [details] [diff] [review]
Patch, v1

Nice change, lgtm.

Comment 3

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5cba11c8204

Comment 4

[Justin Lebar (not reading bugmail)]

Comment on attachment 718073 [details] [diff] [review]
Patch, v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a

User impact if declined: Audio player can die when switching tracks.

Testing completed: I'm not sure this field applies to b2g18 requests, given the frequency with which we're landing on that branch...

Risk to taking this patch (and alternatives if risky): This is the least-risky way I know of to fix this bug.  Due to a separate bug in the audio service, we were never downgrading processes's priorities after they paused their audio, so things were "working" before this patch only by accident.

String or UUID changes made by this patch: n/a

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d5cba11c8204

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Justin Lebar [:jlebar] from comment #4)
> Testing completed: I'm not sure this field applies to b2g18 requests, given
> the frequency with which we're landing on that branch...

Local/QA testing, especially in the case of a risky bug.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/fb5dd4d89b7e

Comment 8

[Justin Lebar (not reading bugmail)]

FWIW we effectively have to take this on 1.0.1 for bug 844323, which is tef+.  I'm going try to merge the changes in to the uplift in that bug, so there won't be a separate commit.  (Essentially, I'm just going to force ProcessPriorityManager.cpp on b2g18_1.0.1 to match ProcessPriorityManager.cpp on b2g18.)

Comment 9

[Justin Lebar (not reading bugmail)]

Actually, I take that back: It was better to explicitly land this on 1.0.1.  See the commit message for an explanation.

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/e57b5de3f8a6