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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=bb9f3a57e669c5a9c32c2e1fdd1b40c21adabccf460f316a05c20dfcd38e9942


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

mojo-linux-perf

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

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

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

Hi hajimehoshi@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 : Hajime Hoshi
  Commit : 3b236983f49cd29b183e446f0bad908ac3cab88b
  Date   : Mon Aug 07 07:24:17 2017
  Subject: Fix a leak of a document that is held by AutofillAgent

Bisect Details
  Configuration: linux_perf_bisect
  Benchmark    : loading.desktop.network_service
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/Free.fr
  Change       : 199.42% | 39.3713333333 -> 117.88475

Revision             Result                  N
chromium@492268      39.3713 +- 5.32276      6      good
chromium@492275      43.7141 +- 11.075       6      good
chromium@492277      42.0943 +- 13.7114      6      good
chromium@492278      117.196 +- 22.6371      6      bad       <--
chromium@492281      117.885 +- 18.2986      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=Free.fr loading.desktop.network_service

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

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


For feedback, file a bug with component Speed>Bisection

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

Suspected Commit
  Author : Hajime Hoshi
  Commit : 3b236983f49cd29b183e446f0bad908ac3cab88b
  Date   : Mon Aug 07 07:24:17 2017
  Subject: Fix a leak of a document that is held by AutofillAgent

Bisect Details
  Configuration: linux_perf_bisect
  Benchmark    : loading.desktop.network_service
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/Free.fr
  Change       : 184.29% | 42.4848333333 -> 120.779833333

Revision             Result                  N
chromium@492268      42.4848 +- 15.2478      6      good
chromium@492275      43.7954 +- 8.98164      6      good
chromium@492277      42.4288 +- 10.852       6      good
chromium@492278      126.677 +- 23.8846      6      bad       <--
chromium@492281      120.78 +- 20.3212       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=Free.fr loading.desktop.network_service

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

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


For feedback, file a bug with component Speed>Bisection

 Issue 753654  has been merged into this issue.

+keishi

I have no idea how my CL is related to this issue.

For example, IIUC the test loading.desktop just loads Indian Times and doesn't focus any input elements (I've checked this by document.activeElement).

I don't see how either but the regression seems very serious.
https://chromeperf.appspot.com/group_report?sid=0f964081f5cfc9b815941c4a7e41b6c7e0c3aac26312088c2ba62f37f2860cf0

I think this is an expected behavior:

* All suspicious sites (www.ebay.com, IndianTimes, Free.fr) in WPR calls  AutofillAgent::FocusedNodeChanged when loading, that means |AutofillAgent::element_| is set.
* My CL resets |element_| at AutofillAgent::DidCommitProvisionalLoad, that might change cache lifetime
* All the suspicous tests are timeToFirstMeaningfulPaint with cache warm, and because the cache lifetime changed, the result changes.

My CL fixes an obvious resource leak, and the benchmark tests relied on cache that should not exist. Thus, I think what we see is not regression but the original state.

kouhei@ (an owner of loading.desktop), nasko@ (an owner of page_cycler_v2.basic_oopif), wdyt?

Given the impact of the change, I think we should investigate more.
I agree that the |AutofillAgent::element_| should be cleared, but we should know what cache impacted the ttfcp so much.

-netsvc component as I don't believe this roots to netsvc.

I'd like to propose doing Finch experiment on the fix, the data will be useful for measuring impact of:
- Amount of memory saved by this change
- TTFCP regression

From loading-dev, kinuko@ and I think we should go Finch given the regression size.
haraken: WDYT?

I agree that we should revert the CL for now but doing Finch won't be useful. Since this CL is for fixing memory leaks, we must land the CL in a reasonable timeline anyway.

How long should the cache be kept alive? We can explicitly implement the cache lifetime so that we can land the CL without causing performance regression.

As we discussed offline, we have concluded to use Finch to know how much regression this causes in the real world.

* If the regression is small enough, we can ship the leak fix.
* If not, we need to implement cache system before fixing the leak.

I'll revert the leak fix once anyway.

-memory-dev sorry for the noise.

I removed "Restrict" label as I don't think we have anything confidential here.

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

commit 53820e225c939a5a061a5e55a140a8ff5310997e
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Aug 17 02:28:11 2017

Finch testing for document leak fix in AutofillAgent

The document leak in AutofillAgent was fixed once at https://chromium-review.googlesource.com/c/597482,
but reverted because of performance regression. The fix changed the
lifetime of documents and resources.

This CL introduces Finch testing for document leak fix in AutofillAgent
to compare UMAs like TTFMP or memory usages. If TTFMP is not regressed
so much, we can ship the fix again. If not, we might need to consider
to introduce a more explicit resource life-time manager.

Bug:  753071 , 734427, 755489
Change-Id: Ica05258f730a74e7832e171fb4172fc53baac9f2
Reviewed-on: https://chromium-review.googlesource.com/612939
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495054}
[modify] https://crrev.com/53820e225c939a5a061a5e55a140a8ff5310997e/components/autofill/content/renderer/autofill_agent.cc

Any update here?

The regression itself has already been fixed.