Closed Bug 994553 Opened 6 years ago Closed 6 years ago
[Sora][Message][Contacts]Contacts's head portrait do not display in Message
97.55 KB, application/octet-stream
238.00 KB, text/plain
41.46 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review|
8.82 MB, video/3gpp
Firefox OS v1.3 AU_LINUX_GECKO_B2G_JB_3.2.01.03.00.112.278 Mozilla build ID:20140323004002 DEFECT DESCRIPTION: Contacts's head portrait do not display in Message REPRODUCING PROCEDURES: 1.Enter into Message 2.edit message-->add a contacts(such as mei,10010)-->input content-->send 3.Enter into contacts-->edit this contact(mei)-->add a picture 4.Enter into Message,back to thread list view 5.Contacts's head portrait do not display in Message, and other's portrait is disappear. EXPECTED BEHAVIOUR: K.O---Contacts's head portrait should display in Message ASSOCIATE SPECIFICATION: TEST PLAN REFERENCE: TOOLS AND PLATFORMS USED: USER IMPACT:Medium REPRODUCING RATE:5/5 For FT PR, Please list reference mobile's behavior: It can't be reproduced on v1.1.
Deos the picture appears if you kill and the restart the app?
I don't think it should cause this, but we could check bug 983885. It changed how we handle canvases after attaching an image. Maybe we need to reload something in order to handle the activity correctly.
(In reply to Julien Wajsberg [:julienw] from comment #1) > Deos the picture appears if you kill and the restart the app? Yes, if you kill the sms app and restart it again, it will be ok.
I don't reproduce on my peak 1.3. Reporter, could you please attach a video?
Switching to qawanted - let's check we can reproduce this on the Moz side on 1.3.
(In reply to Julien Wajsberg [:julienw] from comment #5) > I don't reproduce on my peak 1.3. > > Reporter, could you please attach a video? OK, I will give you some video ASAP.
blocking-b2g: --- → 1.3?
ni Tian for video. QAWanted please check if reproducible
Issue is reproducible on 1.3: Device: Buri 1.3 MOZ BuildID: 20140414004002 Gaia: 8b92c56267fe50772095f1f25d6cc1f9c9966eb4 Gecko: 3e26908fca71 Version: 28.0 Firmware Version: v1.2-device.cfg Repro steps: Pre-requisite: Have 2 contacts A and B exist in Contacts. A without a portrait picture, B with a portrait picture. 1) Text B a message to create a Message thread 2) Text A a message to create a Message thread 3) Observe within Messages app, that thread A (conversation with A) doesn't have a picture, while thread B does 4) Go to Contacts app, edit A to add a picture 5) Go back to Message app and observe Expected: What was observed on step 3 should be observed again Actual: Both thread A and thread B lose their pictures Attaching a logcat reproducing the issue.
We need a screenshot.
Adding a combined screenshot demonstrating comment 9 's repro step 3 on the left of the screenshot, and on the right shows what happens on step 5.
This one is found by the carriers during the IOT test, so it is not a IOT blocker
Please, I still really need a video to be able to reproduce, because currently I can't. I understand the issue's symptom but without reproducing it's useless. Thanks !
(In reply to Julien Wajsberg [:julienw] from comment #13) > Please, I still really need a video to be able to reproduce, because > currently I can't. I understand the issue's symptom but without reproducing > it's useless. > > Thanks ! Here you go~ https://www.youtube.com/watch?v=VWtdHomcY88&feature=youtu.be Thanks and please let me know if you have any question about reproduce steps
ok I reproduced the issue. I think this happen only when the thread was not existing before. I don't think this bug should be a blocker.
reproduce on 1.4 too.
Backlog from 1.3
blocking-b2g: 1.3? → backlog
ok, I got other ways to reproduce. Still not a blocker in my opinion but this looks worse than initially. For example, it can happen * when you add/remove an image for a thread that is displayed (no need to send a message) * when you delete a thread (no need to send a message either) here is what I think happens: * we use blob url to display the images * we revoke the blob url too soon I removed the "revoke" call, and then it actually works fine. I think this will happen more when we have a big DOM and reflow takes more time; maybe that's why I reproduce the bug without opening a thread whereas the reporter needs to open the thread before. So what's the solution here? Obviously we still need to eventually revoke the blob. But maybe it's fine to not revoke it until we either change the src (in the same function, we could store it on a dataset to make it easier to fetch it) or remove the dom node (should be easy to find the places where this happens). Another solution is to schedule the revoke at a very later time (say: setTimeout for 10 seconds). Steve, what do you think?
Vance, can you try the WIP patch in https://github.com/julienw/gaia/compare/mozilla-b2g:v1.4...994553-v14-revoke-later.patch ? Thanks !
(In reply to Julien Wajsberg [:julienw] from comment #19) > Vance, can you try the WIP patch in > https://github.com/julienw/gaia/compare/mozilla-b2g:v1.4...994553-v14-revoke- > later.patch ? Thanks ! This patch can work well.
Hi Julien, the WIP patch works fine. can that be a workaround solution for 1.3 branch?
I need to see exactly where we use this, because this can prevent some memory from being freed. That's why also I'd like to have Steve's advice on this.
This bug is not a blocker anyway, right ?
No it is not a blocker for now, but TCL worries that if other carriers also spot this bug during the IOT test, they might block it since the symptom is quite obvious
(In reply to Julien Wajsberg [:julienw] from comment #18) > I think this will happen more when we have a big DOM and reflow takes more > time; maybe that's why I reproduce the bug without opening a thread whereas > the reporter needs to open the thread before. > > So what's the solution here? Obviously we still need to eventually revoke > the blob. But maybe it's fine to not revoke it until we either change the > src (in the same function, we could store it on a dataset to make it easier > to fetch it) or remove the dom node (should be easy to find the places where > this happens). Another solution is to schedule the revoke at a very later > time (say: setTimeout for 10 seconds). > > Steve, what do you think? I also have this problem in Bug 996516 patch because there is no proper timing to wait for backgroung image url revoke. That's why I didn't revoke it in Bug 996516 at first. Set a timeout is a solution but you can't guarantee that there is no edge case that could block DOM rendering(even we know 10 sec sould be very safe). I think revoke url while removing the dom node should be a reasonalbe solution here and it won't change the code base too much. BTW, if we want to save the url to dataset, maybe we should also apply visibility monitor lib in shared to clear the url when element out of sight and set it back when visible. It can actually release the image memory, but the drawback is it will affect the scrolling performance. Unless we get serious memory concern, I think it shouldn't be our top priority because of performance issue.
See also the thread in dev-b2g https://groups.google.com/forum/#!topic/mozilla.dev.b2g/81lxhZQgevM It's not obvious to me when some memory is released. It seems to me that the imglib is keeping the blob data around anyway, but I don't know if it's copied or just a reference or...
Duped bug 1003789 is 2.0+. Note that it's not a regression because it exists since 1.3 (likely regression of bug 972245).
blocking-b2g: backlog → 2.0?
(In reply to Julien Wajsberg [:julienw] (away until 5th May) from comment #28) > Duped bug 1003789 is 2.0+. Note that it's not a regression because it exists > since 1.3 (likely regression of bug 972245). No, that still makes a regression. That just means it's a regression from 1.1.
blocking-b2g: 2.0? → 2.0+
Not reproducing the bug in today's master build with hamachi: B-174 Gecko-564e4e1 Gaia-a436da1 BuildID:20140522070059 Requesting qawanted to retest
I was able to reproduce this issue on todays Buri Master using Pi Wei's steps from comment 9. Both conversation's showed no image. 2.0 Environmental Variables: Device: Buri 2.0 MOZ BuildID: 20140522062218 Gaia: 7db23414f2d632f4d00b5023ac1090b6045dc5fd Gecko: 48ef1c52dece Version: 32.0a1 Firmware Version: v1.2-device.cfg
Maria, it's actually intermittent, and it especially depends on the amount of data you have. The more you have threads, the more it happens. For example, on my personal dataset, it's not intermittent anymore ;)
Target Milestone: --- → 2.0 S4 (20june)
Hi Julien, this patch makes the url only revoke when element got removed from the dom tree. It should prevent the photo intermittent disappear issue.
Comment on attachment 8438847 [details] [review] Link to github One question for you on github
Comment on attachment 8438847 [details] [review] Link to github r=me let's do this, thanks !
Attachment #8438847 - Flags: review+
BTW, I still tried some test per your question. In the medium workflow case, it will take 100~200 ms for the additional mutation observer. Comparing the total threads render time(5sec+), I think it's trivial.
100~200ms is too much already :)
Also, can you file a bug to remove the other uses of Utils.asyncLoadRevokeURL ? I think it's not as urgent as here, but we saw that it can fail intermittently.
(In reply to Julien Wajsberg [:julienw] from comment #40) > Also, can you file a bug to remove the other uses of > Utils.asyncLoadRevokeURL ? I think it's not as urgent as here, but we saw > that it can fail intermittently. Bug created in Bug 1025737 :)
Travis is green but some failed on gaia try... Seems some issue in linux desktop 64. In master: 5f9f64a05c15145df6ed33a7807adcebb11931f3
duped bug 1023256 is 1.4+, please set this one to 1.4+ and uplift.
blocking-b2g: 2.0+ → 1.4?
(In reply to Julien Wajsberg [:julienw] from comment #45) > duped bug 1023256 is 1.4+, please set this one to 1.4+ and uplift. Blocking as the DUP was deemed a blocker and we are targeting the real fix here.
blocking-b2g: 1.4? → 1.4+
This issue has been verified successfully on Flame 2.0,2.1 See attachment: Verify_video.3gp Reproducing rate: 0/5 Flame 2.1 build: Gaia-Rev db2e84860f5a7cc334464618c6ea9e92ff82e9dd Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119 Build-ID 20141126001202 Version 34.0 FLame 2.0 build: Gaia-Rev f9d6e3d83c3922e9399a6c27f5ce4cdd27bdfd05 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/45112935086f Build-ID 20141126000203 Version 32.0
You need to log in before you can comment on or make changes to this bug.