Description: While the user was in a call, the user tried to send a picture message and the messaging app force closed Repro Steps: 1) Updated Buri to BuildID: 20140113004002 2) Using 2 devices, make a call from device 1 to device 2 3) Once connected, on device 1 proceed to the message app 4) Select the attachment icon 5) Select camera 6) Take a picture and use that picture Actual: The messaging app force closes Expected: The messaging app converts to mms Environmental Variables: Device: Buri 1.3 MOZ BuildID: 20140113004002 Gaia: b3fc4f712562ee92b0ed0bd17abc61be9a36a8da Gecko: 5bb1837de7c0 Version: 28.0a2 Firmware Version: V1.2-device.cfg

Issue did not occur in 1.2 buri build. The messaging app did not force close. Environmental Variables: Device: Buri 1.2 COM BuildID: 20140113004002 Gaia: 539a25e1887b902b8b25038c547048e691bd97f6 Gecko: e672faf1e6a1 Version: 26.0 RIL Version: 01.02.00.019.102 Firmware Version: V1.2-device.cfg

Can we try with (separately): * SkiaGL disabled (set: "pref("gfx.canvas.azure.accelerated", false);") * APZC disabled (longpress power button, select "disable APZ for all applications", kill all apps And does it happen 100% of tries? Thanks!

It isn't going to be Skia GL related - the problem would have likely reproduced in 1.2 if that was the case. It also isn't likely going to be APZC related - those issues don't usually cause force close behavior. And I don't see evidence that the reporter thinks this is intermittent based on the above comment, so I'm clearing the flag.

Please, I'd still want to know what happens with SkiaGL disabled, as this points to a memory-related issue (I don't reproduce on Peak).

umm, it's not a problem related with the call in progress... I am reproducing the bug in v1.3 while the call is in progress and I attach two photos (not one) taken from the camera in the same message, then the messaging app force closes. But I am testing without the call in progress now and the messaging app is closing too after attaching two pictures from the camera or the gallery... size photos problem in MMS, perhaps Build info: Buri eng v1.3 B-33. Gecko-a037f1d Gaia-96c05de

(In reply to Julien Wajsberg [:julienw] from comment #4) > Please, I'd still want to know what happens with SkiaGL disabled, as this > points to a memory-related issue (I don't reproduce on Peak). I think I made it pretty clear that this isn't going to be related to SkiaGL likely, as we would have saw this bug on 1.2.

Regression Window: Last Working Environmental Variables: Device: Buri v1.3 Mozilla RIL BuildID: 20131123040202 Gaia: e48c2025b97f26db6e23c16d72095143d731b1fa Gecko: 98aa428a392c Version: 28.0a1 Base Image: V1.2-device.cfg First Broken Environmental Variables: Device: Buri v1.3 Mozilla RIL BuildID: 20131124040202 Gaia: c736d91f6a1aada7b81a3d7b7df2635f2f9a655a Gecko: 74ab61b8d0f0 Version: 28.0a1 Base Image: V1.2-device.cfg Note: On the the 11/23/13 build the issue in comment 0 does not occur, but the issue noted in comment 5 can be reproduced.

No commit in the SMS app in this range.

Gabriele please, could you check if there is something we can do or if Buri is definitely to low in memory to achieve this scenario ?

From bug 940729 comment 20: 1. Create a contact in the contacts app. 2. Select that contact and press the message icon. 3. Press the attachment button to select a picture. 4. Crop the picture and accept the changes to add it to the MMS. 5. Press the attachment button a second time and select and crop a picture 6. Observe that the user is returned to the Contacts screen rather than remaining on the SMS thread. See also attachment 8349705 [details] and attachment 8349706 [details] for that bug. From bug 956226 comment 11: 1. Share one pic (200k~300k) to messages 2. Tapping on Home button immediately after SMS is launched and new message page appears. 3. Share another pic or the same pic to messages again --> SMS is forced to stop. But if you wait for some time before sending SMS to background, everything works fine. The difference with this bug is that the pic is smaller, but the resulting behavior is the same. Gabriele, I'm requesting needinfo from Jerry here but if you can move forward without his help, please do!

Smells like OOM to me. Here's my theory: - you receive an incoming call -> launches the communications app and gives it FOREGROUND_HIGH priority, this pretty much guarantees that the communication app will not be killed if we run out of memory - launch the SMS app, all fine - launch the camera app, again all fine, we've got four open applications and we should be able to fit them - take a picture and use it, this in general causes a spike in memory consumption due to the creation of the blob with its backing array and all, since the camera app is in the foreground to make room for more memory there's only the SMS app to be killed... - ... hence the SMS app gets killed I have to try it to be sure though so that I can examine in real-time what's going on. If my theory proves true then there's no easy solution for this; I've got a couple of ideas to reduce memory consumption when creating blobs but they require significant platform work.

QA Wanted - Can we get a dmesg log with this bug?

Gabriele: thanks to Jerry we managed to reduce the memory consumption in the Gallery app, so maybe we can do some easy adjustment to the SMS app. For information, the code is in [1]. [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/utils.js#L296-L388

(In reply to Julien Wajsberg [:julienw] from comment #14) > Gabriele: thanks to Jerry we managed to reduce the memory consumption in the > Gallery app, so maybe we can do some easy adjustment to the SMS app. For > information, the code is in [1]. > > [1] > https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/utils.js#L296- > L388 I'm trying to figure out how to use that to help us in this situation. The problem in general is caused by the generation of the blob on the gallery side; if the original image is too large then the resulting blob eats enough memory to trigger the OOM condition. The code in your link deals with shrinking the image once it's been received by the SMS app. I'm I missing something? I'll try to reproduce it now, I've been a bit stuck with reviews today and I'm a bit late :-(

Attached dmesg log taken immediately after completing the steps from Comment 0.

I'm assuming the renomination was by accident, so setting the blocking flag again.

(In reply to Jason Smith [:jsmith] from comment #17) > I'm assuming the renomination was by accident, so setting the blocking flag > again. Yes, I changed it accidentally; I hadn't even noticed in fact :(

Hi Julien, Did you see the bug 939962? In comment 14,maybe you can try to use "willReadFrequently". var context = canvas.getContext('2d', { willReadFrequently: true }); With this option, we will use skia as canvas backend instead of skiaGL. You can compare the performance between skia and skiaGL. -- Hi Jason, "I think I made it pretty clear that this isn't going to be related to SkiaGL likely, as we would have saw this bug on 1.2." In comment 1, Josh said that it is not occur on 1.2(maybe there is no skiagl in 1.2), so can we make sure that isn't going to be related to SkiaGL? Use skia(software rendering) can reduce some memory usage.

I could make it crash with just one picture. Gabriele, I'm stealing this, at least temporarily.

Disabling skiagl make this bug disappear. I'll reenable skiagl and add "willReadFrequently" to our canvas code. If I understand correctly,it's better to use skiagl for canvases that are just displayed, but not necessarily for canvases that are used to calculate and then thrown away.

see also PR at https://github.com/mozilla-b2g/gaia/pull/15681 This adds willReadFrequently to all our canvas contexts that are not displayed. --- apps/sms/js/attachment.js | 2 +- apps/sms/js/utils.js | 2 +- apps/sms/js/wbmp.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Asking review to Gabriele because he's good at performance and memory stuff ;)

So, all our canvases are used to read back data after writing to them, therefore using "willReadFrequently" everywhere makes a lot of sense (even if this feels like a hack to me).

Comment on attachment 8365041 [details] [diff] [review] patch v1 Review of attachment 8365041 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the stealing. LGTM but have you tried to reproduce the issue with this patch applied? I can try myself if you'd like as soon as I'm finished with bug 963475 (which means later today basically).

Yes of course I did :) I don't request review before trying ;) Without the patch, I was able to make the app crash when attaching _one_ picture with the camera. With the patch I couldn't make it crash.

master: 177e4c8c6459d8b85a840354c27e0d3b37ee4329

(In reply to Julien Wajsberg [:julienw] from comment #26) > Yes of course I did :) I don't request review before trying ;) OK, cool :) I was asking because lately I'm churning out so many patches that I've got a test backlog so I work on a patch while the (slow) tests for the previous one are running so I'm never sure what I've tested already and what I didn't ;) And BTW, thanks for fixing this.

(In reply to Jerry Shih[:jerry] from comment #19) > Hi Julien, > Did you see the bug 939962? > In comment 14,maybe you can try to use "willReadFrequently". > var context = canvas.getContext('2d', { willReadFrequently: true }); > > With this option, we will use skia as canvas backend instead of skiaGL. > You can compare the performance between skia and skiaGL. > > > -- > Hi Jason, > > "I think I made it pretty clear that this isn't going to be related to > SkiaGL likely, as we would have saw this bug on 1.2." > > In comment 1, Josh said that it is not occur on 1.2(maybe there is no skiagl > in 1.2), so can we make sure that isn't going to be related to SkiaGL? > Use skia(software rendering) can reduce some memory usage. Skia GL is enabled on 1.2. So this if reproducing on 1.3 and not on 1.2, then it's a regression with Skia GL in the 1.3 timeframe. Milan - Does the gfx team agree with the approach given in this bug for the Messages app? Or should we consider not using willReadFrequently & get this fixed in gecko?

Yes, willReadFrequently is the way to go (for now - see bug 963559 for some longer term thoughts.) 1.2 sometimes gets away with things as we don't have pmem to shmem fallback (although I didn't see that in this log.) And while it does feel a wee bit like a hack, in lots of cases (probably all these included), it'll probably be faster, as well as use less memory, so it's hard to say no to it.

Uplifted 177e4c8c6459d8b85a840354c27e0d3b37ee4329 to: v1.3: 7aabd6692c5d20edf90c9fdcc20d35a951d416fe

(In reply to Milan Sreckovic [:milan] from comment #30) > Yes, willReadFrequently is the way to go (for now - see bug 963559 for some > longer term thoughts.) 1.2 sometimes gets away with things as we don't have > pmem to shmem fallback (although I didn't see that in this log.) And while > it does feel a wee bit like a hack, in lots of cases (probably all these > included), it'll probably be faster, as well as use less memory, so it's > hard to say no to it. Before doing this change I asked Nical to understand more deeply the change. In all the cases we have in SMS, we use Canvas either to resize images (write a big image + read a small one in a small amount of time) or decode a binary image format (write invididual pixels). Maybe the name "willReadFrequently" is not good enough, or maybe we should have more terms that would also bypass the GPU, eg "willWritePixels" or "willNotDisplay".