DecodePool::NotifyDecodeComplete() calls itself endlessly when async notifications are requested

RESOLVED FIXED in Firefox 37

Status

()

Core
ImageLib
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jerry, Assigned: seth)

Tracking

unspecified
mozilla38
Points:
---

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed)

Details

Attachments

(1 attachment)

When the flame device is just booted(using adb reboot) and then just scroll down for a small distance and click clock app, I still can see homescreen spend 50% cpu time in top tool.

Here is the profiler result:
http://people.mozilla.org/~bgirard/cleopatra/#report=a8710c69c7f84ec05cea8f7f9eab147228a12fa3&filter=%5B{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A8157,%22end%22%3A9219},{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A8425,%22end%22%3A8516}%5D

DecodePool::NotifyDecodeComplete() [1] is always called even though I turn off screen or switch to another app.

[1]
https://hg.mozilla.org/mozilla-central/annotate/3cda3997f45d/image/src/DecodePool.cpp#l369
Seth, do you know why we always schedule a new task for this?
Flags: needinfo?(seth)
(Assignee)

Comment 2

3 years ago
Yep, looks like a bug introduced by bug 1128223. It's an easy fix.
Flags: needinfo?(seth)
(Assignee)

Updated

3 years ago
Summary: DecodePool::NotifyDecodeComplete() is still be called in homescreen app → DecodePool::NotifyDecodeComplete() calls itself endlessly when async notifications are requested
(Assignee)

Updated

3 years ago
Blocks: 1128223
(Assignee)

Comment 3

3 years ago
Created attachment 8559717 [details] [diff] [review]
Don't call back into DecodePool from NotifyDecodeCompleteWorker

Here's the fix.
Attachment #8559717 - Flags: review?(tnikkel)
(Assignee)

Updated

3 years ago
Assignee: nobody → seth
Status: NEW → ASSIGNED
Attachment #8559717 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 5

3 years ago
Thanks for the quick review!
(Assignee)

Comment 6

3 years ago
Requesting checkin for this because it's pretty important, but I'm too tired to stay up anymore and the try job is still running.
Keywords: checkin-needed
(In reply to Seth Fowler [:seth] from comment #6)
> Requesting checkin for this because it's pretty important, but I'm too tired
> to stay up anymore and the try job is still running.

np :) done, good night :)
Duplicate of this bug: 1129980
https://hg.mozilla.org/mozilla-central/rev/8be1b5385390
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

Comment 11

3 years ago
This is not Gonk only, is it?
Nope, I was seeing it on Mac as well, which I filed bug 1129980 about.
OS: Gonk (Firefox OS) → All
Hardware: ARM → All
(Assignee)

Comment 13

3 years ago
Comment on attachment 8559717 [details] [diff] [review]
Don't call back into DecodePool from NotifyDecodeCompleteWorker

Approval Request Comment
[Feature/regressing bug #]: Part of fix for bug 1125272 and bug 1119938.
[User impact if declined]: Already approved.
[Describe test coverage new/current, TreeHerder]: In 38 for a long time.
[Risks and why]: Low risk; in 38 for a long time.
[String/UUID change made/needed]: None.
Attachment #8559717 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 14

3 years ago
Pushed to Aurora:

https://hg.mozilla.org/releases/mozilla-aurora/rev/604a1b7d114a
status-firefox37: --- → fixed
Comment on attachment 8559717 [details] [diff] [review]
Don't call back into DecodePool from NotifyDecodeCompleteWorker

This is part of a collection of ImageLib fixes that have been tested by QE and that Seth had approval to land. Adding approval to the bug after the fact.
Attachment #8559717 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.