Description: When user tries to attach a cropped image to an MMS, the attached image does not appear to be cropped. The whole, uncropped image is sent. Repro Steps: 1) Update a Flame to 20150529010201 2) Launch Messages app 3) Compose new message and tap paperclip to attach image from gallery 4) Crop image on image crop screen and press done 5) Tap attached image and select view Actual: Attached image is not cropped. Expected: Attached image is cropped as per user action. Environmental Variables: Device: Flame 3.0 Build ID: 20150529010201 Gaia: e7d268074ee3c9eeb191c2205c0e35992fb3915d Gecko: f986e55c4e0b Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 Repro frequency: 3/3, 100% Link to failed test case: https://moztrap.mozilla.org/manage/case/10742/ See attached: Youtube video clip & logcat Youtube video link: http://youtu.be/q2WXXDJGnyA

[Blocking Requested - why for this release]: Functional regression failing a smoke test. Requesting a window.

Issue DOES reproduce on Flame 2.2 Attached image is not cropped. Device: Flame 2.2 Build ID: 20150529002502 Gaia: a57ec5786c9f941c690266bbb26049dbb8482b05 Gecko: 6e9875ef5297 Gonk: bd9cb3af2a0354577a6903917bc826489050b40d Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Issue does NOT reproduce on Flame 2.1 Attached image is cropped as per user action. Device: Flame 2.1 Build ID: 20150529001200 Gaia: 2304a1f6327c2ccf35d6995ee16f2231ed1f22a3 Gecko: 894528758073 Gonk: bd9cb3af2a0354577a6903917bc826489050b40d Version: 34.0 (2.1) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Moving nomination to 2.2

I haven't investigated why it occurs. FYI for Julien and David.

Note: This doesn't happen on images taken by Flame's camera. We suspect it could be an image resolution/phone memory based issue.

I see this error in the log: 05-29 08:43:50.795 6718 6718 E Gallery : [JavaScript Error: "Image corrupt or truncated." {file: "blob:app://gallery.gaiamobile.org/f0b73b1e-40e5-4630-a983-30b7649a0296#xywh=2838,1987,2410,1949" line: 0}] I think this is likely in the Gallery side. Can you please share an image that has the issue?

Julien is right that this is not an SMS bug. I'm changing the component to Gallery for now. But based on the logcat, it might actually be Core::ImageLib The photo is a 20mp image from Aries, not a photo from Flame. The logcat error message is revealing: [JavaScript Error: "Image corrupt or truncated." {file: "blob:app://gallery.gaiamobile.org/f0b73b1e-40e5-4630-a983-30b7649a0296#xywh=2838,1987,2410,1949" line: 0}] Seth: has the #xywh= media fragment been removed from 2.2? If so, it has caused this pretty major regression in the gallery app.

Last Working Device: Flame BuildID: 20150520183146 Gaia: 5a7f87b1505ba89b586372cbbbe9507d1016c40c Gecko: 1f6a0d225927 Version: 41.0a1 (3.0 Master) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 First Broken Device: Flame BuildID: 20150520185539 Gaia: 5a7f87b1505ba89b586372cbbbe9507d1016c40c Gecko: edddd285c20c Version: 41.0a1 (3.0 Master) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 Gaia is the same so it's a Gecko issue. Gecko pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1f6a0d225927&tochange=edddd285c20c Possibly caused by Bug 1154974. Notes: 1) None of the bugs in the pushlog have been uplifted on v2.2. I re-checked v2.2 and discovered that this issue is reproducible with 319MB memory only. On v3.0, this issue will occur regardless of memory settings. If it's desired I can also find the window on v2.2. 2) I did this window with unrestricted memory because on my last working build I'd run into an issue very similar to bug 1169407 where the preview is all black. It should affect the accuracy of this window on Central.

Typo: it should *NOT* affect the accuracy of this window on Central.

Let's find the 2.2 regression window here.

I can't figure out what is going on with the #xywh media fragment. I think this used to do something useful. But going back as far as 2.1 I don't see it doing anything. We've got code assuming that it can use xywh to crop a large image without decoding the whole thing. If xywh fails, and the image is 20 megapixels (as it is in this case) then gecko will need 80 megabytes. So it is no surprise to me that it fails to do the crop when memory is low. That failure (with very large image, and very low memory) might to back to 2.1 or before. I don't get what's going on with the failure on master, but it sounds like #xywh is no longer just being ignored but is now actively causing an error. I think we might need a gecko patch to fix 3.0 and a gaia patch to fix 2.2. But before I can do the gaia work, I need to know from Seth whether I should ever expect #xywh to work in 2.2 or whether I should just remove it completely. I guess I'll go ahead and take this bug for myself, though. Seth can open a new one for fixing the regression caused by 1154974.

This issue occurs on the earliest b2g37 build ID 20150112081448 with 319MB memory. There is no window here to find. Removing keyword.

Since it's a smoketest blocker that's been open for past few days, if it can't be fixed soon, could we ask for a backout of bug 1154974? Usually the rule is that smoketest failures should be fixed/backed out within a day (unless the fix is significant that warrants a few day's work, which this bug might be the case) David, do we need Seth's input to create a fix for this bug? When can we expect a fix to be available? Having said that, and looking at the bug, I am less certain now that this should be a ST blocker though. Naoki & Kevin, what do you think? Would it make sense to downgrade this bug?

If I understand correctly, this bug only occurs if you've got a really large jpeg image to test with. I don't think that "put a 20 megapixel image on your device with a 5 megapixel camera" is part of the smoketest, so I don't think that this should be considered a smoketest blocker. I'm still waiting for more information from Seth, and he is busy with 2.2+ bugs. I may have time to look at this later today, but can't promise.

Seth: here's another needinfo for you, but please see comment #8 for the original needinfo. Is backing out bug 1154974 even an option? And in https://bugzilla.mozilla.org/show_bug.cgi?id=1154974#c1 you talk about creating a serial number for FileImpl objects. In the gallery, most of the cases you are trying to optimize will be for blobs created with a slice() call on an underlying file. (We slice to extract the EXIF preview from a jpeg file.)

I suppose this image is a better representative photo. Issue still occurs using this 1920x1080 (2.1MP) image. Image is still full photo and not cropped after sending. Device: Flame 3.0 Build ID: 20150602055237 Gaia: 6d477a7884273886605049b20f60af5c1583a150 Gecko: 9eae3880b132 Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

I think we may have two completely different bug here. On master, we may have a regression caused by bug 1154974. But on 2.2, we've got a low-memory related bug, since 1154974 has not been uplifted to 2.2. It would be interesting to know if this occurs on 2.1 with the small image, or only with the giant image. I didn't realize that 1154974 was on master only. Given that this is affecting smaller images, and given that bug 1167399 is also blamed on the same bug, we should probably just back out 1154974. I'm not able to do that myself, though.

Brogan, Thanks for the new test image. The sample images that this happens for have EXIF previews that are too small to use as previews. So we are probably using #-moz-samplesize to scale them down. I suspect a bad interaction between #-moz-samplesize and bug 1154974. If I'm right, then this bug will not occur for PNG images. You could test this by converting the image from comment #17 to PNG and testing with that.

After converting ExampleImage to png format the issue no longer occurs. Image in messages displays and is sent as user cropped image, full image is no longer shown as the full image. Device: Flame 3.0 Build ID: 20150602055237 Gaia: 6d477a7884273886605049b20f60af5c1583a150 Gecko: 9eae3880b132 Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

So it is most likely caused by Bug 1154974 based on Comment 20. Seth, should we back out this patch or can we have a quick fix for this issue?

(In reply to David Flanagan [:djf] from comment #8) > Seth: has the #xywh= media fragment been removed from 2.2? If so, it has > caused this pretty major regression in the gallery app. Sorry for the slow response here, but Gecko does not and never did support #xywh. I can assure you that's true, because I'm the person assigned to implement it. =)

(In reply to David Flanagan [:djf] from comment #12) > We've got code assuming that it can use xywh to crop a large image without > decoding the whole thing. There is no simple way of doing that. It would be possible to implement using a similar approach to downscale-during-decode, but don't hold your breath. It won't make 3.0, if we ever do implement it. > I need to know from Seth whether I should > ever expect #xywh to work in 2.2 or whether I should just remove it > completely. You should not even expect it for 3.0. My advice is to remove the code that uses it.

(In reply to David Flanagan [:djf] from comment #19) > So we are probably > using #-moz-samplesize to scale them down. I suspect a bad interaction > between #-moz-samplesize and bug 1154974. Bug 1154974 could interfere with #-moz-samplesize in some cases, yes, because it could merge a #-moz-samplesize load with a non-#-moz-samplesize load. I can fix this, and I'll write up a patch to do it. Unless this proves to be much harder than anticipated, I am strongly against backing out bug 1154974 over this issue, since it overall drastically improves performance in the Gallery app. However, just to be clear: #-moz-samplesize is, with 100% certainty, going to be removed before 3.0 ships. It's only still in right now because I'm still fixing bugs and tweaking things related to downscale-during-decode, but at least for JPEG images I expect that work to be over pretty soon.

(In reply to David Flanagan [:djf] from comment #16) > Is backing out bug 1154974 even an option? And in > https://bugzilla.mozilla.org/show_bug.cgi?id=1154974#c1 you talk about > creating a serial number for FileImpl objects. In the gallery, most of the > cases you are trying to optimize will be for blobs created with a slice() > call on an underlying file. (We slice to extract the EXIF preview from a > jpeg file.) I think it's worth clarifying here: the slices receive a different serial number, and will not get merged with the original blob in the image cache.

Here's the patch. This will prevent us from merging image cache entries for requests whose URIs have different refs. This should eliminate problems not only with #-moz-samplesize, but also with #-moz-resolution. Both of those require us to create a new Image object and a new copy of the source data, so we can't safely merge a request that uses one #-moz-samplesize value with a request that uses a different #-moz-samplesize value, or with a request that does not use #-moz-samplesize.

(In reply to David Flanagan [:djf] from comment #12) > I think we might need a gecko patch to fix 3.0 and a gaia patch to fix 2.2. Ack, maybe I should've created a new bug for the Gecko issue. Sorry about that...

(In reply to Seth Fowler [:seth] from comment #28) > (In reply to David Flanagan [:djf] from comment #12) > > I think we might need a gecko patch to fix 3.0 and a gaia patch to fix 2.2. > > Ack, maybe I should've created a new bug for the Gecko issue. Sorry about > that... Thanks for the fix! so I'm guessing your fix includes the gecko issue as well, so when it's merged to master, should it be uplifted to 2.2? (if we can still uplift to 2.2)

(In reply to No-Jun Park [:njpark] from comment #29) > Thanks for the fix! so I'm guessing your fix includes the gecko issue as > well, so when it's merged to master, should it be uplifted to 2.2? (if we > can still uplift to 2.2) This fix isn't needed on 2.2. Apparently on 2.2 the problem is just a memory-related issue, so probably the bugs blocking bug 1166136 will help.

[Blocking Requested - why for this release]: Thanks, I'll change the blocking flag to 3.0? based on comment 30

This is a dupe of 1169055, and 1169055 is spark+, so making this one spark+ instead.

Comment on attachment 8614415 [details] [diff] [review] Don't merge image cache entries for blob URIs with different refs Switching the reviewer to Timothy because we need a review on this one ASAP due to spark+.

Comment on attachment 8614415 [details] [diff] [review] Don't merge image cache entries for blob URIs with different refs This should be pretty easy to write a test for, no?

(In reply to Timothy Nikkel (:tn) from comment #35) > Comment on attachment 8614415 [details] [diff] [review] > Don't merge image cache entries for blob URIs with different refs > > This should be pretty easy to write a test for, no? Yeah. It can even be a reftest. I'll add one to the final version of the patch.

This updated version of the patch includes a test.

QAWANTED: please test fix via : https://tools.taskcluster.net/index/artifacts/#gecko.v1.mozilla-inbound.revision.linux.4f873b18b4bfa16f7678316719f82461440f8880.aries-eng/gecko.v1.mozilla-inbound.revision.linux.4f873b18b4bfa16f7678316719f82461440f8880.aries-eng.opt on aries device.

This issue is verified fixed on Aries but NOT on Flame 3.0. However I think it was never established that this issue occurs on Aries. Bug 1169055 which was duped to this bug is also NOT fixed. I'm going to undupe that bug. STR on bug 1169055 is different from this bug, for this bug we use images from Gallery, and we also crop images, but on 1169055 we use the Camera app instead of Gallery, and we are not cropping images. For this bug, if I use the image attached on comment 17, then the issue does NOT occur. But if I use an image taken by Aries, the preview shows a completely black image and will occasionally LMK the Messages app. Verified fixed (but not established that issue occurred at first place) on: Device: Aries 3.0 Master BuildID: 20150605092159 Gaia: 7ff9463c63adca8e09558ae70544cb7da3bfd5e5 Gecko: 4f873b18b4bf Gonk: 3af1ede0d0956cfbf9c549df7cd9a6807a9efdf2 Version: 41.0a1 (3.0 Master) Firmware Version: D5803_23.1.A.1.28_NCB.ftf User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 Verified fixed with select image (comment 17), but broken in another way with other images taken by Aries on: Device: Flame (KK, full flashed, 319MB) BuildID: 20150605093045 Gaia: 65369b217faac7d70c1a29100c4208c6d1db16e3 Gecko: 4f873b18b4bf Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0 Master) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 NI the assignee to get attention. I will attach another image that was used for testing (taken by Aries).

Please see previous comment

I think there are two bugs here. The patch that landed should have fixed the primary bug. Similar symptoms occur if you test with a very large image (like the 20mp one attached above) on a device with low memory (like a 319mb flame). Pi-Wei: would you check whether this reproduces for you if you have 1024mb on your Flame?

David, this issue, as well as the black preview issue does not occur if I use 1024MB Flame.

David, what's the next steps based on PiWei's findings? Are we to file the bug, or will you file the bug?

This issue is Verified Fixed on Flame 3.0. Attached image is cropped as per user action. Environmental Variables: Device: Flame 3.0 Build ID: 20150608160934 Gaia: ea27c4ed5b6083c9e21d233d4804372ac4d5d353 Gecko: e10e2e8d8bf2 Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0 I'm upgrading bug 1169407 to a smoketest blocker, as we are still seeing the black image issue.

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #48) > David, what's the next steps based on PiWei's findings? Are we to file the > bug, or will you file the bug? Let's use bug 1169055 for the memory-related bug. I'll update that one to make it clear that it should also cover this case.

OK, it looks to me like the platform bug here is fixed, and the remaining issue is related to memory usage. Based on that, I'm going to make bug 1166136 block this bug, as that continues to be the metabug for platform-level issues related to excessive memory usage in the Gallery app. It sounds like there are also issues at the Gaia level that David is addressing in bug 1169055, so we're attacking this from multiple directions.

Moving the bug to the component where the regression came from.