Closed Bug 1037097 Opened 10 years ago Closed 10 years ago

[B2G][Gallery] Zooming all the way into an image causes Gallery to close and returns user to the Homescreen

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0+, b2g-v1.4 unaffected, b2g-v2.0 verified, b2g-v2.1 verified)

VERIFIED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v1.4 --- unaffected
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: jdegeus, Assigned: djf)

References

()

Details

(Keywords: memory-footprint, perf, regression, Whiteboard: [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0])

Attachments

(3 files)

Attached file logcat_close.txt
Description:
When users view an image within the Gallery app, if the user zooms all the way into the image, the Gallery app will close and return the user to the Homescreen


Repro Steps:
1) Update a Flame device to BuildID: 20140708000322
2) Set Flame to 273mb and reboot device
3) Select Gallery> Select an image(if none present take a image with Camera app)
4) Pinch outward to zoom into image
5) Observe app returns user to Homescreen


Actual:
Zooming all the way into an image causes Gallery to close, returning user to Homescreen
Expected:
Users remain in Gallery app

Environmental Variables:
Device: Flame 2.0 (273mb)
BuildID: 20140710000201
Gaia: 35a9b715e7348ec738ff6c8a59f50190390a06f2
Gecko: 94714370dfc3
Version: 32.0a2
Firmware Version: v122
User Agent: 2.0 - Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Repro frequency: 5/5
See attached: Logcat and Video attached
Logcat_close.txt - Video - http://youtu.be/r2fG4sWisO4
This issue DOES occur on Flame 2.1 (273mb). This issue does not occur as frequently on Flame 2.1 (273mb).


Flame 2.1 (273mb)

Environmental Variables:
Device: Flame Master
Build ID: 20140710040201
Gaia: 4e4e579b4b1e35f863ed43ef6ba840f49bfd761c
Gecko: cb75d6cfb004
Version: 33.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Actual: Users will be return to the Homescreen upon zooming all the way into an image.

-----------------------------------------------------------------------------------------------------

This DOES NOT occur on Flame Base v121-2 (273mb), Flame Base v122 (273mb), Flame 1.4(273mb), Flame 2.0(512mb), Buri 1.4, Buri 2.0, Buri 2.1, Open C 1.4, Open C 2.0, Open C 2.1.


Flame 2.0(512mb)

Environmental Variables:
Device: Flame 2.0
BuildID: 20140710000201
Gaia: 35a9b715e7348ec738ff6c8a59f50190390a06f2
Gecko: 94714370dfc3
Version: 32.0a2 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Flame 1.4(273mb)

Environmental Variables:
Device: Flame 1.4
Build ID: 20140710000202
Gaia: b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko: ccabaf8826a4
Version: 30.0 (1.4)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0


Flame Base v122 (273mb)

Environmental Variables:
Device: Flame 1.3
Build ID: 20140616171114
Gaia: e1b7152715072d27e0880cdc6b637f82fa42bf4e
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0


Flame Base v121-2 (273mb)

Environmental Variables:
Device: Flame 1.3
Build ID: 20140610200025
Gaia: e106a3f4a14eb8d4e10348efac7ae6dea2c24657
Gecko: b637b0677e15318dcce703f0358b397e09b018af
Version: 28.0 (1.3)
Firmware Version: v121-2
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0


Buri 2.1

Environmental Variables:
Device: Buri Master
Build ID: 20140709073020
Gaia: c394b7b4205b6f1a6ca44915fc08650f3ad127ec
Gecko: 2d88803a0b9c
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0


Buri 2.0 

Environmental Variables:
Device: Buri 2.0
Build ID: 20140709063007
Gaia: 1774027323bb072b4ebdfea9883572bcf2535c87
Gecko: 11b6493a7d8f
Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Buri 1.4

Environmental Variables:
Device: Buri 1.4
Build ID: 20140710000202
Gaia: b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko: ccabaf8826a4
Version: 30.0 (1.4)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0


Open C 2.1

Environmental Variables:
Device: Open_C Master
Build ID: 20140710071928
Gaia: 09642e74e250fbc62db860c808ef188628fca55d
Gecko: f93c0ef45597
Version: 33.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0


Open C 2.0

Environmental Variables:
Device: Open_C 2.0
Build ID: 20140710000201
Gaia: 35a9b715e7348ec738ff6c8a59f50190390a06f2
Gecko: 94714370dfc3
Version: 32.0a2 (2.0)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Open C 1.4

Environmental Variables:
Device: Open_C 1.4
Build ID: 20140710000202
Gaia: b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko: ccabaf8826a4
Version: 30.0 (1.4)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Actual: Users will be able to consistently zoom into the image without the Gallery app closing.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
This is a regression from 1.4. This issue would impact a lot of end users. Nominating this 2.0?
blocking-b2g: --- → 2.0?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: footprint, perf
Whiteboard: [273MB-Flame-Support], [2.0-exploratory] → [273MB-Flame-Support], [2.0-exploratory] [MemShrink]
blocking-b2g: 2.0? → 2.0+
QA Contact: pcheng
Unable to find a regression window as issue occurs on earliest tinderbox Flame Central build (4/17).
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
David, 

Can you investigate this?

Thanks
Hema
Assignee: nobody → dflanagan
Jordan,

What is the size (in megapixels) of the image you were observing this crash on?  (You can just attach the image if you are not sure).
Flags: needinfo?(jdegeus)
Jordan: would you try this:

  GAIA_MEMORY_PROFILE=256 APP=gallery make install-gaia

I expect that doing that will resolve the bug.  If so, I'll try to create a patch that detects the amount of memory at runtime rather than at build time.
I'm going to try fixing this by modifying shared/js/media/media_frame.js to call navigator.getFeature() to determine device memory and use that value to set the maximum decode size at runtime. This should fix gallery and also camera preview issues.
Diego: this is the patch I discussed with you and Wilson earlier today to query memory dynamically and base the max image decode size on that.  I'm asking Diego to review and am setting feedback? for Wilson and Justin since camera may need to do something similar. Wilson or Justin if you feel like doing the review on this, feel free to steal it from Diego.

If there are any open camera bugs on low-memory Flames having to do with displaying images in preview mode, then this patch may well resolve them. But note that to take advantage of this change in the shared MediaFrame module, the Camera app will need the new 'feature-detection' permission.

I think that if you added that permission to camera, then you might be okay to go back up to 5mp picture by default instead of 3mp.
Attachment #8456657 - Flags: review?(dmarcos)
Attachment #8456657 - Flags: feedback?(wilsonpage)
Attachment #8456657 - Flags: feedback?(jdarcangelo)
Pi Wei,

Could you check whether the attached patch resolves the bug for you?

Note that the patch modifies the manifest file for Gallery, so it is not enough to apply the patch and do a make install-gaia. You'll have to do make reset-gaia to get the manifest change to take effect.
Flags: needinfo?(pcheng)
(In reply to David Flanagan [:djf] from comment #9)
> Note that the patch modifies the manifest file for Gallery, so it is not
> enough to apply the patch and do a make install-gaia. You'll have to do make
> reset-gaia to get the manifest change to take effect.

I did a 'MOZILLA_OFFICIAL=1 make reset-gaia' to build the gaia with this patch.

On master the bug was happening less frequent, and with today's master it seems even harder to reproduce the bug. I managed to get the bug to happen twice in 10 minutes without the patch.
With the patch applied, no crash was observed in 15 minutes of testing.
Flags: needinfo?(pcheng)
Priority: -- → P1
Severity: normal → blocker
Whiteboard: [273MB-Flame-Support], [2.0-exploratory] [MemShrink] → [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0]
Attachment #8456657 - Flags: review?(dmarcos) → review+
Whiteboard: [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0] → [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0][landing eta 7/18]
Comment on attachment 8456657 [details] [review]
link to patch on github

Left some nits on Github
Attachment #8456657 - Flags: feedback?(wilsonpage) → feedback+
Comment on attachment 8456657 [details] [review]
link to patch on github

Wilson,

At your recommendation I've changed the code to make it testable and have added tests, fixing a bug in the process. (Thanks!) Would you take a quick look at the new version of the patch?
Attachment #8456657 - Flags: review?(wilsonpage)
Attachment #8456657 - Flags: review+
Attachment #8456657 - Flags: feedback?(jdarcangelo)
(In reply to David Flanagan [:djf] from comment #5)
> Jordan,
> 
> What is the size (in megapixels) of the image you were observing this crash
> on?  (You can just attach the image if you are not sure).

Hi Dave, my apologize for delay in response. I do not have the images this was occurring on anymore as i had tested the Find My Device Erase function.  

I have been unable to reproduce this issue on todays Flame build:
Environmental Variables:
Device: Flame 2.0
Build ID: 20140717000201
Gaia: aa4f795b81c6147d67c4f06009e166debcf8856e
Gecko: 0ec0b9ac39f0
Version: 32.0a2 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Flags: needinfo?(jdegeus) → needinfo?(dflanagan)
David -- I added some comments to the PR. In summary I raised following 2 issues:

1. I am not sure who we are dynamically determining the RAM size but if it's based on /proc/meminfo then please note that the available MemTotal in /proc/meminfo will be smaller than the provided RAM size.
e.g., 181MB of MemTotal for total RAM size of ~271MB which will put it in less than 256 category rather than in greater than 256 but less than 512.
2. The values of image sizes supported for different configurations should probably be encapsulated in a config so it's easier to change if needed in any case.
(In reply to Inder from comment #14)
> David -- I added some comments to the PR. In summary I raised following 2
> issues:
> 
> 1. I am not sure who we are dynamically determining the RAM size but if it's
> based on /proc/meminfo then please note that the available MemTotal in
> /proc/meminfo will be smaller than the provided RAM size.
> e.g., 181MB of MemTotal for total RAM size of ~271MB which will put it in
> less than 256 category rather than in greater than 256 but less than 512.

What we're querying dynamically is not the availble ram but the hardware configuration. So a 512mb device returns 512, not the smaller MemTotal value.

> 2. The values of image sizes supported for different configurations should
> probably be encapsulated in a config so it's easier to change if needed in
> any case.

Yes, but this is code shared by camera and gallery and I don't want to have to duplicate configuration files in both Camera and Gallery apps.  We still have build-time config options in gallery to limit the image decode size.  The new runtime check in MediaFrame is an extra layer of protection against OOM.
Flags: needinfo?(dflanagan)
Comment on attachment 8456657 [details] [review]
link to patch on github

Ship it!
Attachment #8456657 - Flags: review?(wilsonpage) → review+
Landed on master: https://github.com/mozilla-b2g/gaia/commit/24ada16dff049c657f37518f79eb964c491e64c6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
:djf, I am told that bug 1009645 will not be landed on 2.0. Is |navigator.getFeature('hardware.memory')| work on 2.0 without bug 1009645?

(If so, I plan to do the same thing on keyboard management.)
Flags: needinfo?(dflanagan)
Never mind, bug 983502 clearly reads it enables the support of |navigator.getFeature('hardware.memory')|, sorry about that.
Flags: needinfo?(dflanagan)
v2.0: https://github.com/mozilla-b2g/gaia/commit/68f8124da69fe661c3161bc7dce667db3900c928
Whiteboard: [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0][landing eta 7/18] → [273MB-Flame-Support], [2.0-exploratory] [MemShrink][c=memory p= s= u=2.0]
Target Milestone: --- → 2.0 S6 (18july)
Attached video video
This issue has been verified successfully on Flame 2.0 and 2.1
See attachment: Verify_1037097.MP4
Reproducing rate: 0/10
Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141130000204
Version         32.0

Flame 2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: