1121871 - [Flame][Browser]Pinch to zoom in screen several times, device will auto exit browser.

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[Shine(Leave from Mozilla)]

Attachment: logcat

[1.Description]:
According to comment#5 in Bug 1120875, this bug is filed
[Flame][v2.1&v2.2][Browser]Launch Browser and link to any webpage, pinch to zoom in screen several times, device will auto exit browser and go back to Home screen.
See attachment: logcat_1524.txt & Video1.MP4 & WebIDE Monitor Screenshot.jpg
Found time:15:24

[2.Testing Steps]: 
1. Launch Browser and link to any webpage, such as: www.sina.com.
2. Pinch to zoom in screen several times.


[3.Expected Result]: 
2. User could zoom in webpage normally and it shouldn't auto exit Browser.

[4.Actual Result]: 
2. Device will auto exit browser and go back to Home screen.

[5.Reproduction build]: 
Flame 2.1 build:
Gaia-Rev        6957ac8a322234ec99c8abb7cc18dc6a2e0176db
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/6600eba54256
Build-ID        20150114001300
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150114.035135
FW-Date         Wed Jan 14 03:51:46 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 build:
Gaia-Rev        7c5b27cad370db377b18a742d3f3fdb0070e899f
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/748b20315f75
Build-ID        20150114002502
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150114.040029
FW-Date         Wed Jan 14 04:00:40 EST 2015
Bootloader      L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

[8.Note]:
The repro rate on Flame2.1 is 2/5

Comment 1

[Shine(Leave from Mozilla)]

Attachment: Video

Comment 2

[Shine(Leave from Mozilla)]

Attachment: WebIDE Monitor Screenshot

Comment 3

[William Hsu [:whsu]]

Hi, Shine,

May I know the memory configuration?
319 MB or 1 GB?
Thanks.

Comment 4

[Shine(Leave from Mozilla)]

Hi William,
The memory of Flame 2.1/2.2 are 319MB.
Thanks!

Comment 5

[William Hsu [:whsu]]

(In reply to Shine from comment #4)
> Hi William,
> The memory of Flame 2.1/2.2 are 319MB.
> Thanks!

Thanks Shine!

So, this bug may relate to memory pressure.

Comment 6

[William Hsu [:whsu]]

Hi, Shine,

Does it happen on v2.0 or v2.0m?
Many thanks.

Comment 7

[William Hsu [:whsu]]

01-15 15:24:58.813 I/GonkMemoryPressure(  205): Checking to see if memory pressure is over.
01-15 15:24:58.813 I/GonkMemoryPressure(  205): Memory pressure is over.
01-15 15:24:58.963 I/GonkMemoryPressure( 4078): Checking to see if memory pressure is over.
01-15 15:24:58.963 I/GonkMemoryPressure( 4078): Memory pressure is over.

I saw the information. It seems relate to memory pressure.
Bad user experience. I will nominate this bug after we do a branch check.

Comment 8

[Shine(Leave from Mozilla)]

Hi William,
his problem is verified not to happen on Flame2.0 and woodduck2.0 
Thanks!

Woodduck 2.0 build:
Gaia-Rev        22e3b7855281aa7b6190e01b4bd50c79880a1e6a
Gecko-Rev       20943fe7b54c63a375952fbd8dd396a22ee893e7
Build-ID        20150116050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1421355897
FW-Date         Fri Jan 16 05:05:17 CST 2015

Flame 2.0 build:
Gaia-Rev        736933b25ded904f0cb935a0d48f1f3cf91d33ad
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8ff0d933175c
Build-ID        20150115000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150115.031720
FW-Date         Thu Jan 15 03:17:28 EST 2015
Bootloader      L1TC000118D0

Comment 9

[William Hsu [:whsu]]

[Blocking Requested - why for this release]:

Bad user experience. This bug may relate to app crash.

Request a Repair. Nominate it.

Comment 10

[bhavana bajaj [:bajaj]]

(In reply to William Hsu [:whsu] from comment #9)
> [Blocking Requested - why for this release]:
> 
> Bad user experience. This bug may relate to app crash.
> 
> Request a Repair. Nominate it.

Can we get a regression window here to help narrow down the range here? I am NI'ing :kats to see if there is an obvious issue here given this case is not reproducible in 2.0

Comment 11

[William Hsu [:whsu]]

(In reply to bhavana bajaj [:bajaj] from comment #10)
> (In reply to William Hsu [:whsu] from comment #9)
> > [Blocking Requested - why for this release]:
> > 
> > Bad user experience. This bug may relate to app crash.
> > 
> > Request a Repair. Nominate it.
> 
> Can we get a regression window here to help narrow down the range here? I am
> NI'ing :kats to see if there is an obvious issue here given this case is not
> reproducible in 2.0

Thanks for Bhavana's reminder.

Adding "regressionwindow-wanted" tag.

Comment 12

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

I can repro on master, will investigate.

Comment 13

[William Hsu [:whsu]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> I can repro on master, will investigate.

Thanks Kartikaya! :)

Comment 14

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

Annoyingly it seems to only happen intermittently. So far I haven't been able to reproduce with on my local build with additional logging (which would help me isolate the root cause). I have seen it happen three times but always on a build with insufficient logging info :(

I'll keep trying but I'm starting to wonder if it's related to a particular set of banner ads at the bottom of that page.

Comment 15

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

Cause: Bug 1112332 seems the likely cause of this issue.  This issue was not uplifted to 2.1, however I have also been unable to reproduce this issue on 2.1 as well.  Was the flag set incorrectly by chance?

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150109085501
Gaia: d102cc0a7a1f346531553bec64588eea9e4594eb
Gecko: 78f910177388
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

First Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20150109123130
Gaia: 2c7d14040149e1f9b1bb3972ff150be0472fa6b6
Gecko: da03d6aff37d
Version: 37.0a1 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: d102cc0a7a1f346531553bec64588eea9e4594eb
Gecko: da03d6aff37d

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 2c7d14040149e1f9b1bb3972ff150be0472fa6b6
Gecko: 86396560012

Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=78f910177388&tochange=da03d6aff37d


These are the variables for the build I tested 2.1 Flame KK on.
Environmental Variables:
Device: Flame 2.1
BuildID: 20150121143637
Gaia: 2055fc40a8bd2af1908979cb45da6b7d1c4ced0b
Gecko: 38ac70ca969b
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 16

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

Benoit, can you take a look at this please? This might have been caused by the work done on bug 1112332

Comment 17

[Benoit Girard (:BenWa)]

Yes, I'll look tomorrow.

Comment 18

[Benoit Girard (:BenWa)]

I'm seeing the background of the page disappearing when at max zoom. That's weird. I can reproduce the crash as well.

Comment 19

[Benoit Girard (:BenWa)]

Attaching with GDB gives:
Child terminated with signal = 0x9 (SIGKILL)

So we're OOMing which is what attachment 8549471 [details] also indicates.

Comment 20

[Benoit Girard (:BenWa)]

Ok some progress. My patch makes us take the UseFastPath where we didn't before (or as much). This path likely leaks aggressively. I'll find out why tomorrow.

Comment 21

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

Reassigning to benwa for now; feel free to ping me if you need any help with this, although I was not able to consistently reproduce the issue.

Comment 22

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: Browser Close Logcat.txt

This issue occurs on today's Master build, but is easier to hit. Logcat attatched.

Repro:
1. Connect to the internet with a valid data connection
2. Open Browser app
3. Go to a website (ex: www.Google.com)
4. Zoom all the way in on the site page, and pan around in any direction one time

Actual Result: The browser will force close by itself and bring the user to the homescreen.

Environmental Variables:
Device: Flame 3.0 (319Mb)(Kitkat)(Full Flash)
BuildID: 20150126010231
Gaia: 0f662dffef27599443cfcd790c2b39190a2b35c8
Gecko: fa91879c8428
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Note: The browser will sometimes close even before the user gets the chance to pan around

Repro rate: 10/10 100%

Comment 23

[Peter Bylenga [:PBylenga]]

Can we double check the branch checks on this issue, the severity of this issue now makes it fail smoke tests.

Comment 24

[Benoit Girard (:BenWa)]

Ive been making progress tracking this down. We're taking a snapshot of the surface and it would appear that snapshot is leaking.

Comment 25

[Yeojin Chung [:YeojinC]]

Following STRs in Comment 22, I was able to reproduce the bug on Flame 2.2 and Master.

Result: The browser is closed when the user pans around the screen after fully zooming in.

Device: Flame 3.0 (319mb, shallow flash)
Build ID: 20150126052534
Gaia: 793773bb2944b42a85dd160049e605cbd880c4da
Gecko: 95c76c3b0172
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Device: Flame 2.2 (319mb, shallow flash)
Build ID: 20150126125805
Gaia: 0518f4581a0925c0b703d730ef289ab15cbd1216
Gecko: b27709406790
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

========================================================
I was unable to reproduce this bug following STRs from both Comment 0 and Comment 22 on Flame 2.1 and 2.0.

Result: The browser works properly.
Repro rate: 0/10

Device: Flame 2.1 (319mb, shallow flash)
Build ID: 20150124133233
Gaia: 54d92cc0755e5102223276ab23063b5eee74b514
Gecko: 522d6c980917
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
Build ID: 20150126104142
Gaia: 2989f2b2bd12fcc0e9c017d2db766e76a55873b8
Gecko: 1819d0fc5687
Version: 32.0 (2.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 26

[Benoit Girard (:BenWa)]

Attachment: patch

Nical can you take this review? If not please pass it to :kats.

The issue is that my patch was making use take UseFastPath more often. UseFastPath didn't properly setup for painting which sometimes cause us to paint random sizes and eventually crash.

This patch fixes the calculation of the fast path to perform the paint setup similar to how we do outside the fast path.

Comment 27

[Nicolas Silva [:nical]]

Comment on attachment 8554879 [details] [diff] [review]
patch

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

::: gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ +432,5 @@
> +
> +      // Make sure that tiles that fall outside of the visible region or outside of the
> +      // critical displayport are discarded on the first update. Also make sure that we
> +      // only draw stuff inside the critical displayport on the first update.
> +      if (!mPaintData.mCriticalDisplayPort.IsEmpty()) {

nit: looks like there is the same thing in the non-fast-path code path as well so you could move this out vefore the UseFastPath branch.

Comment 28

[Benoit Girard (:BenWa)]

I could 'mValidRegion = neededRegion' needs to happen before which means the if statement would need to be split into two blocks.

Comment 29

[Parul Mathur [:pragmatic] - out of office - please ni? [:bajaj] instead]

This bug is a smoketest blocker as per test case https://moztrap.mozilla.org/manage/cases/?filter-id=15267

Comment 30

[Benoit Girard (:BenWa)]

https://tbpl.mozilla.org/?tree=Try&rev=798567ca569b

Comment 31

[Nicolas Silva [:nical]]

(In reply to Benoit Girard (:BenWa) from comment #28)
> I could 'mValidRegion = neededRegion' needs to happen before which means the
> if statement would need to be split into two blocks.

ah, right, my mistake.

Comment 32

[Benoit Girard (:BenWa)]

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

Comment 33

[Benoit Girard (:BenWa)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8a5428a9052

Comment 34

[Benoit Girard (:BenWa)]

Attachment: patch v2 r=nical

Add check for empty paint

Comment 35

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de96fc93d9ec

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/de96fc93d9ec

Comment 37

[William Hsu [:whsu]]

Please help uplift to v2.2 and v2.1.
Many thanks.

Comment 38

[Lance（leave from Mozilla）]

This bug has been verified to fail on latest Flame 3.0 build.

Steps:
1.Launch Browser.
2.Go to "www.sina.com.cn".
3.Zoom all the way in on the site page, and pan around in any direction.

Expected Result: 
3. User could zoom in webpage normally and it shouldn't auto exit Browser.

Actual Result: 
3. Device will auto exit browser and go back to Home screen.

Fail rate:4/5
Found time:14:36
See attachment:1436.mp4 and logcat_1436.txt

Flame 3.0 version:
Gaia-Rev        ba613ae583a706131c45e885f65d428d4a541a81
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/9b6b80222e66
Build-ID        20150128101733
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150128.134719
FW-Date         Wed Jan 28 13:47:30 EST 2015
Bootloader      L1TC000118D0

Comment 39

[Lance（leave from Mozilla）]

Attachment: logcat_1436.txt

Comment 40

[Lance（leave from Mozilla）]

Attachment: 1436.MP4

Comment 41

[Ryan VanderMeulen [:RyanVM]]

Please request b2g34 and b2g37 approval on this when you get a chance so we can get bug 1112332 uplifted as well.

Comment 42

[Benoit Girard (:BenWa)]

Lance can you reproduce the original bug on www.sina.com?

I want to avoid mixing up two issues since the page at www.sina.com.cn is heavier. We should default to assuming it's a separate bug until we can prove otherwise.

Comment 43

[Benoit Girard (:BenWa)]

Comment on attachment 8555674 [details] [diff] [review]
patch v2 r=nical

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 #): Unclear, Bug 1112332 causes the buggy branch to be taken more frequently but it didn't introduce it.
User impact if declined: OOM Crashes & wasted performance performing bogus paints.
Testing completed: manual, central for a day.
Risk to taking this patch (and alternatives if risky): Medium since changing painting logic is non trivial.
String or UUID changes made by this patch: None

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/b89c9677e227
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/5f08c87a7116

Comment 45

[Lance（leave from Mozilla）]

(In reply to Benoit Girard (:BenWa) from comment #42)
> Lance can you reproduce the original bug on www.sina.com?
> 
> I want to avoid mixing up two issues since the page at www.sina.com.cn is
> heavier. We should default to assuming it's a separate bug until we can
> prove otherwise.

Hi Benoit,

I can reprodeced this issue at "www.sina.com" on latest Flame 3.0 build.

Steps:
1. Launch Browser and go to "www.sina.com".
2. Pinch to zoom in screen several times, and pan around in any direction.

Expected Result: 
2. User could zoom in webpage normally and it shouldn't auto exit Browser.

Actual Result: 
2. Device will auto exit browser and go back to Home screen.

Fail rate:3/10
Found time:20:05
See attachment:logcat_2005.txt

Flame 3.0 version:
Gaia-Rev        9d2378a9ef092ab1fc15c3a9f7fc4171aab59d57
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/6bfc0e1c4b29
Build-ID        20150129010239
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150129.043711
FW-Date         Thu Jan 29 04:37:21 EST 2015
Bootloader      L1TC000118D0

Comment 46

[Lance（leave from Mozilla）]

Attachment: logcat_2005.txt

Comment 47

[William Hsu [:whsu]]

Hi, Lance,

May I have your help?
Can it be reproduced on latest v2.1 & v2.2 after patch landed?

Many thanks.

Comment 48

[Benoit Girard (:BenWa)]

I can still reproduce an OOM crash but it's not triggered by UseFastPath. I'm going to spin off this bug since we're likely dealing with something else.

Comment 49

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/5f08c87a7116

Comment 50

[Lance（leave from Mozilla）]

Clone bug 1128868 to track this issue, and we deleted the verifyme.

Comment 51

[Martin Shuman [:Marty]]

This issue is verified Fixed on the latest Flame 3.0, 2.2, and 2.1 builds.
Browser does not close when zoomed all the way in and panning the browser window.

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150206010204
Gaia: 94af4b42d2ace6c9f38f31de77240604fac68af1
Gecko: 7c5f187b65bf
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Environmental Variables:
Device: Flame 2.2 (319MB)(Full Flash)
Build ID: 20150206002505
Gaia: a52999ce7f783177deb17e267bf003a53e6fde06
Gecko: 01446d5231ef
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Environmental Variables:
Device: Flame 2.1 (319MB)(Full Flash)
Build ID: 20150206001229
Gaia: 5d5163069da2c660261399002e88b4cbb9135f1e
Gecko: c2210e7a21a3
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 52

[Lance（leave from Mozilla）]

The problem is verified not happen on latest 2.1s(512m) build, but it still exist on latest 2.1s(256m) build.

Steps:
1. Launch Browser and go to "www.sina.com".
2. Pinch to zoom in screen several times, and pan around in any direction.

Expected Result: 
2.1s(256m): 2. User could zoom in webpage normally and it shouldn't auto exit Browser.

Actual Result: 
2.1s(256m): 2. Device will auto exit browser and go back to Home screen.
2.1s(512m): 2. User zoom in webpage normally and it can't auto exit Browser.

Found time:14:34
See attachment: Please see bug 1128868.

Fail rate:
2.1s(256m):1/10
2.1s(512m):0/10

2.1s(256m) version:
Build ID               20150209001232
Gaia Revision          bca70e96979fbd714012dc442a92b9fa156f63f7
Gaia Date              2015-02-03 00:37:47
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/afac5ac46ff6
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150209.035424
Firmware Date          Mon Feb  9 03:54:36 EST 2015

2.1s(512m) version:
Build ID               20150209001232
Gaia Revision          bca70e96979fbd714012dc442a92b9fa156f63f7
Gaia Date              2015-02-03 00:37:47
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/afac5ac46ff6
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  93
Firmware Date          Thu Jan 22 15:21:20 CST 2015

Comment 53

[Mike Lien[:mlien]]

since current 2.1s project is for 512m, refer to comment 52, mark as verified

Comment 54

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

After discussion with BenWa on IRC, I backed out this fix, because it caused problems like bug 1132741 (and it's many dupes). I suspect the backout will also fix bug 1127877 and its many see-also bugs.

Backout: https://hg.mozilla.org/integration/b2g-inbound/rev/333cd39619fe

Comment 55

[Benoit Girard (:BenWa)]

I'll be picking this bug back up.

Comment 56

[Ryan VanderMeulen [:RyanVM]]

Merge of backout.
https://hg.mozilla.org/mozilla-central/rev/333cd39619fe

Does this need to be backed out from b2g34 and b2g37 as well then?

Comment 57

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

It will probably need to be backed out from b2g37 and b2g34 but I would like to make sure we have a proper fix in hand before we do that. It would be nice to uplift both the backout and the new fix together. I could be convinced otherwise. I'm going to try and poke other bugs to see how much of an impact this is really having.

Comment 58

[Benoit Girard (:BenWa)]

In the mean time I moved this back to the top of my queue. I'll be looking at this tomorrow.

Comment 59

[Carsten Book [:Tomcat]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #56)
> Merge of backout.
> https://hg.mozilla.org/mozilla-central/rev/333cd39619fe
> 
> Does this need to be backed out from b2g34 and b2g37 as well then?

I backed this out since this push caused on b2g-i and the other trees after a merge a perma c2 test failure

Comment 60

[Carsten Book [:Tomcat]]

https://treeherder.mozilla.org/logviewer.html#?job_id=1047267&repo=mozilla-central was the failure as exmaple

Comment 61

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

I think the reason this test failed is just because it took much longer. It does 200 iterations of a bunch of changes, and when I ran it locally it got through 77 iterations before it timed out. Based on what I saw in gdb I suspect it is doing readbacks during gralloc operations which is what is slowing it down.

Comment 62

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

Upon further investigation I don't think this has to do with gralloc, at least not specifically. It's just that this patch makes the test paint a 1024x1024 area (sometimes 1024x1536) instead of a 2944x2944 (sometimes 2944x4172) area and so it takes much longer.

Comment 63

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

Interestingly, when I roll back to the cset just before de96fc93d9ec, the painted area is always 800x1000. So something that landed *after* this bug caused the visible region to get expanded for what appears to be no good reason.

I'll try bisecting but it'll be slow.

Comment 64

[Benoit Girard (:BenWa)]

Looking into a proper fix for sina.com I'm seeing layout size the layer to a huge size which causes OOM:

#3  0xb5802040 in mozilla::SetOuterVisibleRegion (aLayer=0xb2f51800, aOuterVisibleRegion=0xbe9bfffc, aLayerContentsVisibleRect=0x0) at /home/benoit/mozilla/b2g-flame/gecko/layout/base/FrameLayerBuilder.cpp:1876
1876	  aLayer->SetVisibleRegion(*aOuterVisibleRegion);
(gdb) print *aOuterVisibleRegion
$1 = {mImpl = {mImpl = {extents = {x1 = 0, y1 = 0, x2 = 2765, y2 = 4661}, data = 0x0}}}

Comment 65

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

The bisection pointed to bug 950934; specifically the bit that makes WantSubAPZC return true in the B2G parent process. Turning on APZ dramatically increases the visible region of the layer that is painted in the test. With BenWa's patch this region was then clamped to the critical displayport so it was still pretty fast to run through. With BenWa's patch backed out we ended up painting the entire low-res displayport area in high-res (because we are taking the fast-path) and this causes it to be very slow and time out.

Also this ties in to what BenWa and I just discussed on IRC. The root cause of this bug is that the fast-path doesn't distinguish between high-res and low-res areas, and just draws the entire visible region in high-res. However the visible region is computed using the low-res displayport in the layout code, and so is much larger than the area we actually want to paint in high-res.

That also explains why this is a regression from 1112332 (per comment 15) because that made us take the fast-path even in cases where we have a low-precision rendering, and where we know the visible region is going to be massive.

Comment 66

[Benoit Girard (:BenWa)]

I've got a patch that addresses this issue as outlined by :kats but I'm trying to also adjust the fix for bug 1112332 and testing that it works properly. I'll probably complete this tomorrow.

Comment 67

[Benoit Girard (:BenWa)]

I filed bug 1134762 which includes a backout of this patch and a proper fix. This bug will likely get duped to bug 1134762 once it's resolved.

Comment 68

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

I think since this bug has a net zero change since comment 54 we should close it back up and pretend nothing happened. The patch in bug 1134762 should be treated as any other follow-up bug.