Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1152)

Issue 1927913002: Old lines may be detached / extracted during layout. (Closed)

Created:
4 years, 7 months ago by mstensho (USE GERRIT)
Modified:
4 years, 7 months ago
CC:
chromium-reviews, szager+layoutwatch_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Old lines may be detached / extracted during layout. Back out over-simplified code from https://codereview.chromium.org/1915803004/ Since lines from an old layout pass that haven't yet been relaid out may not be in the line box list at all at some given point during layout, lastRootBox() didn't work as expected. It would either return the wrong last-line, or even nullptr. BUG=607451 Committed: https://crrev.com/7c96d3817765ae0d848bdf2962c7cc1c52384848 Cr-Commit-Position: refs/heads/master@{#390508}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -2 lines) Patch
A third_party/WebKit/LayoutTests/fast/block/float/remove-line-above-float-above-line-crash.html View 1 chunk +13 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/fast/block/float/remove-line-above-float-above-line-crash-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp View 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 12 (5 generated)
mstensho (USE GERRIT)
https://www.youtube.com/watch?v=tcGQpjCztgA
4 years, 7 months ago (2016-04-28 11:37:47 UTC) #2
szager1
lgtm
4 years, 7 months ago (2016-04-28 17:36:10 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1927913002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1927913002/1
4 years, 7 months ago (2016-04-28 20:20:53 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_chromeos_rel/builds/179980) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, ...
4 years, 7 months ago (2016-04-28 21:02:30 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1927913002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1927913002/1
4 years, 7 months ago (2016-04-28 21:06:25 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 7 months ago (2016-04-28 22:25:38 UTC) #10
commit-bot: I haz the power
4 years, 7 months ago (2016-04-30 17:22:04 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/7c96d3817765ae0d848bdf2962c7cc1c52384848
Cr-Commit-Position: refs/heads/master@{#390508}

Powered by Google App Engine
This is Rietveld 408576698