1169407 - [Contacts] Adding cropped photo as contact picture displays all black image

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[Brogan Zumwalt [Inactive]]

Attachment: Logcat

Description:
When user adds a picture from gallery as a contact photo, cropping it in the picture chooser (from contacts) results in contact photo displaying as an all black image.

User must crop photo to the smallest size allowed by selector. This only seems to work with photos that are wider than they are tall.

Repro Steps:
1) Update a Flame to 20150528143550
2) Launch contacts app and tap to edit an existing contact
3) Tap to change or add photo to contact with gallery as source
4) Crop photo to smallest possible size and press done

Actual:
All of the contact picture is a black square.

Expected:
Contact picture is displayed as cropped.

Environmental Variables:
Device: Flame 3.0
Build ID: 20150528143550
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%
See attached: Youtube video clip & logcat
Youtube video: https://youtu.be/JWb2LfMT9P8

Comment 1

[Brogan Zumwalt [Inactive]]

Issue does NOT reproduce on Flame 2.2

Contact picture is displayed as cropped, no black areas visible in picture.

Device: Flame 2.2
Build ID: 20150528002504
Gaia: 999bc627063d16c20f703e702f31a5cf0da8b4a6
Gecko: 351101ec82ba
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

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

[Blocking Requested - why for this release]:

This is a bad regression from 2.2 so nominating this 3.0?

Comment 3

[Johan Lorenzo [:jlorenzo]]

Comms triage: Regression

Comment 4

[Francisco Jordano [:arcturus] [:francisco]]

Hei Wilson, do you think we are getting the image correctly from the gallery?

Comment 5

[Wilson Page [:wilsonpage]]

I know :djf has been having issues with image decoding in Gecko recently so he might have some clues.

Comment 6

[David Flanagan [:djf]]

There was a gecko change recently (Seth probably has the bug #) where in low memory situations, canvas.drawImage() (used for cropping) doesn't decode the image and just draws a black rectangle.  In 2.2 it throws an exception instead.

Brogan: A few questions for you about this bug. (1) Does this only occur on a 319mb flame? (2) Is the photo you're seeing this bug with an ordinary photo taken on the Flame? If not, would you attach it please? (3) Would you check this on 2.2 again? I wouldn't expect to see a black image on 2.2, but instead some other kind of failure, like the inability to pick a cropped contact photo at all. (4) Can you reproduce this bug when picking and cropping an image from the SMS app instead of the contacts app?

If there are symptoms in 2.2 then this may be related to bug 1166136. If it really is 3.0, then it may have something to do with the changes made for Aries to remove image size restrictions that are still needed for low-memory flames.

In any case, I'm pretty sure it is a gallery bug, not a contacts bug. I'll take it and make sure it goes away when the other stuff gets fixed, too.

Comment 7

[John Thomas [:Johnt]]

B2G Inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20141105103947
Gaia: a7904af8e602d3f54c31aeb44a4a9773f99265f6
Gecko: 1139ec0f1ac3
Version: 36.0a1 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0



First Broken 
Environmental Variables:
Device: Flame 2.2
Build ID: 20141105105744
Gaia: bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e
Gecko: 22b67157c1d7
Version: 36.0a1 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0




Last Working gaia / First Broken gecko - This issue 
Gaia: a7904af8e602d3f54c31aeb44a4a9773f99265f6
Gecko: 22b67157c1d7

First Broken gaia / Last Working gecko - This issue 
Gaia: bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e
Gecko: 1139ec0f1ac3

B2G Inbound Pushlog:
https://github.com/mozilla-b2g/gaia/compare/a7904af8e602d3f54c31aeb44a4a9773f99265f6...bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e


Issue appears to occur due to changes made in Bug 1064600

Addiontal Note: In later builds cropping images to as small as possible will have the image appear as black. Moving further back the image will appear blank. I did not trace exactly when this difference occurs but thought it was worth mentioning although I believe it to be the same issue.

Comment 8

[KTucker [:KTucker][Inactive 3/4/2016]]

Justin, can you take a look at this please? This might have been caused by the landing for bug 1064600.

Comment 9

[Brogan Zumwalt [Inactive]]

Attachment: ExampleImage

Issue reproduces in 512 as well, cropping image as described in comment 0 results in an all black image being displayed for contact.

Issue does not reproduce with images taken with device camera (attached an example affected image.)

As per comment 7 the issue does reproduce on Flame 2.2

Different issue occurs in SMS app (see bug 1169680)


Device: Flame 3.0
Build ID: 20150601010203
Gaia: e6dc0f4c583407a4a52a66ce7cb11f058302a762
Gecko: f8d21278244b
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

Comment 10

[Martin Shuman [:Marty]]

Upgrading to smoketest blocker, as this blocks case https://moztrap.mozilla.org/manage/case/10742/ now that bug 1169680 is resolved.

Comment 11

[No-Jun Park [:njpark]]

Hi David, wondering about the status of this bug- this is a smoketest blocker that made us to disable OTA. If jdarcangelo thinks the Comment 7 shows the cause for this bug, can we back out or add a quick fix?  It's been open since the beginning of this week so we need to have it resolved asap.  Thanks!

Comment 12

[Justin D'Arcangelo [:justindarc]]

Setting NI? for Punam to take a closer look as I'm low on bandwidth at the moment. The patch in question landed over 7 months ago and I just skimmed through it and don't see anything obvious that might cause this.

Comment 13

[No-Jun Park [:njpark]]

Peter, could you double check the regression range in Comment 7?  the range is in build 36, and I don't think we didn't know about this bug for that long, (as you know, we're currently in 41) especially when 2.2 is unaffected.

Comment 14

[Peter Bylenga [:PBylenga]]

We were counting it as a ST for a MMS case, perhaps the root cause is the same but was exposed at different times per app?  Also it's only happening with certain files instead of being a global issue. Because of these reasons going to denom as a smoketest.  This is still a 3.0+ and should be fixed.

Comment 15

[Punam Dahiya [:pdahiya]]

I am able to replicate the issue with attached image and not with image taken from Flame camera. This issue is seen for all cases where pick activity is used  (via contacts or Messaging app). 

Device used: Flame-kk 319 MB

Error seen in logs

06-12 05:48:52.395 E/Gallery ( 3421): [JavaScript Error: "Image corrupt or truncated." {file: "blob:app://gallery.gaiamobile.org/6802fef1-d4a1-46b7-9d79-bb13bb8a6c1f#xywh=1594,1228,2099,2007" line: 0}]

which indicates we are using #xywh media fragment to crop while decoding for this image. Will continue to investigate further and update  with findings.

Comment 16

[Punam Dahiya [:pdahiya]]

On Flame-kk 1024 MB this bug is not seen and I can successfully see cropped picture added to contacts indicating this bug is happening only in low memory for attached image.

Comment 17

[David Flanagan [:djf]]

The test image is 20 megapixels, which is a pretty unrealistic test case for the Flame. I don't think this needs to be a smoketest blocker.

In comment #6 I said I'd take the bug and investigate, but apparently I didn't actually take it, and I forgot about it.  I am taking it now.

Punam's comment #15 is a good clue.  I could swear that #xywh used to work. Seth tells me that it never has worked and never will.  Certainly it does not work today. If I modify crop_resize_rotate.js so it does not use #xywh, then it will be forced to use #-moz-samplesize intead, and that will probably fix this.

Comment 18

[David Flanagan [:djf]]

It turns out that hte #xywh error message is a red herring. The cropResizeRotate() function only falls back to #xywh if it is not going to use #-moz-samplesize. So even if I get rid of #xywh (which I should do since it has never worked and probably never will) we will still have the bug which is that we're failing to downsample the image before cropping it.

The problem is that in the pick activity code we are not passing a maximum size to cropResizeRotate(). The problem may well be with bug 1064600, since we did refactor that code into pick.js in that bug.

Comment 19

[David Flanagan [:djf]]

Okay, here's what happened. 

- I began work on bug 1064600

- I fixed the issue in this bug as part of bug 1065887

- Justin took over my patch in bug 1064600.  That patch had already copied the bad code from gallery.js to pick.js, so it would have been very tricky to notice the merge error and fix it correctly.

So basically, this issue was fixed once, but then we accidentally broke it again with bug 1064600.

Patch coming up.

Comment 20

[David Flanagan [:djf]]

Attachment: link to patch on github

Punam,

This patch restores code that was added in bug 1065877 to limit the maximum image decode size when cropping images picked during the pick activity. It actually makes it more aggressive than it was in 1065877 because I was still seeing failures when I just restored the old code.

Also, the patch adds code to use maxFileSizeBytes from the pick activity as a hint about the desired output size. Contacts and Messages set this parameters in their pick requests, and using it dramatically reduces the size of the images we return, which is a good thing for all of the apps.

Comment 21

[David Flanagan [:djf]]

I had the bug number wrong in comment #19. I meant bug 1069877.

Comment 22

[David Flanagan [:djf]]

I've filed bug 1174367 to remove #xywh, but that is not required to resolve this bug.

Comment 23

[Punam Dahiya [:pdahiya]]

Comment on attachment 8621907 [details] [review]
link to patch on github

Hi David, Patch looks good and successfully works for pick flow on 319 MB. Thanks!

Comment 24

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/eecd9d2bb220e10792762fc274c910cb3febbd72

Comment 25

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on latest Nightly build of Flame v3.0 by the STR in Comment 0.

Actual results: Adding and cropping the example image provided in Comment 9, then saved as contact picture, it displays cropped photo normally. 
See attachment: verified_v3.0.3gp
Reproduce rate: 0/10


Device: Flame v3.0 build(319mb, Verified)
Build ID               20150616160206
Gaia Revision          9d42e94446f2dc01b519172b6d75d81d4d435bdc
Gaia Date              2015-06-16 16:16:47
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/27caa5299f9f
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150616.194325
Firmware Date          Tue Jun 16 19:43:36 EDT 2015
Bootloader             L1TC000118D0

Comment 26

[Shally Li（leave from Mozilla）]

Attachment: verified_v3.0.3gp