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

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


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

chromium-rel-mac11
chromium-rel-win7-gpu-intel
win-high-dpi

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

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

Hi lukasza@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 : Lukasz Anforowicz
  Commit : 6b4a012891b2b9ab32b2c12fe196d8c79cae0001
  Date   : Thu Nov 09 20:41:38 2017
  Subject: Avoid restoring subframes created *dynamically*.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : blink_perf.parser
  Metric       : iframe-append-remove/iframe-append-remove
  Change       : 11.38% | 344.278721764 -> 305.112511596

Revision             Result                  N
chromium@515202      344.279 +- 16.6642      6      good
chromium@515251      342.126 +- 8.18067      6      good
chromium@515276      340.346 +- 9.71041      6      good
chromium@515278      339.123 +- 6.25624      6      good
chromium@515279      308.633 +- 6.09263      6      bad       <--
chromium@515282      306.115 +- 24.6241      6      bad
chromium@515288      304.873 +- 7.67431      6      bad
chromium@515300      305.113 +- 3.55047      6      bad

Please refer to the following doc on diagnosing blink_perf regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/speed/benchmark_harnesses/blink_perf.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.parser

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

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


For feedback, file a bug with component Speed>Bisection

I can understand that r515279 might have impacted iframe-append-remove benchmark (which repeatedly creates a dynamic frame and immediately removes it).  I think the CL shouldn't impact |should_ask_browser| in RenderFrameImpl::DecidePolicyForNavigation, but the CL does mean that the set of unique names grows linearily (it stayed constant before).  We might be able to prune the set to help with the benchmark.

tdresser@: I don't understand why r515279 would impact the fixed-grid-lots-of-stretched-data benchmark.  Would it make sense / would it be possible to open a separate bug to track the regression of the other benchmark?  I suspect / hope that the regression is not caused by r515279.

Split it out, thanks.

tdresser@ - done - please see  issue 784506 .  I hope I've added all the relevant labels, but I have never did perf-sheriffing work, so I might have missed some place where I should have associated the bug with the specific regression/benchmark.

FWIW, I've verified that |should_ask_browser| doesn't enter the picture at all in the scenario exercised by the benchmark - in this scenario we don't even enter the if's body, because info.is_history_navigation_in_new_child_frame is false.

I have a WIP CL @ https://crrev.com/c/767627 that removes stale entries when removing frames that were created by a script.  I speculate that this CL will address the observed performance regression in third_party/WebKit/PerformanceTests/Parser/iframe-append-remove.html

 Issue 784506  has been merged into this issue.

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

commit 7bfb2e935b46953feff17003977ffebbed62e878
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Wed Nov 22 17:19:45 2017

When removing a dynamically created frame, remove its FNEs as well.

A dynamically created frame (i.e. a frame created from javascript)
always gets a fresh, random, unique name when it gets created
and recreated.  This prevents reusing the frame's older history
entries when the frame is recreated (or when another dynamic frame
is created and confused with the frame - see  https://crbug.com/500260 ).
This means that the old history entries are never used and therefore
can be safely deleted when a dynamically created frame is removed.
Deleting these entries saves memory (and indirectly CPU time).

This CL combines 5 changes:
1. Propagating |is_created_by_script| from
   RenderFrameImpl::CreateChildFrame, through IPC, into
   FrameTreeNode::is_created_by_script.
2. Having FrameTreeNode::RemoveChild remove the corresponding
   history entries if the child was dynamically created.
3. Adding an explicit browser test that verifies that
   the history entries of dynamically created frames are removed.
4. Tweaking existing tests so they match the new behavior.
5. Opportunistic |git cl lint| fixes.

Bug:  784356 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Id72c5a0fda6d63ff91f4ec3bacb0e0c412fd02be
Reviewed-on: https://chromium-review.googlesource.com/767627
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518664}
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree.h
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree_node.h
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree_node_blame_context_unittest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/frame_tree_unittest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/frame_host/render_frame_message_filter.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/browser/top_document_isolation_browsertest.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/common/frame_messages.h
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/test/data/navigation_controller/page_with_iframe_simple.html
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/content/test/test_render_frame_host.cc
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/fast/history/history-length-append-subframe-no-hash-expected.txt
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/fast/history/history-length-append-subframe-with-hash-expected.txt
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/fast/history/redirect-via-iframe-expected.txt
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/fast/loader/frame-location-change-not-added-to-history-expected.txt
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/fast/loader/frame-src-change-not-added-to-history-expected.txt
[modify] https://crrev.com/7bfb2e935b46953feff17003977ffebbed62e878/third_party/WebKit/LayoutTests/http/tests/navigation/back-to-dynamic-iframe-expected.txt

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

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

Hi lukasza@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 : Lukasz Anforowicz
  Commit : 7bfb2e935b46953feff17003977ffebbed62e878
  Date   : Wed Nov 22 17:19:45 2017
  Subject: When removing a dynamically created frame, remove its FNEs as well.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : blink_perf.parser
  Metric       : iframe-append-remove/iframe-append-remove
  Change       : 11.20% | 300.531028812 -> 334.197505705

Revision             Result                  N
chromium@518603      300.531 +- 3.14597      6      good
chromium@518634      299.442 +- 7.44657      6      good
chromium@518649      300.835 +- 6.36369      6      good
chromium@518657      302.232 +- 7.80404      6      good
chromium@518661      303.59 +- 14.1446       6      good
chromium@518663      296.735 +- 10.6105      6      good
chromium@518664      333.306 +- 7.7875       6      bad       <--
chromium@518725      334.198 +- 16.9869      6      bad

Please refer to the following doc on diagnosing blink_perf regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/speed/benchmark_harnesses/blink_perf.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.parser

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

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


For feedback, file a bug with component Speed>Bisection

As confirmed by #c14, r518664 has fixed around 75% of the perf regression reported by this bug (the original regression caused by r515279 was around -40, this got fixed by around +30 by r518664).  I hope this is good enough - I don't see how we could improve this further.