Closed Bug 1113002 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect

Tracking

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 unaffected)

VERIFIED FIXED
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- unaffected

People

(Reporter: sync-1, Assigned: julienw)

Details

Attachments

(10 files)

800.86 KB, application/x-rar-compressed
Details
9.81 MB, video/mp4
Details
4.46 MB, application/octet-stream
Details
800.86 KB, application/x-rar-compressed
Details
9.81 MB, video/mp4
Details
7.00 MB, application/octet-stream
Details
800.86 KB, application/x-rar-compressed
Details
46 bytes, text/x-github-pull-request
azasypkin
: review+
Details | Review
46 bytes, text/x-github-pull-request
azasypkin
: review+
Details | Review
2.83 MB, video/3gpp
Details
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:
Attached file Picture
Attached video Previoius SW 20E
Attached file NEW SW 20L_2
Attached file Picture
Attached video Previoius SW 20E
Attached file NEW SW 20L
Attached file Picture
Hey,

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

Thanks
Flags: needinfo?(sync-1)
(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.
Flags: needinfo?(sync-1)
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
Flags: needinfo?(tianm)
(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.
Flags: needinfo?(tianm)
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.
Assignee: nobody → felash
Attached file 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.
Attachment #8542999 - Flags: review?(azasypkin)
Attached file v2.1 github PR
I'm not sure we can do a test for this?
Attachment #8543001 - Flags: review?(azasypkin)
Comment on attachment 8542999 [details] [review]
master github PR

Thanks for clean-up!
Attachment #8542999 - Flags: review?(azasypkin) → review+
(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.
Answered :)
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!
Attachment #8543001 - Flags: review?(azasypkin) → review+
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
Attachment #8543001 - Flags: approval-gaia-v2.1?(bbajaj)
cleanup patch on master: d1500f765853ef0c1b7d6def99eb36f83ac99f2a

(will keep the bug open until we land in v2.1)
Fixed the nit, waiting for approval.
blocking-b2g: --- → 2.1+
Keywords: verifyme
Attachment #8543001 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
v2.1: 64db236bea9a0510567ab7ced2f2b4688737123c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attached video 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
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.