1049251 - [B2G][HERE Maps] HERE Maps is closed out by LMK when tapping "Nokia Beta Labs" link

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Performance, defect, P1)

Description

[Jordan de Geus(JordanD)]

Attachment: dmesg.txt

Description:
When users are in the HERE Now app and viewing the "About" tab, if the user selects the "Nokia Beta Labs" link listed under the "Help make HERE Maps even better"

Setup: 
Navigate to Marketplace> Search "HERE Now"> Install app

Repro Steps:
1) Update a Flame to 20140805000238
2) Launch "HERE Now"
3) Select the Menu button on top right corner> About
4) Tap on "Nokia Beta Labs"
5) Observe app eventually closes resulting in LMK

Actual:
Tapping "Nokia BEta Labs" within About causes app result in LMK

Expected:
App does not close out due to LMK

Environmental Variables:
Device: Flame 2.0 (319mb)
Build ID: 20140805000238
Gaia: 597975839c04e0198eb98c2c77474f057b5531e7
Version: 32.0 (2.0)
Gecko: ddeead927143
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Repro frequency: 3/5
See attached: Video, dmesg and Logcat
dmesg.txt - logcat_About.txt - http://youtu.be/DX8SrUlA5s4

Comment 1

[Jordan de Geus(JordanD)]

Attachment: logcat_About.txt

This issue DOES NOT occur on Flame 2.1 (319mb), Flame 2.0 (512mb), Flame 1.4 (319mb), Flame Base 122 (319mb), Flame Base 121-2 (319mb), Buri 2.1, Buri 2.0, Buri 1.4, Open C 2.1, Open C 2.0, and Open C 1.4

Actual: The app will not be closed out by LMK 

Flame 2.1 (319mb)

Environmental Variables:
Device: Flame Master
Build ID: 20140805040204
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 7f81be7db528
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0 (512mb)

Environmental Variables:
Device: Flame 2.0
Build ID: 20140805000238
Gaia: 597975839c04e0198eb98c2c77474f057b5531e7
Gecko: ddeead927143
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Flame 1.4 (319mb)

Environmental Variables:
Device: Flame 1.4
Build ID: 20140804183020
Gaia: 9377274b17200a60cebcd2427d489a7756c4cc72
Gecko: 24bc2ae19c59
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 122 (319mb)

Environmental Variables:
Device: Flame 1.3 (319mb)
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 121-2 (319mb)

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: 20140805073149
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 2aaedcdf69f6
Version: 34.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Buri 2.0

Environmental Variables:
Device: Buri 2.0
Build ID: 20140805063011
Gaia: 4959d1eb92fa82198409c90f471aad6d68c463b3
Gecko: c18fb97addb8
Version: 32.0 (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: 20140804183020
Gaia: 9377274b17200a60cebcd2427d489a7756c4cc72
Gecko: 24bc2ae19c59
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: 20140805040204
Gaia: 19bf9795263e2ccc15d824a52ebf23c2670fa9b9
Gecko: 7f81be7db528
Version: 34.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Open C 2.0 

Environmental Variables:
Device: Open_C 2.0
Build ID: 20140805000238
Gaia: 597975839c04e0198eb98c2c77474f057b5531e7
Gecko: ddeead927143
Version: 32.0 (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

v1.4 Environmental Variables:
Device: Open_C v1.4 MOZ
BuildID: 20140728000238
Gaia: 0a864988f5dce7f9f3dea9609e8ef054679c30ff
Gecko: 2da96d621030
Version: 32.0
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 2

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

[Blocking Requested - why for this release]:

Since this is a regression from 1.4 and the app is being force closed, I'm nominating 2.0?

Comment 3

[bhavana bajaj [:bajaj]]

Blocking given this is a regression in 2.0 and impacts a critical app not a TE issue.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Does this still reproduce on builds 20140806 and newer?  I'm just wondering if bug 1045680 helps.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

I'm unable to reproduce this LMK on today's build in 319MB following STR. Repro rate 0/5.
I also tried a 2.0 build on 8/5 when this bug was filed and couldn't repro either.
Removing window-wanted tag and adding qawanted so others could try to repro.

Comment 6

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I was able to reproduce this issue on today's 2.0 build as well as the reporter's build.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140807094215
Gaia: bb3d2cf36c10f7186899431d4a21a2489864d9c6
Gecko: 4ad32f9626ad
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 7

[Joshua Mitchell (Inactive)]

returning Regression-Window-Wanted based on the ability to repro

Comment 8

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I was also able to reproduce this issue on 2.1 Flame.

Environmental Variables:
Device: Flame Master
BuildID: 20140805081353
Gaia: e93780f9da8b34f370a4113abd4df9780d58e443
Gecko: b50bb656674e
Version: 34.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

1.4 Flame did not reproduce though.

Environmental Variables:
Device: Flame 1.4
BuildID: 20140807110116
Gaia: e9dce1f60f729e228810f751417681b5ff937b6b
Gecko: 509ebd2e22f7
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 9

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Mozilla-inbound Regression Window

Last working 
Environmental Variables:
Device: Flame Master
BuildID: 20140429163002
Gaia: db3bcec51a361daddb7d3d4ba4d8a2a664b7b6aa
Gecko: de19c62cbc6b
Version: 32.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0



First Broken 
Environmental Variables:
Device: Flame Master
BuildID: 20140429193004
Gaia: db3bcec51a361daddb7d3d4ba4d8a2a664b7b6aa
Gecko: c0d658d3f739
Version: 32.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Last working gaia / First broken gecko - Issue DOES occur
Gaia: db3bcec51a361daddb7d3d4ba4d8a2a664b7b6aa
Gecko: c0d658d3f739

First broken gaia / Last working gecko - Issue does NOT occur

Gecko Pushlog:  https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=de19c62cbc6b&tochange=c0d658d3f739

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0d658d3f739 is the likely candidate.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Well, bisecting with a flame using low-memory profile is likely to point at that commit.  It uses a heuristic based on screen size.  As far as I know we do nothing to reduce the high dpi of the device when we restrict its memory.  So the heuristic is probably wrong for such a device.  That may not be representative of real devices, though.

It seems to me skia just started using a lot more memory at some point as reflected in bugs like bug 990174.  That should be root caused.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

SkiaGL uses more memory and makes things go faster.  Every time we make a change that pushes more scenarios into SkiaGL (increasing the maximum size, decreasing the minimum size) or increase the maximum size of the SkiaGL cache, we will use more memory.  It's a feature.  And then, there are the memory leaks, bug 990174 potentially being one of them.
Just turn off SkiaGL completely, and see if the problem goes away? Set gfx.canvas.azure.accelerated to false.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Milan Sreckovic [:milan] (PTO 8/11 - 8/15) from comment #12)
> cache, we will use more memory.  It's a feature.  And then, there are the
> memory leaks, bug 990174 potentially being one of them.
> Just turn off SkiaGL completely, and see if the problem goes away? Set
> gfx.canvas.azure.accelerated to false.

From what I understand we import new skia versions regularly.  Do we have any tests to catch memory regressions when we do this?  It seems like bug 990174 and the issue it blocks started around one of these imports.

Comment 14

[Mason Chang [Inactive] [:mchang]]

Triage: Since this is a third party app, Harold can you please take a look at this?

Comment 15

[Mason Chang [Inactive] [:mchang]]

I misread the comments, since this is a regression in gecko, this is our responsibility.

Comment 16

[Kai-Chih Hu [:khu]]

(In reply to Ben Kelly [:bkelly] from comment #13)
> (In reply to Milan Sreckovic [:milan] (PTO 8/11 - 8/15) from comment #12)
> > cache, we will use more memory.  It's a feature.  And then, there are the
> > memory leaks, bug 990174 potentially being one of them.
> > Just turn off SkiaGL completely, and see if the problem goes away? Set
> > gfx.canvas.azure.accelerated to false.
> 
> From what I understand we import new skia versions regularly.  Do we have
> any tests to catch memory regressions when we do this?  It seems like bug
> 990174 and the issue it blocks started around one of these imports.

Milan, do you have any idea about this? Looks like you're on PTO. Ni you to make sure you're aware of the comment made by Ben. 

Thanks.

Comment 17

[Ting-Yu Chou [:ting] (away)]

I was trying to reproduce this, and there's once HERE gets LMKed after I tap "share". It seems memory was consumed very fast, which not only HERE but alsy Marketplace and Homescreen were killed at the same time.

I haven't reproduced the same symptom of this bug.

Comment 18

[Milan Sreckovic [:milan] (needinfo for best results)]

Don't know of anything in particular that tries to catch increased memory usage with canvas and new versions of SkiaGL.  I'm not sure if this is still reproducible, but if it is, see my comment 12 for something to try to make sure it is SkiaGL caching that is causing trouble here.

Comment 19

[Ken Chang[:kenkai]]

Hi Jayme, according to comment 17 and comment 18, can you set gfx.canvas.azure.accelerated to false and try again? Thanks.

Comment 20

[Jason Smith [:jsmith]]

QA Wanted for comment 19

Comment 21

[Ting-Yu Chou [:ting] (away)]

I just managed to repro now, set high resolution map in preference menu will make it easily reproduced (even without tapping "Nokia Beta Labs"). /d/kgsl/proc/[pid]/mem shows kgsl texture taking dozens of MBs like bug 1049195.

Should I mark this a dup direclty or have QA's double confirm?

Comment 22

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Removing the needinfo? request on me since there is a qawanted tag.  If someone else from the qawanted team can handle this sooner that would be great.

Comment 23

[Joshua Mitchell (Inactive)]

Jayme - that is fine

Comment 24

[bhavana bajaj [:bajaj]]

Marking DUP per comment #21.

Comment 25

[Sotaro Ikeda [:sotaro]]

I do not think this is a dup of bug 1049195, even though the problems started to happen by using SkiaGL in the STR. From Comment 8, this bug's problem happens also on b2g-v2.1. But bug 1049195 happens only on b2g v2.0.

And bug 1049195 needs only "HERE Maps" process alive, but this bug needs Browser and "HERE Maps" processes need to be alive and Browser is in the Top.

Comment 26

[Sotaro Ikeda [:sotaro]]

From Bug 1049195 Comment 55, I tested to call glFlush() for each draw calls. But the "HERE Maps" was occasionally killed by LMK.

Comment 27

[Sotaro Ikeda [:sotaro]]

I confirmed a wired thing. Even when SkiaGL is disabled, I still saw the crash.

Comment 28

[Sotaro Ikeda [:sotaro]]

The following the code is the code to open "Nokia Beta Labs".

> <a href="https://betalabs.nokia.com/apps/nokia-maps-for-mobile-web/forum" class="forumlink" target="_blank" onclick="if(event._fake) {event.preventDefault();event.stopPropagation();}">Nokia Beta Labs</a>

It trigger browser app's tab open and the "HERE Maps" app is set to background state. The "HERE Maps"'s state is just a normal background app. Within background apps, more recently used app has higher priority.
But it's priority is lower than background Homescreen.

Application's priority becomes the following.
 Foreground app > background homescreen > most recently used background app > other background apps.

Therefore, this bug requests three apps "one browser tab"(foreground), homescreen, HERE Maps(background) are alive during the STR.


More precise priority is defined by the following.
  http://mxr.mozilla.org/mozilla-central/source/hal/HalTypes.h#81

Comment 29

[Sotaro Ikeda [:sotaro]]

This bug says that RAM usage is increased since v1.4. But the b2g system does not ensure/require that the most recently used background app is always alive.

Comment 30

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #27)
> I confirmed a wired thing. Even when SkiaGL is disabled, I still saw the
> crash.

I modified wrong code:-( I am going to confirm it again.

Comment 31

[Sotaro Ikeda [:sotaro]]

I re-confirmed that HERE Maps was killed even when SkiaGL was disabled.

Comment 32

[Sotaro Ikeda [:sotaro]]

By using "adb shell b2g-ps -p", I check process/memory states of the followings. On b2.0, HERE Maps was killed even with SkiaGL was disabled. When SkiaGL is enabled, HERE Maps is killed more often than disabled.
- b2g-v1.4 flame 319MB
- b2g-2.0 SkiaGL enabled(before HERE Maps killed) flame 319MB
- b2g-2.0 SkiaGL disabled(before HERE Maps killed) flame 319MB


** b2g-v1.4 flame 319MB **

APPLICATION      USER     PID   PPID  VSIZE  RSS   PRIO  NICE  RTPRI SCHED
b2g              root      290   1     237308 37544 20    0     0     0     
(Nuwa)           root      906   290   53364  4952  20    0     0     0     
Homescreen       u0_a967   967   906   72052  11412 38    18    0     0     
HERE Maps        u0_a1204  1204  906   79620  19848 38    18    0     0     
(Preallocated a  u0_a1255  1255  906   60536  6716  38    18    0     0     
Browser          u0_a1300  1300  290   76312  8764  38    18    0     0     
Browser          u0_a1656  1656  290   78104  9856  38    18    0     0     
Browser          u0_a1704  1704  290   78040  9864  38    18    0     0     
Browser          u0_a1747  1747  290   78040  15436 38    18    0     0     
Browser          u0_a1804  1804  290   87136  34928 21    1     0     0     
  
** b2g-2.0 SkiaGL enabled(before HERE Maps killed) flame 319MB **

APPLICATION    SEC USER     PID   PPID  VSIZE  RSS   PRIO  NICE  RTPRI SCHED
b2g              0 root      2314  1     231832 45164 20    0     0     0  
(Nuwa)           0 root      2412  2314  54964  2232  20    0     0     0 
Homescreen       2 u0_a3081  3081  2412  78676  12528 38    18    0     0 
HERE Maps        2 u0_a3406  3406  2412  119392 44672 27    7     0     0 
Browser          2 u0_a3467  3467  2412  81488  26576 21    1     0     0 
(Preallocated a  0 u0_a3506  3506  2412  59124  9504  38    18    0     0 

** b2g-2.0 SkiaGL disabled(before HERE Maps killed) flame 319MB ** 

APPLICATION    SEC USER     PID   PPID  VSIZE  RSS   PRIO  NICE  RTPRI SCHED
b2g              0 root      292   1     226660 63272 20    0     0     0
(Nuwa)           0 root      852   292   54964  8932  20    0     0     0
Homescreen       2 u0_a879   879   852   76620  23184 38    18    0     0
HERE Maps        2 u0_a1206  1206  852   76772  25160 38    18    0     0
Browser          2 u0_a1630  1630  852   79060  27608 21    1     0     0
(Preallocated a  2 u0_a1656  1656  852   61176  15380 38    18    0     0

Comment 33

[Sotaro Ikeda [:sotaro]]

On b2g-v1.4, a lot of browser tabs were alive. But on b2g, only one Borwser Tab was alive, even when SkiaGL was disabled. And all b2g-v1.4's processes uses less memory compared to b2g-v2.0.

Comment 34

[Sotaro Ikeda [:sotaro]]

Attachment: about-memory b2g-v1.4

Comment 35

[Sotaro Ikeda [:sotaro]]

Attachment: about-memory disable SkiaGL b2gv2.0

Comment 36

[Sotaro Ikeda [:sotaro]]

On b2g-v2.0, when just start the STR, there is a case that multiple Browser tab's processes exist. All browser tab's rss seems a lot bigger than b2g-v1.4. On 1.4, active Tab's RSS is big, but other browser tab's process' RSS is small.

** b2g-2.0 SkiaGL disabled(before HERE Maps killed) flame 319MB ** 

APPLICATION    SEC USER     PID   PPID  VSIZE  RSS   PRIO  NICE  RTPRI SCHED
b2g              0 root      279   1     221968 65140 20    0     0     0 
(Nuwa)           0 root      867   279   54964  9308  20    0     0     0 
Homescreen       2 u0_a905   905   867   76612  23684 38    18    0     0 
HERE Maps        2 u0_a1211  1211  867   75748  28020 38    18    0     0  
Browser          2 u0_a1359  1359  867   73824  26080 38    18    0     0  
Browser          2 u0_a1408  1408  867   73504  26456 38    18    0     0   
Browser          2 u0_a1442  1442  867   79316  28200 21    1     0     0   
(Preallocated a  2 u0_a1484  1484  867   61112  16036 38    18    0     0

Comment 37

[Sotaro Ikeda [:sotaro]]

Attachment: about-memory disable SkiaGL b2gv2.0 with multiple browser tabs

Comment 38

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #36)
> On b2g-v2.0, when just start the STR, there is a case that multiple Browser
> tab's processes exist. All browser tab's rss seems a lot bigger than
> b2g-v1.4. On 1.4, active Tab's RSS is big, but other browser tab's process'
> RSS is small.

On b2g-v2.0, when RAM size setting is changed from 319MB to 340MB, browser tab's process' shrink was also observed.

Comment 39

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #38)
> 
> On b2g-v2.0, when RAM size setting is changed from 319MB to 340MB, browser
> tab's process' shrink was also observed.

But the shrink is not big enough compared to b2g-v1.4(319MB).

On b2g-v2.0(RAM was 340MB) the result was following.
- SkiaGL On: HERE Maps was killed by the STR
- SkiaGL Off: HERE Maps was not killed by the STR

Comment 40

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #32)
> By using "adb shell b2g-ps -p", I check process/memory states of the
> followings. On b2.0, HERE Maps was killed even with SkiaGL was disabled.
> When SkiaGL is enabled, HERE Maps is killed more often than disabled.
> - b2g-v1.4 flame 319MB
> - b2g-2.0 SkiaGL enabled(before HERE Maps killed) flame 319MB
> - b2g-2.0 SkiaGL disabled(before HERE Maps killed) flame 319MB
> 
> 
> ** b2g-v1.4 flame 319MB **
> 
> APPLICATION      USER     PID   PPID  VSIZE  RSS   PRIO  NICE  RTPRI SCHED
> b2g              root      290   1     237308 37544 20    0     0     0     
> (Nuwa)           root      906   290   53364  4952  20    0     0     0     
> Homescreen       u0_a967   967   906   72052  11412 38    18    0     0     
> HERE Maps        u0_a1204  1204  906   79620  19848 38    18    0     0     
> (Preallocated a  u0_a1255  1255  906   60536  6716  38    18    0     0     
> Browser          u0_a1300  1300  290   76312  8764  38    18    0     0     
> Browser          u0_a1656  1656  290   78104  9856  38    18    0     0     
> Browser          u0_a1704  1704  290   78040  9864  38    18    0     0     
> Browser          u0_a1747  1747  290   78040  15436 38    18    0     0     
> Browser          u0_a1804  1804  290   87136  34928 21    1     0     0     

khuey, on b2g-v1.4 flame, background's Browser tab's rss' were gradually decreased. Were they swapped out to zram? Or, are they decreased by other reason? Thanks.

Comment 41

[Sotaro Ikeda [:sotaro]]

In the STR, when more than 4 Browser Tab's were opened, their RSS stared to decreased. On b2g-v2.0 flame(319MB), in the STR, at most 3 Browser Tabs were opened. Therefore, I did not saw Browser Tabs RSS decrease on b2g-v2.0 flame(319MB).

Comment 42

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #31)
> I re-confirmed that HERE Maps was killed even when SkiaGL was disabled.

On latest v2.0, just disabling SkiaGL does not fix the problem, though it makes the symptom better. It seems better to handle canvas 2d's memory usage problem in a different bug.

Comment 43

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #42)
> On latest v2.0, just disabling SkiaGL does not fix the problem, though it
> makes the symptom better. It seems better to handle canvas 2d's memory usage
> problem in a different bug.

But it just means that disabling SkiaGL on low memory device(less than 319MB) when the canvas size is larger than its screen size. It is partial revert of Bug 999841. It disable SkiaGL on some condition on low memory device, but it is not a regression since b2g-v1.4.

Comment 44

[Sotaro Ikeda [:sotaro]]

Bug 1042305 improve the situation. On latest v2.0. The problem happens when I opened 5th Browser Tab in the STR in Comment 0.

Comment 45

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #44)
> Bug 1042305 improve the situation. On latest v2.0. The problem happens when
> I opened 5th Browser Tab in the STR in Comment 0.

When SkiaGL is disabled, the problem happens when I opened 8th-10th Browser Tabs on Flame(319MB). Therefore, Skia seems to use less RAM than SkiaGL, but on latest v2.0 does not fix the problem even with SkiaGL is disabled.

Comment 46

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

They might be swapped out.  b2g-info will tell you that.

Comment 47

[Sotaro Ikeda [:sotaro]]

I talked with milan offline. We agreed to disable SkiaGL if canvas size is larger than screen size on low memory device.

Comment 48

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sotaro Ikeda [:sotaro] from comment #47)
> I talked with milan offline. We agreed to disable SkiaGL if canvas size is
> larger than screen size on low memory device.

Don't we already do this?  I tried to add it here:

http://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#864

Comment 49

[Sotaro Ikeda [:sotaro]]

Yhea, that code already do almost all. We need to add disabling SkiaGL from screen size to 980*480 on low memory devices.

Comment 50

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sotaro Ikeda [:sotaro] from comment #49)
> Yhea, that code already do almost all. We need to add disabling SkiaGL from
> screen size to 980*480 on low memory devices.

Ok, then lets make sure people don't expect games or apps like FishIETank's mobile version to run at high fps on these devices.

Comment 51

[Sotaro Ikeda [:sotaro]]

I tested on v2.0 flame JB about how much memory is necessary to fix the problem. When flame's total memory was 350MB, the problem was fixed and total system memory size was 243MB.

- 319MB: Total System Memory size: 213MB
- 330MB: Total System Memory size: 224MB
- 350MB: Total System Memory size: 243MB

Comment 52

[Sotaro Ikeda [:sotaro]]

Frame's screen size is 480*854 and default scale is 1.5. Then, 720*1281 becomes SkiaGL usage's threshold. On low memory device, SkiaGL need to be disabled if the canvas size is more than screen buffer size.

Comment 53

[Ben Kelly [:bkelly, not reviewing]]

Keep in mind our default displayport is 480*980, even if its bigger than the physical screen.

Comment 54

[Sotaro Ikeda [:sotaro]]

I am not sure why CanvasRenderingContext2D::CheckSizeForSkiaGL() cares about a default scale. Flame has 1.5 default scale. But if I create an application that have 480*854 size canvas. The canvas was always fullscreen size(not scaled) except APZ was enabled. Application did not scale canvas size except APZ is enabled.

AppUnits to GfxUnits transform is done by nsPresContext::AppUnitsToGfxUnits(). 
> nsPresContext::AppUnitsToGfxUnits()
> ->nsDeviceContext::AppUnitsToGfxUnits()
> ->nsDeviceContext::AppUnitsPerDevPixel()
> ->return nsDeviceContext::mAppUnitsPerDevPixel;

nsDeviceContext::mAppUnitsPerDevPixel is initialized to 1.5 at first. But it is soon overrided to 1.0 by nsPresContext::Init().
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#960

Therefore canvas always seems rendered by 1.0 scale by default except APZ enabled.

Comment 55

[Sotaro Ikeda [:sotaro]]

bkelly, snorp can you comment to Comment 54?

Comment 56

[Sotaro Ikeda [:sotaro]]

kats, do you know about Comment 54?

Comment 57

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #54)
> I am not sure why CanvasRenderingContext2D::CheckSizeForSkiaGL() cares about
> a default scale. Flame has 1.5 default scale. But if I create an application
> that have 480*854 size canvas. The canvas was always fullscreen size(not
> scaled) except APZ was enabled. Application did not scale canvas size except
> APZ is enabled.
> 
> AppUnits to GfxUnits transform is done by
> nsPresContext::AppUnitsToGfxUnits(). 
> > nsPresContext::AppUnitsToGfxUnits()
> > ->nsDeviceContext::AppUnitsToGfxUnits()
> > ->nsDeviceContext::AppUnitsPerDevPixel()
> > ->return nsDeviceContext::mAppUnitsPerDevPixel;
> 
> nsDeviceContext::mAppUnitsPerDevPixel is initialized to 1.5 at first. But it
> is soon overrided to 1.0 by nsPresContext::Init().

Correction.
nsDeviceContext::mAppUnitsPerDevPixel is 40(60/1.5) at first.

Comment 58

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Partially disable SkiaGL on Low total system memory

Comment 59

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2gv2.0 - Partially disable SkiaGL on Low total system memory

Comment 60

[Sotaro Ikeda [:sotaro]]

By applying the patch, SkiaGL is disabled in HERE Maps on flame(319MB). It change the symptom better. But the problem is still exist. graphics memory usage is already minimized. b2gv2.0's memory usage is very high than b2gv1.4. This seems not a graphics problem, but general memory usage problem.

After the canvas' fix. If this is still judged as a blocker of b2gv2.0. It seems better to handle the problem as a different bug.

Comment 61

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=3a09f8c9c7fe

Comment 62

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sotaro Ikeda [:sotaro] from comment #54)
> I am not sure why CanvasRenderingContext2D::CheckSizeForSkiaGL() cares about
> a default scale. Flame has 1.5 default scale. But if I create an application
> that have 480*854 size canvas. The canvas was always fullscreen size(not
> scaled) except APZ was enabled. Application did not scale canvas size except
> APZ is enabled.

I'm sorry, I don't remember exactly.  I seem to recall it was suggested by :kats and :mwu. See bug 999841 comment 8.

Comment 63

[Kartikaya Gupta (email:[e-mail])]

I don't really recall anything about this. I looked at the IRC conversation I think that comment is referencing [1] and I don't see anything about default-scale values in there. Maybe I found the wrong conversation since that one didn't have mwu.

[1] http://logs.glob.uno/?c=mozilla%23gfx&s=28+Apr+2014&e=28+Apr+2014#c163463

Comment 64

[Kartikaya Gupta (email:[e-mail])]

Attachment: old irc conversation

Ah, I think I found the conversation in question. From the sounds of what mwu was saying it looks like the 1.5 device scale was being applied, which contradicts what sotaro was saying in comment 54. Or maybe the device scale is ignored in fullscreen mode and mwu wasn't viewing in fullscreen? I don't really know.

Comment 65

[Sotaro Ikeda [:sotaro]]

mwu, do you know something about Comment 64?

Comment 66

[Michael Wu [:mwu]]

Sotaro, I think the 1.5x scaling was for pages doing 100% x 100% sized canvases without a meta viewport tag. Not 100% sure. I was also testing in a browser, so it wasn't fullscreen, if that makes any difference.

Comment 67

[Sotaro Ikeda [:sotaro]]

Thanks, it becomes clear.

Comment 68

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8487183 [details] [diff] [review]
patch - Partially disable SkiaGL on Low total system memory

Review of attachment 8487183 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/CanvasRenderingContext2D.cpp
@@ +889,5 @@
>    static int32_t gScreenPixels = -1;
>    if (gScreenPixels < 0) {
>      // Default to historical mobile screen size of 980x480.  In addition,
>      // allow skia use up to this size even if the screen is smaller.  A lot
>      // content expects this size to work well.

Add a comment here about which content this is referring to.

@@ +929,2 @@
>      }
>    }

Let's just always have a scale of 1.0. We can always change this if we find a lot of content that needs it.

Comment 69

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Partially disable SkiaGL on Low total system memory

Apply the comments. Carry "r= jrmuizel".

Comment 70

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #68)
> >      // Default to historical mobile screen size of 980x480.  In addition,
> >      // allow skia use up to this size even if the screen is smaller.  A lot
> >      // content expects this size to work well.
> 
> Add a comment here about which content this is referring to.

If I understand correctly, this is practically all content on mobile that uses device width/height because we default out display port 980x480.

> Let's just always have a scale of 1.0. We can always change this if we find
> a lot of content that needs it.

Well, I'm fairly certain not using the real scale broke the FishIETank benchmark and other GL games performance.  That's why I was asked to add it in bug 999841.

Comment 71

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2gv2.0 - Partially disable SkiaGL on Low total system memory

Comment 72

[Sotaro Ikeda [:sotaro]]

(In reply to Ben Kelly [:bkelly] from comment #70)
> > Let's just always have a scale of 1.0. We can always change this if we find
> > a lot of content that needs it.
> 
> Well, I'm fairly certain not using the real scale broke the FishIETank
> benchmark and other GL games performance.  That's why I was asked to add it
> in bug 999841.

Are you going to say canvas2d Game? WebGL is different.

Comment 73

[Ben Kelly [:bkelly, not reviewing]]

Sorry, I mispoke.  I meant canvas related games.

Comment 74

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Partially disable SkiaGL on Low total system memory

Fix nits.

Comment 75

[Sotaro Ikeda [:sotaro]]

Attachment: patch for b2gv2.0 - Partially disable SkiaGL on Low total system memory

Fix nits.

Comment 76

[Sotaro Ikeda [:sotaro]]

I checked a canvas size on popular game applications on Firefox Marketplace. All application's game canvas sizes except "Penguin Pop" were smaller than Flame's display size. Though there are some cases that the max canvas size is more than display size.

<Canvas size on Flame(854*480)>

- Pacman Canvas (Beta):
     + max canvas size: 540*390
- Penguin Pop:
     + max canvas size: 700*881
- Zombie Apocalypse
     + game screen canvas: 320*545
     + max canvas size: 480*690
- CutTheRope
     + max canvas size: 480*320
- Candy Crush
     + max canvas size: 639*567
- Slice Fruits
     + max canvas size: 512*512
- Rooftops
     + max canvas size: 480*300
- Blozzle
     + canvas 2d seems not used
- Pasjans/Solitaire
     + canvas 2d seems not used
- AirCombat
     + max canvas size: 320*480
- Fruit Cut Ninja
     + game screen canvas: 320*480
     + max canvas size: 480*818
- DuckShooting
     + canvas 2d seems not used
- BlockedCar
     + game screen canvas: 320*569
     + max canvas size:800*1200
- Chess
     + max canvas size: 304*304
- Hora de la aventura
     + canvas 2d seems not used
- Finger Print
     + canvas 2d seems not used
- GoldNuggets
     + canvas 2d seems not used
- Tulis Farm
     + canvas 2d seems not used

Comment 77

[Sotaro Ikeda [:sotaro]]

I checked FishIEtank on flame.

On portrait, at first canvas size was small. It seems Bug 1059483. In this case, the canvas size was 320*451. By using pinch zoon, the canvas becomes bigger.When the canvas fit to screen, the canvas size was 980*1380. on Landscape, canvas size was 980*347.

FishIEtank's canvas width seems fixed to 980 when it fits to screen on flame. From it, the default scale(1.5 on flame) seems not related to FishIEtank case.

Comment 78

[Sotaro Ikeda [:sotaro]]

attachment 8488094 [details] [diff] [review] is not a regression from b2g-v1.4. From Comment 76, game screen canvas seems normally created within screen display size.

Comment 79

[Sotaro Ikeda [:sotaro]]

width=980 seems to come from the following.

> static const int32_t  kViewportDefaultScreenWidth = 980;
  http://dxr.mozilla.org/mozilla-central/source/content/base/public/nsViewportInfo.h#19

But somehow, its value seems to be used in FishIEtank case. Though, from Bug 999841 Comment 3, FishIEtank has the following viewport tag.

> <meta name="viewport" content="width=480, initial-scale=1, maximum-scale=1, user-scalable=no"/>

Comment 80

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #79)
> width=980 seems to come from the following.
> 
> > static const int32_t  kViewportDefaultScreenWidth = 980;
>  
> http://dxr.mozilla.org/mozilla-central/source/content/base/public/
> nsViewportInfo.h#19
> 
> But somehow, its value seems to be used in FishIEtank case. Though, from Bug
> 999841 Comment 3, FishIEtank has the following viewport tag.

It seems a problem of Bug 1059483.

Comment 81

[Sotaro Ikeda [:sotaro]]

When canvas element is created without viewport tag, the canvas does not fill the screen. From comment 66, at first it is because of default scaling(1.5x on Flame). But it seems to be caused by "kViewportDefaultScreenWidth = 980".

The following link is one example.
  http://people.mozilla.org/~sikeda/simple_canvas.html

Comment 82

[Sotaro Ikeda [:sotaro]]

Attachment: Screenshot of Comment 81's simple_canvas.html on flame.

Canvas' width is almost 1/2 of display. From it, "kViewportDefaultScreenWidth = 980" seems valid.

Comment 83

[Sotaro Ikeda [:sotaro]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27ce050ada04

Comment 84

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/27ce050ada04

Comment 85

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8488094 [details] [diff] [review]
patch - Partially disable SkiaGL on Low total system memory

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 999841
User impact if declined: some application might be killed more because of oom.
Testing completed: locally tested
Risk to taking this patch (and alternatives if risky): low.
String or UUID changes made by this patch:

Comment 86

[bhavana bajaj [:bajaj]]

Comment on attachment 8488094 [details] [diff] [review]
patch - Partially disable SkiaGL on Low total system memory

[Triage Comment]

Sotaro, I am approving this to land on aurora as well to fix the issue in 2.1

Rya, lets hope the patch applies cleanly:P else NI sotaro.

Comment 87

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/475dda11aded
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/bb6a6337270c

Comment 88

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/bb6a6337270c

Comment 89

[Yeojin Chung [:YeojinC]]

This issue is verified on Flame 2.2 and Flame 2.1:

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141006040204
Gaia: 470826d13ae130a5c3d572d1029e595105485fb0
Gecko: e0d714f43edc
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141006000205
Gaia: 778ebac47554e1c4b7e9a952d73e850f58123914
Gecko: c4a4b04c617c
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The app does not close out. Tapping the link displays "This Connection is Untrusted" page, and the user can access the page after selecting to proceed.

Comment 90

[Adam Alldredge [:AdamA][Inactive]]

This issue is verified fixed on Flame 2.0.

Flame 2.0 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.0
BuildID: 20141006000202
Gaia: 092d2b7678774c8b0b06dca0e0a8119e9eafdec3
Gecko: 69ca61f7edf3
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

The app does not close out. Tapping the link displays "This Connection is Untrusted" page, and the user can access the page after selecting to proceed.

Comment 91

[Coler]

This issue has been verified successfully on Woodduck 2.0.
Reproducing rate: 0/5
See attachment: Verify_Woodduck_Heremap.MP4

build version:
Gaia-Rev        87b23fa81c3b59f2ba24b84f7d686f4160d4e7cb
Gecko-Rev       d049d4ef127844121c9cf14d2e8ca91fd9045fcb
Build-ID        20141124050313
Version         32.0

Comment 92

[Coler]

Attachment: Verify_Woodduck_Heremap.MP4

Comment 93

[William Hsu [:whsu]]

Remove "verifyme" from Keywords field because we have verified this bug on v2.0 and v2.0m build.