Hmm, is this a real regression, or a bug in tracking? That's really bad if it's real o_O

which android version were you using to test this? mostly are you using N or pre-N?

Hmm, looks like an accounting bug. "Total resident" looks to be from system rather than accounting, and that's not going up? +primiano, does that make sense?

it could be a tracking bug (in which case it should be fixed) or it could be a real regression and those cc resources are tied to gl memory. 
if the device supports the memtrack api there is a way to get that data in tracing : you have to build the memtrack_helper target, push on the device and run it as root. tracing will do the rest. 
alternatively try to see with dumpsys meminfo if the gl memory grows 

curious, how is "Total resident" computed then, if it's not from something like dumpsys?

Tested on 53.0.2785.134. That looks like a real leak to me.

So we must be dropping resources compositor resources each time we go through detach/attach.

There is a *little* time left for m54.. maybe?

There are two issues:
1, The cc::Display of HardwareRender(SurfaceInstance) is destroyed after the reset of surface_factory_, which causes BVR can't get the resources returned by the parent compositor, as the surface_factory_ is null:
https://cs.chromium.org/chromium/src/cc/surfaces/surface_aggregator.cc?q=surface_aggregator&sq=package:chromium&l=123
I made a hack to destroy Display before the reset of surface_factory_ in the HardwareRenderer dtor and resolved the leak issue, but it needs formal fix.

2, with #1 fixed, the resources aren't released immediately after detach, because AwGLFunctor.onDetachedFromWindow is called after the nativeOnDetachedFromWindow of AwContents/BVR, which grows the memory footprint when WebView is in detached state. I created a CL to fix this: https://codereview.chromium.org/2417213002/

> curious, how is "Total resident" computed then, if it's not from something like dumpsys?
We can't use dumpsys from chrome/webview for permission reasons. total resident is counted looking at /proc/*/status (or statm).
But with that and the memtrack helper we get essentially the same data.
If your gpu driver doesn't map GL memory in process (it's often the case) that is counted separately.

re #9
+jbauman for 1, ie surface_factory has to be destroyed after display? is there a way to force Display to drop everything first? submit an empty frame?

2. I think that should be ok. But that's a tricky area, so need to cover all corner cases..

re #10: That makes total sense now. Thanks!

looks like submitting an empty frame works, I'll write up a CL

And thanks shouqun for hunting this one down! Would have taken me a lot of time hunt this one down.

+picksi@ and +perezju@ to see if there's anything we can do as part of our perf analysis to detect this stuff sooner (along with primiano@ who's already CC'ed).

In my mind at least, memory tests should be triggering the same code already, because putting the app into the background and then running trim should execute the same code path as detaching the webview. I could be wrong though..

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df

commit 2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df
Author: boliu <boliu@chromium.org>
Date: Fri Oct 14 21:19:34 2016

aw: Fix effective frame source leak after detach

shouqun@ discovered that destroying the SurfaceFactory before all
resources belonging to that surface are returned will lead to those
resources being dropped completely, since cc::Display only holds onto a
weakptr to SurfaceFactory to return resources.

Webview goes through this cycle each time it's detached and re-attached,
so the "lost" resources will keep growing until the memory budget is
reached. Note though that destroying the webview does free these
resources, so they are not lost forever. But detach/attach is very
common, and this is still a severe leak.

Fix by submitting an empty frame into the Surface, which forces
resources from the previous frame to be returned.

BUG= 655434 

Review-Url: https://codereview.chromium.org/2423473002
Cr-Commit-Position: refs/heads/master@{#425466}

[modify] https://crrev.com/2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df/android_webview/browser/hardware_renderer.cc

Your change meets the bar and is auto-approved for M55 (branch: 2883)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75fa605f8ee74897558c6ccba1f6ed525c9d3fe1

commit 75fa605f8ee74897558c6ccba1f6ed525c9d3fe1
Author: Bo Liu <boliu@chromium.org>
Date: Fri Oct 14 22:57:56 2016

[Merge M55] aw: Fix effective frame source leak after detach

shouqun@ discovered that destroying the SurfaceFactory before all
resources belonging to that surface are returned will lead to those
resources being dropped completely, since cc::Display only holds onto a
weakptr to SurfaceFactory to return resources.

Webview goes through this cycle each time it's detached and re-attached,
so the "lost" resources will keep growing until the memory budget is
reached. Note though that destroying the webview does free these
resources, so they are not lost forever. But detach/attach is very
common, and this is still a severe leak.

Fix by submitting an empty frame into the Surface, which forces
resources from the previous frame to be returned.

BUG= 655434 

Review-Url: https://codereview.chromium.org/2423473002
Cr-Commit-Position: refs/heads/master@{#425466}
(cherry picked from commit 2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df)

Review URL: https://codereview.chromium.org/2417343003 .

Cr-Commit-Position: refs/branch-heads/2883@{#125}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/75fa605f8ee74897558c6ccba1f6ed525c9d3fe1/android_webview/browser/hardware_renderer.cc

oops..

[Automated comment] Less than a week to go before stable on M54, we might already have a stable candidate build. Manual review required.

Godspeed.  Approved for M54 branch 2840.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56a61bc20d6a87ac2526ab8890b25a8d34973579

commit 56a61bc20d6a87ac2526ab8890b25a8d34973579
Author: Bo Liu <boliu@chromium.org>
Date: Fri Oct 14 23:49:19 2016

[Merge M54] aw: Fix effective frame source leak after detach

shouqun@ discovered that destroying the SurfaceFactory before all
resources belonging to that surface are returned will lead to those
resources being dropped completely, since cc::Display only holds onto a
weakptr to SurfaceFactory to return resources.

Webview goes through this cycle each time it's detached and re-attached,
so the "lost" resources will keep growing until the memory budget is
reached. Note though that destroying the webview does free these
resources, so they are not lost forever. But detach/attach is very
common, and this is still a severe leak.

Fix by submitting an empty frame into the Surface, which forces
resources from the previous frame to be returned.

BUG= 655434 

Review-Url: https://codereview.chromium.org/2423473002
Cr-Commit-Position: refs/heads/master@{#425466}
(cherry picked from commit 2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df)

Review URL: https://codereview.chromium.org/2417393002 .

Cr-Commit-Position: refs/branch-heads/2840@{#743}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/56a61bc20d6a87ac2526ab8890b25a8d34973579/android_webview/browser/hardware_renderer.cc

forked  crbug.com/656213  to investigate why we didn't catch this

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75fa605f8ee74897558c6ccba1f6ed525c9d3fe1

commit 75fa605f8ee74897558c6ccba1f6ed525c9d3fe1
Author: Bo Liu <boliu@chromium.org>
Date: Fri Oct 14 22:57:56 2016

[Merge M55] aw: Fix effective frame source leak after detach

shouqun@ discovered that destroying the SurfaceFactory before all
resources belonging to that surface are returned will lead to those
resources being dropped completely, since cc::Display only holds onto a
weakptr to SurfaceFactory to return resources.

Webview goes through this cycle each time it's detached and re-attached,
so the "lost" resources will keep growing until the memory budget is
reached. Note though that destroying the webview does free these
resources, so they are not lost forever. But detach/attach is very
common, and this is still a severe leak.

Fix by submitting an empty frame into the Surface, which forces
resources from the previous frame to be returned.

BUG= 655434 

Review-Url: https://codereview.chromium.org/2423473002
Cr-Commit-Position: refs/heads/master@{#425466}
(cherry picked from commit 2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df)

Review URL: https://codereview.chromium.org/2417343003 .

Cr-Commit-Position: refs/branch-heads/2883@{#125}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/75fa605f8ee74897558c6ccba1f6ed525c9d3fe1/android_webview/browser/hardware_renderer.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56a61bc20d6a87ac2526ab8890b25a8d34973579

commit 56a61bc20d6a87ac2526ab8890b25a8d34973579
Author: Bo Liu <boliu@chromium.org>
Date: Fri Oct 14 23:49:19 2016

[Merge M54] aw: Fix effective frame source leak after detach

shouqun@ discovered that destroying the SurfaceFactory before all
resources belonging to that surface are returned will lead to those
resources being dropped completely, since cc::Display only holds onto a
weakptr to SurfaceFactory to return resources.

Webview goes through this cycle each time it's detached and re-attached,
so the "lost" resources will keep growing until the memory budget is
reached. Note though that destroying the webview does free these
resources, so they are not lost forever. But detach/attach is very
common, and this is still a severe leak.

Fix by submitting an empty frame into the Surface, which forces
resources from the previous frame to be returned.

BUG= 655434 

Review-Url: https://codereview.chromium.org/2423473002
Cr-Commit-Position: refs/heads/master@{#425466}
(cherry picked from commit 2fd5e46f7fba4945e86a5c4cd9297a6eaf2417df)

Review URL: https://codereview.chromium.org/2417393002 .

Cr-Commit-Position: refs/branch-heads/2840@{#743}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/56a61bc20d6a87ac2526ab8890b25a8d34973579/android_webview/browser/hardware_renderer.cc