1113002 - The MMS app is closed unexpectly when is attached an second image from Gallery

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect, P1)

Description

[sync-1]

Mozilla Build ID: 20140616024002
 +++ This bug was initially created as a clone of Bug #861746 +++
 PROBLEM DESCRIPTION:
 The MMS app is closed unexpectly when is attached an second image from Gallery
 
 
 EXPECTED BEHAVIOUR:
 The Aplication should allow the second attachment without close
 
 STEPS TO REPRODUCE:
 Turn on the device 
 Go to MMS in Main screen
 Attach an picture from Gallery - Done
 Attach other picture from Gallery (on this moment the APP is crashed)
 ++++++++++ end of initial bug #861746 description ++++++++++
 
 		
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Attachment: Picture

Comment 2

[sync-1]

Attachment: Previoius SW 20E

Comment 3

[sync-1]

Attachment: NEW SW 20L_2

Comment 4

[sync-1]

Attachment: Picture

Comment 5

[sync-1]

Attachment: Previoius SW 20E

Comment 6

[sync-1]

Attachment: NEW SW 20L

Comment 7

[sync-1]

Attachment: Picture

Comment 8

[Julien Wajsberg [:julienw]]

Hey,

I'd need your Firefox OS version, the Gaia version, and the amount of memory you have on your device.

Thanks

Comment 9

[田旻]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #8)
> Hey,
> 
> I'd need your Firefox OS version, the Gaia version, and the amount of memory
> you have on your device.
> 
> Thanks

We have fixed this bug by release the memory timely during resizing pictures.
Just use "img.src = ''" after "context.drawImage". This bug can be closed now.

Comment 10

[Julien Wajsberg [:julienw]]

Tianm, can you confirm where you do this?
Is it in _resizeImageBlobWithRatio ? I think we did this in [1], can you confirm this is where you missed this?

Note that it should be there since v2.0 already.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/utils.js#L362

Comment 11

[田旻]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #10)
> Tianm, can you confirm where you do this?
> Is it in _resizeImageBlobWithRatio ? I think we did this in [1], can you
> confirm this is where you missed this?
> 
> Note that it should be there since v2.0 already.
> 
> [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/utils.js#L362

In sms/js/attachment.js and sms/js/utils.js.

Comment 12

[Julien Wajsberg [:julienw]]

Ok, I see, I'll do a quick fix for v2.1 as well. I don't think we need this for master anymore though.

Comment 13

[Julien Wajsberg [:julienw]]

Attachment: master github PR

Quite unrelated to the bug here, but it's impacting the same method. In this patch I'm simply removing it because it does not seem to be used anymore.

Comment 14

[Julien Wajsberg [:julienw]]

Attachment: v2.1 github PR

I'm not sure we can do a test for this?

Comment 15

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8542999 [details] [review]
master github PR

Thanks for clean-up!

Comment 16

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] (PTO until 1/5) from comment #14)
> Created attachment 8543001 [details] [review]
> v2.1 github PR
> 
> I'm not sure we can do a test for this?

Asked question at GitHub, keeping r? for now.

Comment 17

[Julien Wajsberg [:julienw]]

Answered :)

Comment 18

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8543001 [details] [review]
v2.1 github PR

(In reply to Julien Wajsberg [:julienw] (PTO until 1/5) from comment #17)
> Answered :)

Okay, looks good then with a small nit.

Thanks!

Comment 19

[Julien Wajsberg [:julienw]]

Comment on attachment 8543001 [details] [review]
v2.1 github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): -
[User impact] if declined: OOM Kill in low memory condition
[Testing completed]: yes, both manually and unit tested
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Comment 20

[Julien Wajsberg [:julienw]]

cleanup patch on master: d1500f765853ef0c1b7d6def99eb36f83ac99f2a

(will keep the bug open until we land in v2.1)

Comment 21

[Julien Wajsberg [:julienw]]

Fixed the nit, waiting for approval.

Comment 22

[Julien Wajsberg [:julienw]]

v2.1: 64db236bea9a0510567ab7ced2f2b4688737123c

Comment 23

[Alissa（leave from Mozilla）]

Attachment: verify1.3gp

This issue has been verified successfully on Flame 2.1,see verify1.3gp
Build:
ia-Rev        64db236bea9a0510567ab7ced2f2b4688737123c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/273f24a1d1fe
Build-ID        20150111001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.035122
FW-Date         Sun Jan 11 03:51:33 EST 2015
Bootloader      L1TC000118D0