Images disappearing/remaining blank

RESOLVED FIXED

Status

()

Core
ImageLib
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cwiiis, Assigned: Joe Drew (not getting mail))

Tracking

({regression})

Trunk
ARM
Gonk (Firefox OS)
regression
Points:
---

Firefox Tracking Flags

(blocking-b2g:tef+, firefox23 unaffected, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
There appear to be various invalidation issues where things aren't invalidated when they should be in v1-train. v1.0.1 is unaffected.

STR:

1- Turn volume to max
2- Wait for overlay to disappear
3- Press volume-up key
4- Notice missing speaker/vibrate icons

I've seen this in various other places, this just happens to be the easiest place to reproduce it.

I have noticed invalidation issues in v1.0.1, but nothing quite as obvious or reproduceable as this. I've verified this using v1-train.

Updated

5 years ago
blocking-b2g: leo? → leo+
Keywords: regression, regressionwindow-wanted
Can we please clarify which device this was tested on?
(Reporter)

Comment 2

5 years ago
(In reply to Marcia Knous [:marcia] from comment #1)
> Can we please clarify which device this was tested on?

This was tested on a Geeksphone Keon, but the issue is highly unlikely to be device-specific.

Comment 3

5 years ago
Tested on Leo:

Environmental  Variables:
Leo Build ID: 20130503070204
Kernel Date: Apr 25
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8becaf2a0bc7
Gaia: b0aca0dd1e2955e11190ede725e1fb9ee596438b

Issue sort of reproduces. 

After setting the volume to the max using the hardware rocker button, do not touch the device for about a minute or so, then press and hold the volume up button. User will see just the volume bar for a few seconds but then the speaker and vibrate icons do appear.

Comment 4

5 years ago
Comment #3 was using a Leo on the NIGHTLY channel.

The issue is continuously reproducing on the Leo BETA channel.

Environmental  Variables:
Leo Build ID: 20130503070204
Kernel Date: Apr 25
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8becaf2a0bc7
Gaia: b0aca0dd1e2955e11190ede725e1fb9ee596438b

Comment 5

5 years ago
Here are the regression window results for when this issue started occurring: 

b2g build 2013-04-28-23-02-04 PASS
b2g build 2013-04-29-07-02-04 PASS
b2g build 2013-04-29-23-02-05 FAIL
b2g build 2013-04-30-07-02-04 FAIL
Keywords: regressionwindow-wanted
http://hg.mozilla.org/releases/mozilla-b2g18/pushloghtml?startdate=2013-04-29&enddate=2013-04-30

Updated

5 years ago
Blocks: 862970
If I had to guess a bug that caused this regression off of that push log, I'd guess it's bug 862970.
If the regressing bug is bug 862970, then this might be reproducible on 1.01. Can we retest on v1.01 to see what behavior we're getting there?
status-b2g18-v1.0.1: unaffected → ?

Updated

5 years ago
Component: General → ImageLib
Product: Boot2Gecko → Core
Version: unspecified → Trunk
QA Wanted to see if this reproduces on 1.01.
Keywords: qawanted
Joe - Any ideas? Can you confirm this is a regression from bug 862970?
Flags: needinfo?(joe)

Comment 11

5 years ago
I just checked the 1.01 build, and yes it is affected as well.   


Unagi
Build ID: 20130507070205
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/7e9e10889942
Gaia: 29f5faf992aa2bacf6a9d500494093edd9472c69
status-b2g18-v1.0.1: ? → affected
Keywords: qawanted
We need to backout bug 862970.
blocking-b2g: leo+ → tef?
Flags: needinfo?(joe)
(Assignee)

Updated

5 years ago
Summary: Invalidation issues (not invalidating when it should) → Images disappearing/blank
(Assignee)

Updated

5 years ago
Summary: Images disappearing/blank → Images disappearing/remaining blank
(Assignee)

Comment 13

5 years ago
Does this happen on trunk (mozilla-central) too?
Keywords: qawanted
Talked with Andrew about this. He's suggesting we patch on to the work done in bug 862970 instead of backing out bug 862970.

Updated

5 years ago
status-firefox23: --- → ?
(Assignee)

Updated

5 years ago
Assignee: nobody → joe
(Assignee)

Comment 15

5 years ago
So the problem here (at least, on first boot of the emulator when pressing the volume buttons) is that we've reached our "tracked images" limit, so when the image gets inserted into the document we don't call RequestDecode() on it. (We do decode on Draw(), though.)

What I haven't figured out yet is why the Draw()-invoked decode doesn't cause the image to draw once the decode is done.

Updated

5 years ago
Duplicate of this bug: 869857

Updated

5 years ago
Duplicate of this bug: 869856

Comment 18

5 years ago
This issue is not occurring on the m-c build.

Checked on the Unagi device 
Build ID: 20130508030640
Gecko: http://hg.mozilla.org/mozilla-central/rev/b980d32c366f
Gaia: 752e1bf908dc8a57f1f85cf0c9505aa0ab24c03a
Keywords: qawanted

Updated

5 years ago
status-firefox23: ? → unaffected

Comment 19

5 years ago
Joe are you working on this?
(Assignee)

Comment 20

5 years ago
Yes. I have at least a partial fix.
(Assignee)

Comment 21

5 years ago
Created attachment 747060 [details] [diff] [review]
Let RequestDecode finish the decode, and just use the image it if so

If RequestDecode finishes a decode, we still return false from nsImageRenderer::PrepareImage. We shouldn't! This patch makes us take yes for an answer.

(I thought that this *might* be only a partial fix because we should have been listening to invalidation, but since this actually happens during painting, I bet we ignore those [synchronous] invalidations, and this is the only way to fix it.)
Attachment #747060 - Flags: review?(tnikkel)
(Assignee)

Comment 22

5 years ago
Try:  https://tbpl.mozilla.org/?tree=Try&rev=30ee8f51d833 (I have no idea if this is going to work - this is mozilla-b2g18)
(Assignee)

Comment 23

5 years ago
This was fixed in bug 799335 on mozilla-central.
Do we need the IsEmpty() early-return from this same chunk in bug 799335's patch, too?
(Assignee)

Comment 25

5 years ago
I don't think so, but I have 0 objections to changing the code to look like that for reduced risk.
Ah, looks like IsEmpty() should always imply !IsComplete() (since IsEmpty is a check for eStyleImageType_Null[1], and IsComplete always returns false for eStyleImageType_Null [2])

So this should be OK as-is, but it seems worth including that early-return, to avoid unnecessary work and in the spirit of making the backport as similar as possible to what landed on trunk.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.h#243
[2] http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.cpp#1637
Comment on attachment 747060 [details] [diff] [review]
Let RequestDecode finish the decode, and just use the image it if so

Stealing review, per joe's request in #gfx.

r=me with the early-return from bug 799335 (so just applying the nsCSSRendering part of that bug's patch verbatim, I think), per last few comments.
Attachment #747060 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 28

5 years ago
Created attachment 747086 [details] [diff] [review]
v2

Applied review comments.
Attachment #747060 - Attachment is obsolete: true
Attachment #747086 - Flags: review+
(Assignee)

Comment 29

5 years ago
This only needs to be checked in to mozilla-b2g18.
Keywords: checkin-needed
(Assignee)

Comment 30

5 years ago
Created attachment 747087 [details] [diff] [review]
for check-in

mq didn't have all the metadata in the patch.
Attachment #747086 - Attachment is obsolete: true
Attachment #747087 - Flags: review+
This is fallout from bug 862970 and needs to be fixed so tef+.
blocking-b2g: tef? → tef+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/508437f33ad4
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/656a44e7b2b5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-b2g18: affected → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: affected → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.