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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxrbQuAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxraMpQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxsforQoM


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

chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia

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

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

Hi trchen@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Remove extraneous frameView->setNeedsLayout() in LayoutPart
Author  : trchen
Commit description:
  
This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.

Review-Url: https://codereview.chromium.org/2188483004
Cr-Commit-Position: refs/heads/master@{#408529}
Commit  : 6f72ce705eea7d49483b1cc75439260ebde643c4
Date    : Fri Jul 29 00:25:18 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@408504  1321136  24352.1  5  good
chromium@408527  1320216  20309.4  5  good
chromium@408528  1298201  15993.4  5  good
chromium@408529  1427835  17136.8  5  bad    <--
chromium@408530  1413374  24355.7  5  bad
chromium@408532  1415202  27383.6  5  bad
chromium@408538  1432437  12100.4  5  bad
chromium@408550  1426105  22213.7  5  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 633199

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_desktop
Test Metric: load_tools-memory:chrome:all_processes:reported_by_chrome:skia:effective_size_avg/load_tools_docs
Relative Change: 7.95%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1762
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005528720466869520


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5896788314161152

| 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!

Perf sheriff checking in: any update on this regression trchen@? Could you investigate or revert the suspected CL?

I spent some time to research this issue last week, but couldn't figure out the root cause. My best guess is that because the CL changed the timing of layout a little bit, and it could allow things to start to raster earlier. I think it is very unlikely to cause a real regression on released product.

> My best guess is that because the CL changed the timing of layout a
little bit, and it could allow things to start to raster earlier.

The benchmark waits until the page loads (plus a few more extra
seconds), so everything should raster in either case (no?).

> I think it is very unlikely to cause a real regression on released
> product.

Why do you think so? The story loads Google Docs. That seems like a very
common use case. In general, System Health benchmarks aim to be as
realistic as possible.

Alright. I agree 8~9% is a lot to ignore by speculation. As there already work built on top of it, I will make a manual revert.

Thanks! You can run the benchmark locally using the following command:

$ tools/perf/run_benchmark system_health.memory_desktop --browser=release --device=desktop --story-filter=load:tools:docs

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

commit 329f73bb8976808995da6fb5f9783ac5aed7d128
Author: trchen <trchen@chromium.org>
Date: Sat Aug 13 03:06:32 2016

Revert "Remove extraneous frameView->setNeedsLayout() in LayoutPart"

Manually reverts https://crrev.com/6f72ce705eea7d49483b1cc75439260ebde643c4

The commit is suspected to cause a memory regression. The root cause is not
trivial. Revert it first while we research the issue.

BUG= 633199 

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

[modify] https://crrev.com/329f73bb8976808995da6fb5f9783ac5aed7d128/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/329f73bb8976808995da6fb5f9783ac5aed7d128/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
[modify] https://crrev.com/329f73bb8976808995da6fb5f9783ac5aed7d128/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/329f73bb8976808995da6fb5f9783ac5aed7d128/third_party/WebKit/Source/core/frame/FrameView.h
[modify] https://crrev.com/329f73bb8976808995da6fb5f9783ac5aed7d128/third_party/WebKit/Source/core/layout/LayoutPart.cpp