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

Sure, I'll take a look now and provide a patch ASAP. 

=== Auto-CCing suspected CL author jfernandez@igalia.com ===

Hi jfernandez@igalia.com, 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 : [css-align] New CSS Value 'normal' for Self Alignment
Author  : jfernandez
Commit description:
  
The Box Alignment specification defines a new value 'normal' to be
used as default for the different layout models, which will define the
specific behavior for each case. This patch adds a new CSS value in
the parsing logic and adapts the Self Alignment properties to the new
value.

The 'auto' value is no longer valid for the 'align-items' property and
the Computed Value will be always the specified value. However, it's
still valid for justify-items and both of Self Alignment
properties. We will use the StyleAdjuster to resolve these 'auto'
values.

In order to deal with repaint and relayout whenever some alignment
property changes and resolve again the 'auto' values, current
implementation relies on triggering a Reattach whenever any of these
properties changes. This is a source of many problems for animations,
plugins, ... (See  bug 580070  for details). The correct way of
implementing this is using "Inherit" instead so we can run the
StyleAdjuster again only for the nodes affected by the change.

This approach reduces considerably the number of style invalidations
and let us unskip the align-items repaint and relayout related
tests. These, as well as the justify-items related ones, had to be
rebaselined due the different invalidation areas generated.

Because of resolving everything in the StyleAdjuster, we don't need to
duplicate the resolution logic for generating the computed style. We
only have to deal with the 'auto' flag duality, which is not both
'auto' and 'normal' in any post-adjustement logic. This also gives us
the opportunity to get rid of the ShadowDOM and slotted elements
issues, since we don't have to deal with parent's style.

Additionally, this patch updates the layout logic as well, for both
Flexbox and Grid layout models.

BUG= 565883 ,  474798 ,   448371 ,  582230 

Review-Url: https://codereview.chromium.org/1709963002
Cr-Commit-Position: refs/heads/master@{#412728}
Commit  : fe59cb90e7b08f1faa146088ddbd7589d87eebb5
Date    : Thu Aug 18 03:58:16 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@412724  34.7202  0.133823  5  good
chromium@412727  34.6257  0.103728  5  good
chromium@412728  38.0545  0.187044  5  bad    <--
chromium@412729  37.4985  0.121239  5  bad
chromium@412734  37.5156  0.276455  5  bad
chromium@412744  37.2497  0.204636  5  bad

Bisect job ran on: android_nexus5X_perf_bisect
Bug ID: 642263

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_style.top_25
Test Metric: update_style_cold/update_style_cold
Relative Change: 7.29%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/622
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9002923217378762464


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

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

After analyzing the change identified as the root cause of the regression and once all the perf try jobs have been completed, I can extract the following conclusions:

* the root cause of the regression is the StyleAdjuster logic (not a surprise, since it's always been)
     
* our efforts to reduce the impact on performance of the new syle adjustment logic are insufficient or not correctly implemented.

I'll focus on analyzing this change:

https://chromium.googlesource.com/chromium/src/+/fe59cb90e7b08f1faa146088ddbd7589d87eebb5%5E!/#F38

We designed to avoid copy-on-write penalties as much as possible by resolving the 'auto' values ONLY when there is a explicit CSS rule in the parent element, avoiding resolving default values.

I've found that that when we are using a harcoded value when trying to determine if a Self-Alignment 'auto' value is going to be resolved using its parent's default value. The problem is that such default value might be different depending on whether we have Grid Layout enabled or not in the runtime flags.

I've sent additional perf try jobs with a patched version of the style adjustment logic, so we can figure out whether that was the reason why our optimizations didn't get the expected results.

I think I managed to solve the performance regression. I've sent a tentative fix for review at https://crrev.com/2298613004

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

commit bab582c0efeee11acecc6754e91d1af982044bd1
Author: jfernandez <jfernandez@igalia.com>
Date: Wed Aug 31 23:09:18 2016

[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in  bug 642263 .

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG= 642263 

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

[modify] https://crrev.com/bab582c0efeee11acecc6754e91d1af982044bd1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
[modify] https://crrev.com/bab582c0efeee11acecc6754e91d1af982044bd1/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp

This issue should be FIXED now. Could someone verify it please ?

Looks like it's been fixed from the graph, thanks for the quick fix! I'm re-opening this for now, will close once we merge this to 54.

Your change meets the bar and is auto-approved for M54 (branch: 2840)

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

commit 36d9fdb4a1f1a27447d502ef1ea457f2156154d6
Author: Timothy Loh <timloh@chromium.org>
Date: Fri Sep 02 06:02:37 2016

[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in  bug 642263 .

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG= 642263 

Review-Url: https://codereview.chromium.org/2298613004
Cr-Commit-Position: refs/heads/master@{#415795}
(cherry picked from commit bab582c0efeee11acecc6754e91d1af982044bd1)

Review URL: https://codereview.chromium.org/2299803004 .

Cr-Commit-Position: refs/branch-heads/2840@{#120}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/36d9fdb4a1f1a27447d502ef1ea457f2156154d6/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
[modify] https://crrev.com/36d9fdb4a1f1a27447d502ef1ea457f2156154d6/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp

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

commit 36d9fdb4a1f1a27447d502ef1ea457f2156154d6
Author: Timothy Loh <timloh@chromium.org>
Date: Fri Sep 02 06:02:37 2016

[css-align] Check runtime flags for align-items 'auto' value resolution

The StyleAdjuster::adjustStyleForAlignment logic is optimized to avoid
resolving 'auto' values on alignment properties using its parent's
align-items default value. This way we can minimize the copy-on-write
overhead, since 'auto' is the default value of some CSS Alignment
properties.

However, align-items default value is different depending on whether the
CSS Grid Layout feature is enabled in runtime flags or not. We were not
checking out the runtime flags when applying the optimization mentioned
before, hence, when Grid is disabled, we are overwriting all the
align-self properties with 'auto' values (the default) with its parent's
align-items default value (stretch). This was verified as the root cause
of the performance regression reported in  bug 642263 .

This patch solves the issue by using the proper default value, depending
on the runtime flags, when evaluating whether resolving the align-self's
'auto' values or not.

BUG= 642263 

Review-Url: https://codereview.chromium.org/2298613004
Cr-Commit-Position: refs/heads/master@{#415795}
(cherry picked from commit bab582c0efeee11acecc6754e91d1af982044bd1)

Review URL: https://codereview.chromium.org/2299803004 .

Cr-Commit-Position: refs/branch-heads/2840@{#120}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/36d9fdb4a1f1a27447d502ef1ea457f2156154d6/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
[modify] https://crrev.com/36d9fdb4a1f1a27447d502ef1ea457f2156154d6/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp