All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=681663

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgwNeN8goM


Bot(s) for this bug's original alert(s):

android-one

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

=== Auto-CCing suspected CL author benwells@chromium.org ===

Hi benwells@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : benwells
  Commit : b3c6a28850a816162c59526005665c683ca42308
  Date   : Fri Jan 13 14:13:45 2017
  Subject: Show the search geolocation disclosure from geolocation API use.

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : smoothness.top_25_smooth
  Metric       : mean_input_event_latency/https___www.google.com__hl_en_q_barack+obama
  Change       : 20.32% | 7.60983333333 -> 9.15583333333

Revision             Result                   N
chromium@443534      7.60983 +- 0.483525      6      good
chromium@443544      7.51967 +- 0.471446      6      good
chromium@443549      7.682 +- 0.771343        6      good
chromium@443551      7.62167 +- 0.419535      6      good
chromium@443552      7.84267 +- 0.483029      6      good
chromium@443553      9.15583 +- 0.903205      6      bad       <--

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=https...www.google.com..hl.en.q.barack.obama smoothness.top_25_smooth

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8990282530292684192

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5255279773483008


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!

OK, I think I have an idea about this.

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

commit c7b66b0e4837e533f8ec3edf9125144e5e3430bd
Author: benwells <benwells@chromium.org>
Date: Wed Feb 01 22:51:22 2017

Improve performance of search geolocation disclosure check.

This check is run for all navigations, but the disclosure is only shown
in a small number of cases. This change reorders the checks to do the
more expensive Android permission check later in the process.

BUG= 681663 

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

[modify] https://crrev.com/c7b66b0e4837e533f8ec3edf9125144e5e3430bd/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc

Turns out my fix didn't fix the regression, and upon looking at the code change I believe the regression is because running this benchmark now causes an infobar to be shown, whereas it didn't before.

I'd like to verify that the infobar is the cause of the perf regression, but I'm having trouble running the benchmark locally. The results seem very noisy. I'll try again next week but am not really confident about it.

This alert is only on some android devices. Ned, is there currently a way to record a video (if we send a job to the trybots)?

It seems like we wouldn't want the infobar showing during the performance test, unless it's indicative of normal user experience (i.e. 95% of page loads show this infobar). Is there any way to turn it off?

You can add "--profiler=android-screen-recorder" command line when run the job in trybot to get the video (look for the log that say uploading it to cloud storage in the bot log).

Though I am not sure whether looking at the video is the most useful thing for you. You may want to annotate the trace when the inforbar pops up & verify whether that aligns with the jank that occurs.

I think turning the infobar off in this test is the right thing to do, users only see it up to 3 times ever, and only once per day.

Do the perf tests run with special flags or something that I could check when deciding whether to show the infobar or not?

To #9: we don't have one flag to disable all the features for perf. Instead, we have lots of flag, one for each features we disable (see https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py#L77). This would allow us to carefully track how Chrome under perf test is different from the CHrome that our users use.

Great, thanks for that. I'll add a flag to not show the disclosure and update the tests to use it.

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

commit f714d2c9fc6f4d8309833d3329fc58b8bba4832a
Author: benwells <benwells@chromium.org>
Date: Thu Feb 23 06:10:26 2017

Early exit from the search geolocation disclosure if it is disabled.

There is no need to perform all the checks for showing the disclosure if
the new search geolocation consistency feature is disabled.

BUG= 681663 

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

[modify] https://crrev.com/f714d2c9fc6f4d8309833d3329fc58b8bba4832a/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc

I realised my theory was wrong as the infobar only shows if a flag is on, and it is off by default. So I landed a change which should make any impact without the flag minimal, but it still hasn't helped.

In the graphs there seem to be two things measured (...+obama and ...+obama_ref_), and the gap between them is what grew. What is the difference between the two things?

I'm now wondering if maybe it wasn't my change at all. Looking at the graphs there is a significant widening of the gap between the two lines, but it isn't clear what point the gap widens at (the gap size is noisy).

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

The ...+obama is running the benchmark on TOT chrome, the ...+obama_ref_ is running the benchmark on stable Chrome (chrome 55). So what's going on here is TOT chrome were having higher input latency compared with stable Chrome :-(

I restarted the bisect to make sure. 

=== Auto-CCing suspected CL author benwells@chromium.org ===

Hi benwells@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : benwells
  Commit : b3c6a28850a816162c59526005665c683ca42308
  Date   : Fri Jan 13 14:13:45 2017
  Subject: Show the search geolocation disclosure from geolocation API use.

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : smoothness.top_25_smooth
  Metric       : mean_input_event_latency/https___www.google.com__hl_en_q_barack+obama
  Change       : 20.76% | 7.682 -> 9.27683333333

Revision             Result                   N
chromium@443534      7.682 +- 0.545447        6      good
chromium@443544      7.656 +- 0.698536        6      good
chromium@443549      7.629 +- 0.504716        6      good
chromium@443551      7.72933 +- 0.668031      6      good
chromium@443552      7.7425 +- 0.409604       6      good
chromium@443553      9.27683 +- 0.702929      6      bad       <--

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=https...www.google.com..hl.en.q.barack.obama smoothness.top_25_smooth

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8986776680064822096

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5811591930445824


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!

Hmm ... ok ...

What flags does this run with? Is there any chance it could have some finch experiments applied (e.g. ones in testing/variations/fieldtrial_testing_config.json)??

Sorry for the confusion, the perfbots DO run with the flags in testing/variations/fieldtrial_testing_config.json

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

commit 333a6dee3c08b91580f27ab196288bb7cdb21fdb
Author: benwells <benwells@chromium.org>
Date: Wed Mar 01 00:44:53 2017

Add switch to disable showing the search geolocation disclosure UI.

This UI can impact perf tests, so a switch is being added to allow the
impact to be disabled.

BUG= 681663 

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

[modify] https://crrev.com/333a6dee3c08b91580f27ab196288bb7cdb21fdb/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc
[modify] https://crrev.com/333a6dee3c08b91580f27ab196288bb7cdb21fdb/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.h
[modify] https://crrev.com/333a6dee3c08b91580f27ab196288bb7cdb21fdb/chrome/common/chrome_switches.cc
[modify] https://crrev.com/333a6dee3c08b91580f27ab196288bb7cdb21fdb/chrome/common/chrome_switches.h

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

commit 4a49718819f42a744f99682a9571006b99258f8a
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Thu Mar 02 02:01:39 2017

Roll src/third_party/catapult/ bfa19dece..b06826430 (2 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/bfa19decec4a..b06826430a44

$ git log bfa19dece..b06826430 --date=short --no-merges --format='%ad %ae %s'
2017-03-01 benwells Add argument to chrome perf tests to disable search geo disclosure.
2017-03-01 jessimb Update not available row values to not available.

Created with:
  roll-dep src/third_party/catapult
BUG= 681663 

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=catapult-sheriff@chromium.org

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

[modify] https://crrev.com/4a49718819f42a744f99682a9571006b99258f8a/DEPS

I think it's better now, but it's a bit hard to tell.

Ben: it seems like TOT chrome is now close to ref build again, which is great. Let wait for a few more days to check the data, then we can close this bug, I think.

OK, I'm calling it. Reopen if you disagree...