New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 852976 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.3%-11.9% regression in blink_perf.layout at 565841:565876

Project Member Reported by briander...@chromium.org, Jun 14 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jun 14 2018

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

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


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

android-nexus5
android-webview-nexus6
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jun 15 2018

Cc: atotic@chromium.org
Owner: atotic@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/146bd8f9240000

[LayoutNG] LayoutBox::AvailableLogicalHeight can be indefinite by atotic@chromium.org
https://chromium.googlesource.com/chromium/src/+/eec965aea44873b0f77c6bd085c4ff3f7166cdd3
138.6 → 124.4 (-14.25)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Jun 16 2018

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/117787e5240000
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Jun 16 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/1794d4f5240000

[LayoutNG] nested-percent-height regression by atotic@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/1103289/1
121.3 → 129.1 (+7.734)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Jun 17 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16fa8825240000

[LayoutNG] nested-percent-height regression by atotic@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/1103289/2
121.7 → 115.2 (-6.467)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Jun 18 2018

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14ad159d240000

[LayoutNG] nested-percent-height regression by atotic@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/1103289/3
119.6 → 135.6 (+16)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 18 2018

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

commit ffe80a5718e6630109983b084f168c59d6f48006
Author: Aleks Totic <atotic@chromium.org>
Date: Mon Jun 18 20:05:19 2018

[LayoutNG] nested-percent-height regression

Will need help with this one.
I think it is a real regression.
Offending CL modifed LayoutBox::AvailableLogicalHeight
https://chromium-review.googlesource.com/c/chromium/src/+/1093996
LayoutBox::ComputePercentageLogicalHeight gets called 740 times
for each layout.

I was unable to reproduce this on my Linux box. Uploading
so I can run on Android.

The test also renders incorrectly in Legacy, correctly in NG.
The root cause of incorrect rendering is that
LayoutBox::ComputePercentageLogicalHeight for TableCell is
incorrect in Legacy when:

  #target {
    overflow: auto;
    height: 100%;
  }
  td {
    height: 100%;
  }
  table {
  }

<table>
  <td>
    <div id="target">

LayoutBox::ComputePercentageLogicalHeight should in this case return
intrinsic_logical_height instead of 0.

Bug:  852976 
Change-Id: Iaf65c226eb9727877d14e68156d522421af57c87
Reviewed-on: https://chromium-review.googlesource.com/1103289
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568123}
[modify] https://crrev.com/ffe80a5718e6630109983b084f168c59d6f48006/third_party/blink/renderer/core/layout/layout_box.cc

Status: Fixed (was: Assigned)

Sign in to add a comment