chaopeng: can you PTAL. Thanks.

I dont see browse:social:facebook_infinite_scroll fails in https://chromium-review.googlesource.com/627117. Just create a revert patch for test.

Here are some details about these benchmarks (system health), and how to run them:
https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

TL;DR:

To run locally probably try:

- build e.g. ChromePublic.apk
- $ tools/perf/run_benchmark system_health.memory_mobile --browser android-chromium --story-filter facebook_infinite_scroll --pageset-repeat 5

To run on a tryjob:

$ tools/perf/run_benchmark android-nexus5X system_health.memory_mobile --story-filter facebook_infinite_scroll --pageset-repeat 5

I've added the --pageset-repeat 5 as I don't know how persistent or flaky the error is. You may drop that to 1 or increase e.g. to 10 respectively on how easy it is to reproduce the failure.

Also, if a culprit/fix is not found soon, it would probably be a good idea to disable all of:

- browse:social:tumblr_infinite_scroll
- browse:social:facebook_infinite_scroll
- browse:media:flickr_infinite_scroll

which are failing on all platforms (at least Android and Windows for sure).

Just ran locally on a Nexus 7 running M and couldn't repro the failure. Used:

tools/perf/run_tests -v --browser=exact --device=079fb992 --browser-executable out-gn/Release/apks/ChromePublic.apk --chromium-output-directory out-gn/Release --total-shards=15 --shard-index=12

This is still a problem, can we disable the test?

One suspicious logcat message I see on the failing bot that I don't see locally in my passing run:

08-29 20:25:10.203 28293 28293 I chromium: [INFO:CONSOLE(0)] "Uncaught (in promise) TypeError: Failed to register a ServiceWorker: A bad HTTP response code (404) was received when fetching the script.", source: https://m.facebook.com/shakira (0)

Doesn't seem like this should break the test though...

+nednguyen FYI

I tried to run this test locally on Nexus 6 Android 7.0 with/without my patch(https://chromium-review.googlesource.com/627117). Both of them got timeout because a notification popup. If I click block, then the test can finish. 

I suspect the timeout because we can not close the notification popup automatically.

Also tried turn on airplane mode.

Deleted: IMG_20170830_125049.jpg 2.8 MB

	IMG_20170830_125049.jpg 2.8 MB View Download

Yep, this is happening on the bots. From a failed swarming job:

  	UI dump
  	- (no package):
  	  - (no id)
  	- org.chromium.chrome:
  	  - (no id)
  	  - android:id/button1['Allow']
  	  - android:id/button2['Block']
  	  - android:id/content
  	  - org.chromium.chrome:id/action_bar_root
  	  - org.chromium.chrome:id/buttonPanel
  	  - org.chromium.chrome:id/custom
  	  - org.chromium.chrome:id/customPanel
  	  - org.chromium.chrome:id/parentPanel
  	  - org.chromium.chrome:id/text['m.facebook.com wants to send you notifications.']

https://chromium-swarm.appspot.com/task?id=38486da35af1bc10&refresh=10&show_raw=1&wide_logs=true

Not sure why we suddenly started getting these notifications. We're loading facebook from a recording from a time when (I assume) notifications were not even a thing.

The "ServiceWorker" error noted above also seems relevant now.

I also tried on android_n5x_swarming_rel with a dummy patch and it can run until finish. https://chromium-review.googlesource.com/c/chromium/src/+/643587

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

commit ebb3386b79cc9378008a63f2218fc40f4c65377a
Author: chaopeng <chaopeng@chromium.org>
Date: Wed Aug 30 20:47:55 2017

Disable facebook_infinite_scroll test

This test got timeout because an unexpexted notification popup.
In this patch we disable this test for waterful. perezju will
investigate further.

Bug:  760319 
Change-Id: I878ca0474699bfedf745154abb3cb3991455373e
Reviewed-on: https://chromium-review.googlesource.com/643333
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498602}
[modify] https://crrev.com/ebb3386b79cc9378008a63f2218fc40f4c65377a/tools/perf/page_sets/system_health/expectations.py

Removing sheriff label since the test is now disabled. Good luck!

Removing sheriff label since the test is now disabled. Good luck!

By locally testing different builds from:
https://luci-milo.appspot.com/buildbot/chromium.perf/Android%20Nexus5X%20Perf

I've narrowed down the introduction of the problem to the CL range:
http://test-results.appspot.com/revision_range?start=497956&end=498063

Apparently the popup has always been shown. What changed is that before it was a small notification at the bottom of the screen and, although the rest of the page was dimmed, one could go ahead and scroll the page anyway. Now the notification is a modal dialog, which won't allow to interact with the page until tapping or otherwise dismissing it. (See screenshots.)

Although it might be nice to figure out what specific CL caused the change, the fix will probably be to either:

1. update the story and make it dismiss the dialog itself.
2. pass some flag to Chrome (if one exists?) to disable such notifications which may interfere with our tests?

I'm leaning a bit towards 1 as that seems more "realistic". Any thoughts or opinions?

Deleted: screen_before.png 258 KB

	screen_before.png 258 KB View Download

Deleted: screen_after.png 166 KB

	screen_after.png 166 KB View Download

+1 to adding action to dismiss the dialog

Actually I worry that the dialog may make the loading metrics noisy. Thought?

A few more facts I've uncovered. The problem is indeed only happening on browse:social:facebook_infinite_scroll.

This used to cause a WebSocketTimeoutException aborted the current story and somehow, caused the trace collection bit of code to be skipped. This _then_ caused the next story (either pinterest or flickr on different devices) to fail with "AssertionError: Data not collected from last trace.". Filed  issue 760922  for that.

Disabling browse:social:facebook_infinite_scroll does prevent the error and allows other stories to run normally.

The disabling in #15 missed system_health.common_mobile, where the story was still failing for the same reason. I'll disable that too until a fix is in place.

Also, on top of that, all stories and most benchmarks are now failing on Android now due to  issue 760919 . This appears to be an unrelated issue.

And also left to figure out why the story also seems to be often failing on Windows.

Re #19 - how big a pain would it be to try both options?

Ideally we'd go with updating the story to dismiss the dialog, but if we can demonstrate that this approach introduces a bunch of noise relative to getting rid of the dialog altogether, we might want to just get rid of the dialog.

Hmm.. a similar thing is happening on Windows too. From:
https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FWin_10_High-DPI_Perf%2F956%2F%2B%2Frecipes%2Fsteps%2Fsystem_health.memory_desktop_on_Intel_GPU_on_Windows_on_Windows-10-10240%2F0%2Fstdout

browse:social:facebook_infinite_scroll times out on:

    tab.WaitForJavaScriptCondition('window.__scrollActionDone', timeout=60)

the story fails and then browse:media:flickr_infinite_scroll dies with:

    AssertionError: Data not collected from last trace.

Sadly no screen shots are available.

Do we know if on windows the popup might also be turned into a modal dialog?

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8969746574900033712

Re #25: If there is a quick flag or setting I could switch to turn off these popups I'll do the experiment to compare both approaches. Does anybody know if there is one?

=== BISECT JOB RESULTS ===
NO Test failure found

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:all_processes:dump_count_avg/browse_social/browse_social_facebook_infinite_scroll

Revision             Exit Code      N
chromium@497956      0 +- N/A       10      good
chromium@498063      0 +- N/A       10      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=browse.social.facebook.infinite.scroll system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8969746574900033712


For feedback, file a bug with component Speed>Bisection

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

commit 41df646478cc77acee7e2ee53730c51f992c69c9
Author: Juan A. Navarro Perez <perezju@chromium.org>
Date: Thu Aug 31 14:05:28 2017

[tools/perf] Disable browse:social:facebook_infinite_scroll on Android and Windows

Disable the story on all of:

- system_health.common_mobile
- v8.browsing_mobile
- system_health.common_desktop (WIN)
- system_health.memory_desktop (WIN)
- v8.browsing_desltop (WIN)

where it's also failing.

Bug:  760319 
Change-Id: I665a857ab571cba71c978112132b8facbe001c8b
Reviewed-on: https://chromium-review.googlesource.com/645690
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498853}
[modify] https://crrev.com/41df646478cc77acee7e2ee53730c51f992c69c9/tools/perf/page_sets/system_health/expectations.py

+peter: See #19. Is the modal popup an intended behavior? And is would there be an easy way to disable such dialogs?

Yeah, this is new, see  Issue 606138 . You can disable it through the following command line flag:

--disable-features=ModalPermissionPrompts

Ned, is there an easy way to add that to the list of disabled features Telemetry already supplies? Maybe we can land that as an quick fix; and do the experiment to compare 1 vs 2 afterwards.

Juan: you can add to the list of disabled feature in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py#L108

Your plan of disabling first, experiment later SGTM.

I'm not exactly sure what it was that broke but we should keep in mind that the feature flag is temporary and find a long term fix if that's necessary. 

Ok, I found that this is an experiment being enabled as part of a field trial [1].

Some context to recap: There is ongoing work ( issue 632269 ) to experiment with modal dialogs for permission prompts on Android.

Perf bots on the waterfall picked up the experiment; however several of the test themselves, which used to just ignore the presence of a permission prompt at the bottom of the page, now fail when trying to interact with the page blocked by the modal dialog.

I suggest as a short term fix to disable the experiment for now; as this is preventing important tests that prevent memory leaks to run (e.g. issue 760901). Sending CL out for review for this: https://chromium-review.googlesource.com/c/chromium/src/+/650347

For the longer term, before enabling the experiment again, we need to decide:

1. Whether to add some sort of permanent flag so that perf tests can disable such permission prompts. E.g. maybe some flag that just blocks and denies all such permission requests showing no UI at all.

2. Whether we want tests to actually deal with these permission prompts and dismiss them. This is tricky solve generally in a way which is brittle and prone to race conditions, since we don't really know which stories will trigger a permission prompt and when. We would have to be solving issues on a case by case basis as things break.

Thoughts?

[1]: https://cs.chromium.org/chromium/src/testing/variations/fieldtrial_testing_config.json?rcl=04935f101e6b5cf9d594447e48cbd5f26b9785f3&l=2151

Btw. Not sure why the test also started failing on Windows at a similar time. The issues appear not to be related, since this flag is specific to Android.

c#37: my initial reaction is that these perf tests would be much more realistic if they actually did something with the permission prompt rather than just ignore it. From the sounds of it we don't have any perf coverage for when a site starts using a feature gated by a permission since the tests don't interact with the prompt at all. That's potentially a whole class of memory leaks that never get covered, right?

We are intending to launch these prompts in M63, and we want as much coverage in the prerelease channels as possible. Disabling the experiment solely because perf tests are failing doesn't seem like a good solution because it takes away that testing coverage. I would strongly prefer that you don't disable the experiment if possible; blocking the launch entirely seems like overkill. 

Ideally we wouldn't be carrying a permanent flag just for the purposes of disabling this UI since it's a core part of the web platform.

I agree there would be great value on adding stories _specifically_ crafted to test interactions with features gated by such permissions.

However in some perf scenarios the prompts just get in the way. For example, in one story we just want to measure memory after loading google.com. But that triggers a notification request for location permission. That's not something we would like to interfere with the test.

Maybe what we need is some compromise that works well to test both kinds of scenarios separately?

Also, regardless of what the longer term solution is, we also need a short term fix to deploy hopefully today or tomorrow.

It's also worths noting that these permission prompt are one time only for the real users, so always enabling them in perf test doesn't add much value for making them representative of the real world, IMO.

--disable-features=ModalPermissionPrompts in benchmarks seems reasonable to me.

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

commit 4e56f178767143148dca2895dfd8945a57b24a66
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Tue Sep 05 20:29:24 2017

Roll src/third_party/catapult/ d4179a057..4ab7a181d (7 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/d4179a057d4b..4ab7a181db1c

$ git log d4179a057..4ab7a181d --date=short --no-merges --format='%ad %ae %s'
2017-09-05 perezju Revert of Add tab.IsServiceWorkerReadyOrNotRegist() which tells whether serviceworker registration is finished (patchset #8 id:140001 of https://chromiumcodereview.appspot.com/3012573002/ )
2017-09-05 perezju [Telemetry] Temporarily disable modal permission prompts on Android
2017-09-04 dtu [pinpoint] Abbreviated Job dict for the Jobs listing page.
2017-09-04 dtu [pinpoint] Fix GTest parameters.
2017-09-04 NghiaNguyenBH Rename directory from mip to mips
2017-09-04 djordje.golubovic.imgtec Add MIPS d8 binary (version 5.7.492.65)
2017-09-04 yukiy Add tab.IsServiceWorkerReadyOrNotRegistered() which tells whether serviceworker registration is finished

Created with:
  roll-dep src/third_party/catapult
BUG= 736697 , 760319 , 736697 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: Ia2ba493dffdaf0f52106ba3c481e40a018c2b247
Reviewed-on: https://chromium-review.googlesource.com/650686
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499734}
[modify] https://crrev.com/4e56f178767143148dca2895dfd8945a57b24a66/DEPS

This problem actually keeps arising in several different situations (mostly testing related). I think that we should consider having some flags to manipulate permissions for testing purposes. In this case it would just be a matter of automatically denying all permission requests.

Re #43: Yes, I agree that would be the best path forward. Who do you think could look into that work?

For the moment we've landed a hack to just suppress the experiment flags within Telemetry tests. One of the internal bots already recovered, I'll re-enable the facebook story in system health too.

Raymes filed  bug 762335  on me to do this :)

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

commit fb6307b34a561b32652b930ea2c4536bf56e9925
Author: Juan A. Navarro Perez <perezju@chromium.org>
Date: Wed Sep 06 11:32:32 2017

[tools/perf] Reenable facebook_infinite_scroll on Android

Workaround is in place for story to run now.

TBR=nednguyen@google.com

Bug:  760319 
Change-Id: I65a1fdf1d96759ca12dfb8426bdbd35ac59be72b
Reviewed-on: https://chromium-review.googlesource.com/652306
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499930}
[modify] https://crrev.com/fb6307b34a561b32652b930ea2c4536bf56e9925/tools/perf/page_sets/system_health/expectations.py

I'm going to close this as fixed for now, and tag along  issue 762335  for when a more permanent solution becomes available.

 Issue 760682  has been merged into this issue.