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.
Can we please clarify which device this was tested on?
(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.
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 #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
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
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?
QA Wanted to see if this reproduces on 1.01.
Joe - Any ideas? Can you confirm this is a regression from bug 862970?
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
We need to backout bug 862970.
Does this happen on trunk (mozilla-central) too?
Talked with Andrew about this. He's suggesting we patch on to the work done in bug 862970 instead of backing out bug 862970.
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.
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
Joe are you working on this?
Yes. I have at least a partial fix.
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.)
Try: https://tbpl.mozilla.org/?tree=Try&rev=30ee8f51d833 (I have no idea if this is going to work - this is mozilla-b2g18)
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?
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, and IsComplete always returns false for eStyleImageType_Null ) 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.  http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.h#243  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.
Created attachment 747086 [details] [diff] [review] v2 Applied review comments.
This only needs to be checked in to mozilla-b2g18.
Created attachment 747087 [details] [diff] [review] for check-in mq didn't have all the metadata in the patch.
This is fallout from bug 862970 and needs to be fixed so tef+.