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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 210043008: Kill outlineBox (Closed)

Created:
6 years, 9 months ago by Julien - ping for review
Modified:
6 years, 8 months ago
CC:
blink-reviews, ed+blinkwatch_opera.com, krit, gyuyoung.kim_webkit.org, zoltan1, dsinclair, bemjb+rendering_chromium.org, eae+blinkwatch, leviw+renderwatch, kouhei+svg_chromium.org, fs, blink-layers+watch_chromium.org, dglazkov+blink, f(malita), adamk+blink_chromium.org, jchaffraix+rendering, pdr., rwlbuis, Stephen Chennney, rune+blink
Visibility:
Public.

Description

Kill outlineBox The outline box concept wasn't clearly defined. It was also unneeded if we track the outline into the visual overflow, which we now do! This change is a mechanical removal of the associated code. It keeps the logic in repaintAfterLayout that handles changes in visual overflow as it is required to correctly invalidate some cases where our simplistic visual overflow model using a single rect falls short. Most tests changes correspond to less invalidation, except for * fast/repaint/4776765.html which is getting 2 new invalidations coresponding to getting a 5px outline. * fast/box-shadow/negative-shadow-box*.html which grew to include the full box-shadow instead of being the box's size. BUG=345452 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170115

Patch Set 1 #

Total comments: 8

Patch Set 2 : Updated patch after the cross-reviews #

Patch Set 3 : Rebaselined patch, hoping it would solve the bot failure I can't reproduce locally #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -181 lines) Patch
M LayoutTests/TestExpectations View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
M LayoutTests/compositing/repaint/resize-repaint-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/box-shadow/negative-shadow-box-shrink-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/repaint/border-radius-repaint-2-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/content-into-overflow-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/fast/repaint/overflow-into-content-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/fast/repaint/table-section-repaint-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/fast/repaint/transform-disable-layoutstate-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/win/fast/repaint/border-radius-repaint-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/platform/win/fast/repaint/border-repaint-glitch-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/platform/win/fast/repaint/box-shadow-dynamic-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M LayoutTests/platform/win/fast/repaint/repaint-resized-overflow-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/win/fast/repaint/selection-after-delete-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/platform/win/fast/repaint/table-collapsed-border-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/frame/FrameView.cpp View 1 2 1 chunk +1 line, -10 lines 0 comments Download
M Source/core/html/HTMLAreaElement.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/LayoutRectRecorder.cpp View 1 2 chunks +0 lines, -6 lines 0 comments Download
M Source/core/rendering/LayoutRepainter.cpp View 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/rendering/RenderBox.h View 1 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 1 chunk +0 lines, -24 lines 0 comments Download
M Source/core/rendering/RenderLayer.cpp View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/rendering/RenderLayerRepainter.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/rendering/RenderLayerRepainter.cpp View 4 chunks +1 line, -6 lines 0 comments Download
M Source/core/rendering/RenderObject.h View 1 2 4 chunks +2 lines, -10 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 2 10 chunks +14 lines, -76 lines 0 comments Download
M Source/core/rendering/RenderView.h View 3 chunks +0 lines, -10 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGModelObject.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGModelObject.cpp View 1 chunk +0 lines, -12 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
Julien - ping for review
Exciting change for review!
6 years, 9 months ago (2014-03-24 23:55:20 UTC) #1
leviw_travelin_and_unemployed
LGTM! https://codereview.chromium.org/210043008/diff/1/LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt File LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt (right): https://codereview.chromium.org/210043008/diff/1/LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt#newcode4 LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt:4: (rect 100 160 640 40) This one also ...
6 years, 9 months ago (2014-03-25 04:40:57 UTC) #2
eseidel
lgtm Amazing. https://codereview.chromium.org/210043008/diff/1/Source/core/html/HTMLAreaElement.cpp File Source/core/html/HTMLAreaElement.cpp (right): https://codereview.chromium.org/210043008/diff/1/Source/core/html/HTMLAreaElement.cpp#newcode106 Source/core/html/HTMLAreaElement.cpp:106: size = obj->absoluteClippedOverflowRect().size(); absoluteClippedOverflowRect does a walk ...
6 years, 9 months ago (2014-03-25 06:32:18 UTC) #3
dsinclair
lgtm https://codereview.chromium.org/210043008/diff/1/Source/core/rendering/RenderObject.cpp File Source/core/rendering/RenderObject.cpp (right): https://codereview.chromium.org/210043008/diff/1/Source/core/rendering/RenderObject.cpp#newcode1511 Source/core/rendering/RenderObject.cpp:1511: if (mustRepaintBackgroundOrBorder() && (newBounds != oldBounds)) nit: This ...
6 years, 9 months ago (2014-03-25 14:36:06 UTC) #4
Julien - ping for review
https://codereview.chromium.org/210043008/diff/1/LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt File LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt (right): https://codereview.chromium.org/210043008/diff/1/LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt#newcode4 LayoutTests/fast/box-shadow/negative-shadow-box-expand-expected.txt:4: (rect 100 160 640 40) On 2014/03/25 04:40:58, Levi ...
6 years, 9 months ago (2014-03-25 21:52:34 UTC) #5
Julien - ping for review
The CQ bit was checked by jchaffraix@chromium.org
6 years, 9 months ago (2014-03-25 21:53:13 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/210043008/50001
6 years, 9 months ago (2014-03-25 21:53:18 UTC) #7
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-25 22:59:26 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-25 22:59:26 UTC) #9
Julien - ping for review
The CQ bit was checked by jchaffraix@chromium.org
6 years, 9 months ago (2014-03-26 00:37:04 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/210043008/50001
6 years, 9 months ago (2014-03-26 00:37:11 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-26 01:47:47 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-26 01:47:48 UTC) #13
Julien - ping for review
The CQ bit was checked by jchaffraix@chromium.org
6 years, 8 months ago (2014-03-26 15:00:53 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/210043008/50001
6 years, 8 months ago (2014-03-26 15:01:03 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-03-26 16:08:47 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 8 months ago (2014-03-26 16:08:48 UTC) #17
Julien - ping for review
The CQ bit was checked by jchaffraix@chromium.org
6 years, 8 months ago (2014-03-26 16:11:23 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/210043008/50001
6 years, 8 months ago (2014-03-26 16:11:29 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-03-26 17:19:04 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 8 months ago (2014-03-26 17:19:04 UTC) #21
Julien - ping for review
The CQ bit was checked by jchaffraix@chromium.org
6 years, 8 months ago (2014-03-27 00:36:40 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/210043008/200001
6 years, 8 months ago (2014-03-27 00:36:46 UTC) #23
commit-bot: I haz the power
6 years, 8 months ago (2014-03-27 01:07:57 UTC) #24
Message was sent while issue was closed. 
Change committed as 170115

Powered by Google App Engine
This is Rietveld 408576698