Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 2857583007: [RLS] Don't add to ScrollableAreaSet if the size is zero (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by MuVen
Modified:
4 days, 8 hours ago
Reviewers:
Srirama, skobes
CC:
chromium-reviews, blink-reviews, dshwang, blink-reviews-paint_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[RLS] Don't add to ScrollableAreaSet if the size is zero Add ScrollableArea only if the box size is not zero and there is a horizontal or vertical overflow scroll. BUG=711474 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2857583007 Cr-Commit-Position: refs/heads/master@{#487525} Committed: https://chromium.googlesource.com/chromium/src/+/03a08a02a028bdc0042b34cf36d229f02f9ce5fc

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add box size check for determining scrollable area #

Total comments: 2

Patch Set 3 : address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -17 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h View 1 2 2 chunks +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp View 1 2 5 chunks +7 lines, -13 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 40 (25 generated)
Srirama
https://codereview.chromium.org/2857583007/diff/1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): https://codereview.chromium.org/2857583007/diff/1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp#newcode1780 third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1780: if (scrolls_overflow_ && frame_view->IsScrollable()) { IsScrollable is calling GetScrollingReasons ...
2 months, 2 weeks ago (2017-05-05 06:21:23 UTC) #8
Srirama
On 2017/05/05 06:21:23, Srirama wrote: > https://codereview.chromium.org/2857583007/diff/1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp > File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): > > https://codereview.chromium.org/2857583007/diff/1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp#newcode1780 > ...
2 months ago (2017-05-19 12:22:48 UTC) #9
skobes
On 2017/05/19 12:22:48, Srirama wrote: > On 2017/05/05 06:21:23, Srirama wrote: > > > https://codereview.chromium.org/2857583007/diff/1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp ...
2 months ago (2017-05-19 20:31:41 UTC) #10
Srirama
On 2017/05/19 20:31:41, skobes wrote: > On 2017/05/19 12:22:48, Srirama wrote: > > On 2017/05/05 ...
2 months ago (2017-05-20 13:11:40 UTC) #11
skobes
On 2017/05/20 13:11:40, Srirama wrote: > FrameView::IsScrollable may not be required. As i mentioned in ...
2 months ago (2017-05-22 14:56:40 UTC) #12
Srirama
On 2017/05/22 14:56:40, skobes wrote: > On 2017/05/20 13:11:40, Srirama wrote: > > FrameView::IsScrollable may ...
3 weeks, 1 day ago (2017-06-30 13:58:50 UTC) #13
skobes
On 2017/06/30 13:58:50, Srirama wrote: > sorry for the delayed response. I debugged further. Following ...
2 weeks, 3 days ago (2017-07-05 18:43:15 UTC) #14
Srirama
On 2017/07/05 18:43:15, skobes wrote: > On 2017/06/30 13:58:50, Srirama wrote: > > sorry for ...
1 week, 3 days ago (2017-07-12 06:16:17 UTC) #23
skobes
https://codereview.chromium.org/2857583007/diff/20001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): https://codereview.chromium.org/2857583007/diff/20001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp#newcode1018 third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1018: if (Box().Size().IsZero()) Actually it would be simpler to put ...
1 week, 1 day ago (2017-07-15 00:44:32 UTC) #24
Srirama
PTAL. https://codereview.chromium.org/2857583007/diff/20001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp File third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp (right): https://codereview.chromium.org/2857583007/diff/20001/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp#newcode1018 third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp:1018: if (Box().Size().IsZero()) On 2017/07/15 00:44:31, skobes wrote: > ...
5 days, 12 hours ago (2017-07-17 14:35:24 UTC) #29
skobes
Please update the change description to be more clear, like "[RLS] Don't add to ScrollableAreaSet ...
5 days, 8 hours ago (2017-07-17 17:38:15 UTC) #30
Srirama
On 2017/07/17 17:38:15, skobes wrote: > Please update the change description to be more clear, ...
4 days, 21 hours ago (2017-07-18 05:20:54 UTC) #34
skobes
lgtm
4 days, 10 hours ago (2017-07-18 16:02:38 UTC) #36
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/2857583007/40001
4 days, 10 hours ago (2017-07-18 16:02:51 UTC) #37
commit-bot: I haz the power
4 days, 8 hours ago (2017-07-18 18:13:37 UTC) #40
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/03a08a02a028bdc0042b34cf36d2...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973