Can you also attach the source for your test app?

Does it grow in the same way if you keep the same WebView instance and just navigate it repeatedly?

Also, is that actually the same process (with the same pid) throughout the entire test? The maps for the first and last sample look very different (same read-only things mapped at different locations).

Do any memory pressure signals get sent during this test? If there's lots of ram available and no memory trim signals are ever received, I think it's somewhat expected that various caches may get larger. Also, what device is this on? Have you tried this on different devices and seen a consistent pattern?

I asked perezju@ to look into the page cycler tests we run on our bots to see if we have a similar pattern of memory usage growing during the run; we don't currently run any tests for so long/so many cycles, though.

Attached source code of test app

#2:
Haven't tried with same webview instance yet. I'll see if this makes this issue better.

#3:
It should be the same process, otherwise i won't able to get the smaps (they were dump by a script, during the test)

#4:
This test was run on a nexus 6, I'm not sure if any memory pressure signals was sent during the test. I'll try again an see this happens.

Actually we found this pattern while testing our webview-based browser app(on  different devices). The malloc part keeps bloating even if we manually trigger the pressure signal.

Deleted: app_src.zip 34.4 KB

app_src.zip 34.4 KB Download

yeah, there isn't enough evidence from the data that there is a leak, not everything can be cleaned up, and looking at the graph, it's not growing anymore in the second half

maybe use this bug to track adding memory testing across many webviews, not just one? perezju?

re #6: On our webview shell [1], would that just mean always navigating to pages through, e.g., the TelemetryActivity (as opposed to devtools)? Or do we need a new activity? Does it matter whether we close/restart the shell between pages?

[1]: https://cs.chromium.org/chromium/src/android_webview/tools/system_webview_shell/apk/src/org/chromium/webview_shell/

I'd have to read the source for this test app first to check (haven't had time yet).

Not sure actually..

I don't know telemetry all that well, but I would guess it's navigating within the same tab, so in webview's case, in the same webview? Does it have concepts like "create a new tab", that kind of thing? If yes, maybe we can hook those for the webview shell as well, where a "tab" maps to a webview, and closing a tab maps to destroying a webview.

If not, maybe we can add a custom intent that 1) destroys the old webview, 2) create new webview and navigate to url from intent?

The existing system health memory test is just navigating a single webview.

+timvolodine to confirm what would be needed to test this scenario

I assume tim is going to give you the exact same answer. The shell isn't all that complicated :p

+nednguyen: Does telemetry have a concept of tabs in chrome? eg operations like open/close a tab, navigate a specific tab, that kind of thing?

Yes, Telemetry does have the concepts of opening/closing tabs, implemented for Chrome by sending commands via devtools: https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/devtools_client_backend.py?l=223

I actually though we already had something like this in tests.. Looking at the attached code it seems the test is performed by firing intents multiple times on a special activity which destroys/recreates the webview on each intent. We could also create a special activity and do something similar for tests. Or maybe just destroy/create webview multiple times within one activity depending on how memory telemetry is performed.

The concept of tabs is nice and we probably should consider implementing it, but for this specific issue does not seem to be really required.

I think adding a new activity that destroys/recreates the webview + navigates to a url given in the intent would be the way to go.

Tim, could you look at this and then assign back to me to create a new benchmark?

ok will have a look

Changing title

thanks Bo, title sounds better now..

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

commit 2ae952d08053dbe229f0a11df5fffc126db1c012
Author: timvolodine <timvolodine@chromium.org>
Date: Fri Jul 08 15:37:08 2016

[WebViewShell] Add activity for opening urls while creating/destroying webviews.

Add WebViewCreateDestroyActivity which makes sure to destroy any existing
instance of webview before opening a url in a new webview on each intent.
This is required for mutiple webview telemetry and memory benchmarking,
see  crbug.com/619812 .

BUG= 619812 

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

[modify] https://crrev.com/2ae952d08053dbe229f0a11df5fffc126db1c012/android_webview/tools/system_webview_shell/BUILD.gn
[modify] https://crrev.com/2ae952d08053dbe229f0a11df5fffc126db1c012/android_webview/tools/system_webview_shell/apk/AndroidManifest.xml
[add] https://crrev.com/2ae952d08053dbe229f0a11df5fffc126db1c012/android_webview/tools/system_webview_shell/apk/res/layout/activity_empty.xml
[modify] https://crrev.com/2ae952d08053dbe229f0a11df5fffc126db1c012/android_webview/tools/system_webview_shell/apk/res/values/strings.xml
[add] https://crrev.com/2ae952d08053dbe229f0a11df5fffc126db1c012/android_webview/tools/system_webview_shell/apk/src/org/chromium/webview_shell/WebViewCreateDestroyActivity.java

Assigning to self to write a benchmark using the new activity provided. Although probably will get back to it by the end of the quarter.

+Petr FYI, not sure how/whether this would fit as part of system health stories.

I think this probably fits into "App switching case" row in https://docs.google.com/spreadsheets/d/1t15Ya5ssYBeXAZhHm3RJqfwBRpgWsxoib8_kwQEHMwI/edit#gid=0 ?

Good question. I suppose it does make sense to include WebView stories in SH if we already plan to include AMP etc. However, I think that such stories will need to be in a separate story set and, probably, a separate benchmark because they require more "special" conditions.

From tobiasjs (context  issue 668692 ):

Running this about 20 times would generate a 1MB leak:
adb shell am start org.chromium.webview_shell/org.chromium.webview_shell.WebViewCreateDestroyActivity
n_threads=$(adb shell dumpsys meminfo org.chromium.webview_shell | grep TOTAL | head -1 | awk '{print $2}')
pss=$(adb shell cat /proc/$(adb shell ps | grep webview_shell | awk '{print $2}')/status | grep Thread | awk '{print $2}')
echo $n_threads $pss

Archiving old issues which are likely no longer relevant and I probably won't work on them.

Reopen if you think an issue should be looked at again.