Shine(Leave from Mozilla) Reporter
	Description • 9 years ago

[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

	William Hsu [:whsu]
	Comment 3 • 9 years ago

Hi, Shine,

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

	Shine(Leave from Mozilla) Reporter
	Comment 4 • 9 years ago

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

	William Hsu [:whsu]
	Comment 5 • 9 years ago

(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.

	William Hsu [:whsu]
	Comment 6 • 9 years ago

Hi, Shine,

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

	William Hsu [:whsu]
	Comment 7 • 9 years ago

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.

	Shine(Leave from Mozilla) Reporter
	Comment 8 • 9 years ago

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

	William Hsu [:whsu]
	Comment 9 • 9 years ago

[Blocking Requested - why for this release]:

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

Request a Repair. Nominate it.

	bhavana bajaj [:bajaj]
	Comment 10 • 9 years ago

(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

	William Hsu [:whsu]
	Comment 11 • 9 years ago

(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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 12 • 9 years ago

I can repro on master, will investigate.

	William Hsu [:whsu]
	Comment 13 • 9 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #12)
> I can repro on master, will investigate.

Thanks Kartikaya! :)

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 14 • 9 years ago

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.

	Jayme Mercado [:JMercado] (Inactive 3/4/2016)
	Comment 15 • 9 years ago

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

	KTucker [:KTucker][Inactive 3/4/2016]
	Comment 16 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 17 • 9 years ago

Yes, I'll look tomorrow.

	Benoit Girard (:BenWa) Assignee
	Comment 18 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 19 • 9 years ago

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

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

	Benoit Girard (:BenWa) Assignee
	Comment 20 • 9 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 21 • 9 years ago

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.

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15
	Comment 22 • 9 years ago

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%

	Peter Bylenga [:PBylenga]
	Comment 23 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 24 • 9 years ago

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

	Yeojin Chung [:YeojinC]
	Comment 25 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 26 • 9 years ago

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.

	Nicolas Silva [:nical]
	Comment 27 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 28 • 9 years ago

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

	Parul Mathur [:pragmatic] - out of office - please ni? [:bajaj] instead
	Comment 29 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 30 • 9 years ago

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

	Nicolas Silva [:nical]
	Comment 31 • 9 years ago

(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.

	Benoit Girard (:BenWa) Assignee
	Comment 32 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 33 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 34 • 9 years ago

Add check for empty paint

	Benoit Girard (:BenWa) Assignee
	Comment 35 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 36 • 9 years ago

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

	William Hsu [:whsu]
	Comment 37 • 9 years ago

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

	Lance（leave from Mozilla）
	Comment 38 • 9 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 41 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 42 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 43 • 9 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 44 • 9 years ago

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

	Lance（leave from Mozilla）
	Comment 45 • 9 years ago

(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

	William Hsu [:whsu]
	Comment 47 • 9 years ago

Hi, Lance,

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

Many thanks.

	Benoit Girard (:BenWa) Assignee
	Comment 48 • 9 years ago

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.

	Vincent Liu[:vliu]
	Comment 49 • 9 years ago

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

	Lance（leave from Mozilla）
	Comment 50 • 9 years ago

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

	Martin Shuman [:Marty]
	Comment 51 • 9 years ago

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

	Lance（leave from Mozilla）
	Comment 52 • 9 years ago

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

	Mike Lien[:mlien]
	Comment 53 • 9 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 54 • 9 years ago

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

	Benoit Girard (:BenWa) Assignee
	Comment 55 • 9 years ago

I'll be picking this bug back up.

	Ryan VanderMeulen [:RyanVM]
	Comment 56 • 9 years ago

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?

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 57 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 58 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 59 • 9 years ago

(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

	Carsten Book [:Tomcat]
	Comment 60 • 9 years ago

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

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 61 • 9 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 62 • 9 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 63 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 64 • 9 years ago

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}}}

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 65 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 66 • 9 years ago

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.

	Benoit Girard (:BenWa) Assignee
	Comment 67 • 9 years ago

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.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 68 • 9 years ago

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.