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 (deleted) —
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.
Attached file link to patch on github (deleted) —
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] link to patch on github Left some nits on Github
Attachment #8456657 - Flags: feedback?(wilsonpage) → feedback+
Comment on attachment 8456657 [details] 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] link to patch on github Ship it!
Attachment #8456657 - Flags: review?(wilsonpage) → review+
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)
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 (deleted) —
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: