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

Issue 2471933002: Reland of Improve how the column balancer handles top margins on floats. (Closed)

Created:
4 years, 1 month ago by mstensho (USE GERRIT)
Modified:
4 years, 1 month 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
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland of Improve how the column balancer handles top margins on floats. (patchset #1 id:1 of https://codereview.chromium.org/2468193002/ ) Reason for revert: csspaint/invalidation-background-image.html was also failing (flaky) before landing this CL. Original issue's description: > Revert of Improve how the column balancer handles top margins on floats. (patchset #3 id:40001 of https://codereview.chromium.org/2465363003/ ) > > Reason for revert: > Speculative revert to fix csspaint/invalidation-background-image.html failure on "WebKit Win7 (dbg)" bot. > > Failed build: > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7%20%28dbg%29/builds/7961 > > Original issue's description: > > Improve how the column balancer handles top margins on floats. > > > > Float margins do not collapse with column boundaries, so we should make room > > for them after the break, if the border box of the float starts in the next > > column. > > > > Let the balancer work on the margin box of the float (and the border box for > > all other objects). For floats, we want to insert breaks before the > > margin-before edge, not the border-before edge. This lets us remove > > some special-code for unbreakable floats in InitialColumnHeightFinder, which > > was the only place that previously bothered about this. > > > > Changed how we determine which objects to process. We used to include the > > overflow both before and after the border box, but we really don't have to > > bother with content preceding it, since that shouldn't undergo fragmentation > > anyway. > > > > Discovered (one test regressed) that logicalHeightIncludingOverflow() also > > included clipped overflow, which certainly wasn't the intention. This didn't > > make much of a difference as long as the method was only called to check if we > > could skip re-layout. But now we also use it to determine the column height. > > Fixed it to only include visible overflow and renamed it to > > logicalHeightWithVisibleOverflow(). > > > > Committed: https://crrev.com/7c82da727f64121aa34aa1decf82452c37ef7a2d > > Cr-Commit-Position: refs/heads/master@{#429245} > > TBR=eae@chromium.org,mstensho@opera.com > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > > Committed: https://crrev.com/58f81484437d367285de9f0fc1fdd4034eb5c333 > Cr-Commit-Position: refs/heads/master@{#429265} TBR=eae@chromium.org,rouslan@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/df41288325ac9c6c5a8fd7bbc8bd18fa9de87ea7 Cr-Commit-Position: refs/heads/master@{#429415}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -34 lines) Patch
A third_party/WebKit/LayoutTests/fast/multicol/balance-float-with-margin-top-and-line-after-break-2.html View 1 chunk +21 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ColumnBalancer.h View 3 chunks +7 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ColumnBalancer.cpp View 11 chunks +46 lines, -26 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.h View 1 chunk +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
mstensho (USE GERRIT)
Created Reland of Improve how the column balancer handles top margins on floats.
4 years, 1 month ago (2016-11-02 22:10:38 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2471933002/1
4 years, 1 month ago (2016-11-02 22:11:20 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 1 month ago (2016-11-02 22:12:37 UTC) #4
commit-bot: I haz the power
4 years, 1 month ago (2016-11-02 22:14:22 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/df41288325ac9c6c5a8fd7bbc8bd18fa9de87ea7
Cr-Commit-Position: refs/heads/master@{#429415}

Powered by Google App Engine
This is Rietveld 408576698