Closed Bug 844970 Opened 8 years ago Closed 8 years ago

Give a grace period for all process-priority decreases

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

RESOLVED FIXED
B2G C4 (2jan on)
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

Details

Attachments

(1 file)

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.
Blocks: 844323
Assignee: nobody → justin.lebar+bug
Attached patch Patch, v1Splinter Review
Attachment #718073 - Flags: review?(jones.chris.g)
Comment on attachment 718073 [details] [diff] [review]
Patch, v1

Nice change, lgtm.
Attachment #718073 - Flags: review?(jones.chris.g) → review+
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
Attachment #718073 - Flags: approval-mozilla-b2g18?
https://hg.mozilla.org/mozilla-central/rev/d5cba11c8204
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(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.
tracking-b2g18: --- → +
Attachment #718073 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
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.)
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
You need to log in before you can comment on or make changes to this bug.