Correct visual rects for scroll corner/resizer and non-frame composited scroll bars.

Correct visual rects for scroll corner/resizer and non-frame composited scroll bars.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938
Committed: https://crrev.com/398e024db042ebd7d81b71dfd4e5abb8419fd5de
Cr-Commit-Position: refs/heads/master@{#385270}

[wkorman, 2016-03-24 19:37:04]

wkorman@chromium.org changed reviewers:
+ chrishtr@chromium.org

[wkorman, 2016-03-24 19:37:05]

Fixes:

compositing/overflow/clip-content-under-overflow-controls.html
compositing/scrollbar-painting.html

I'm currently looking for a test case that fails with this, as I would expect
non-composited scroll corners could have non-zero location, but
compositing/scrollbar-painting.html passes even when modified to not composite.
Sending for discussion while I try dry run and look further.

[wkorman, 2016-03-24 19:37:24]

The CQ bit was checked by wkorman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-24 19:37:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/1

[chrishtr, 2016-03-24 20:54:02]

Per offline conversation: the constructor for LayoutScrollbarPart should always
be passed a
ScrollableArea, from which we should extract the visual rect.

[wkorman, 2016-03-24 21:18:58]

On 2016/03/24 at 20:54:02, chrishtr wrote:
> Per offline conversation: the constructor for LayoutScrollbarPart should
always be passed a
> ScrollableArea, from which we should extract the visual rect.

Done though change not yet uploaded, and the rects for the non-composited case
now look correct. However, two notes:

1. LayoutScrollbarPart already had a hack to forcibly retrieve the
ScrollableArea for a scroll corner/resizer:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

If we still believe it's cleaner to pass the right ScrollableArea in from all
call sites, then we should replace this hack with a direct reference to
m_scrollableArea, which I've presumptively done.

2. For composited scroll corner, the rect is now incorrectly large, since for a
(0,0 15x15) scroll corner in the sample 100x100 scrollable area from
compositing/scrollbar-painting, the visual rect will be the rect for the whole
area, so (0,0 100x100) which will work but is essentially wrong.

[commit-bot: I haz the power, 2016-03-24 22:00:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-24 22:00:24]

Dry run: This issue passed the CQ dry run.

[wkorman, 2016-03-24 23:27:19]

PTAL, updated as discussed.

[chrishtr, 2016-03-24 23:39:34]

https://codereview.chromium.org/1830333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp (right):

https://codereview.chromium.org/1830333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp:43: {
ASSERT(m_scrollableArea)

[chrishtr, 2016-03-24 23:40:18]

Also please update the description to say all the things being done. (In
particular, change
the DisplayItemClient for non-frame, composited ScrollableArea painted content
to be the
GraphicsLayer they belong to).

[wkorman, 2016-03-25 03:24:39]

Description was changed from

==========
Size visual rect properly for scroll corner/resizer.

BUG=529938
==========

to

==========
Size visual rect properly for scroll corner/resizer.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938
==========

[wkorman, 2016-03-25 03:29:10]

Description was changed from

==========
Size visual rect properly for scroll corner/resizer.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938
==========

to

==========
Size visual rect properly for scroll corner/resizer.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

BUG=529938
==========

[wkorman, 2016-03-25 03:30:32]

https://codereview.chromium.org/1830333002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp (right):

https://codereview.chromium.org/1830333002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp:43: {
On 2016/03/24 at 23:39:34, chrishtr wrote:
> ASSERT(m_scrollableArea)

Done.

[wkorman, 2016-03-25 03:31:32]

Description was changed from

==========
Size visual rect properly for scroll corner/resizer.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

BUG=529938
==========

to

==========
Correct visual rects for scroll corner/resizer and non-frame composited scroll
bars.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

BUG=529938
==========

[wkorman, 2016-03-25 03:31:53]

Description was changed from

==========
Correct visual rects for scroll corner/resizer and non-frame composited scroll
bars.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

BUG=529938
==========

to

==========
Correct visual rects for scroll corner/resizer and non-frame composited scroll
bars.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938
==========

[chrishtr, 2016-03-25 03:38:08]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-03-25 03:38:09]

lgtm

[commit-bot: I haz the power, 2016-03-25 03:38:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/40001

[commit-bot: I haz the power, 2016-03-25 04:12:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-25 04:12:55]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wkorman, 2016-03-25 04:15:35]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2016-03-25 04:15:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/40001

[commit-bot: I haz the power, 2016-03-25 04:29:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-25 04:29:56]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[wkorman, 2016-03-25 18:14:01]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2016-03-25 18:14:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1830333002/#ps60001
(title: "Use cached drawing for non-frame composited scrollbars when
available.")

[commit-bot: I haz the power, 2016-03-25 18:14:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/60001

[commit-bot: I haz the power, 2016-03-25 19:03:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-25 19:03:07]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wkorman, 2016-03-25 20:28:52]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2016-03-25 20:28:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1830333002/#ps80001
(title: "We have to mark the paint layer for repaint for composited scroll
controls.")

[commit-bot: I haz the power, 2016-03-25 20:29:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/80001

[commit-bot: I haz the power, 2016-03-25 21:38:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-25 21:38:38]

Try jobs failed on following builders:
  linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)

[wkorman, 2016-03-25 21:59:29]

Chris -- PTAL and make sure I'm not off in the weeds before I go through and add
NeedsRebaseline for the affected LayoutTests failing the latest patch due to new
repaint rects present in the text expectations.

Since you last looked at this:

- Patch 4: unit tests failing showed we were missing a paint layer invalidation:

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...
CompositedLayerMappingTest.InterestRectShouldNotChangeOnPaintInvalidation
CompositedLayerMappingTest.InterestRectChangeOnScroll

Fixed by modifying
PaintInvalidationCapableScrollableArea::invalidatePaintOfScrollbarIfNeeded() to
setNeedsPaintInvalidation on the PaintLayer for the scrollbar. I believe the
comment I removed there is now just out of date to the changes we are making.

- Patch 5: aforementioned text expectations now show an add'l repaint rect in
many tests:

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_oilp...

IMO this is not unexpected but I don't have full background on scrollbar related
optimizations. I worry in particular due to tests named like so:

compositing/repaint/clipping-should-not-repaint-composited-descendants.html

where it's clear we were explicitly ensuring we avoid unnecessary repaints and
this change now introduces... a repaint.

[wkorman, 2016-04-05 00:27:59]

The CQ bit was checked by wkorman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-05 00:28:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/100001

[commit-bot: I haz the power, 2016-04-05 02:02:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-05 02:02:29]

Dry run: Try jobs failed on following builders:
  linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)

[wkorman, 2016-04-05 18:00:17]

The CQ bit was checked by wkorman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-05 18:00:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/120001

[wkorman, 2016-04-05 19:03:04]

https://codereview.chromium.org/1830333002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1830333002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:2314:
return isScrollableAreaLayer(&graphicsLayer) ? true :
m_owningLayer.needsRepaint();
Since last review:

- updated this line. Is always-true reasonable? Tests pass and not clear to me
what other PaintLayer we might delegate to otherwise.
- removed not-appropriate change to PaintLayerScrollableArea.cpp where we would
end up calling box.enclosingLayer()->setNeedsRepaint() which was overkill

[wkorman, 2016-04-05 19:04:19]

https://codereview.chromium.org/1830333002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutScrollbar.cpp (right):

https://codereview.chromium.org/1830333002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutScrollbar.cpp:264: if
(!partLayoutObject && needLayoutObject && m_scrollableArea) {
Also added this check, which fixes otherwise-crashing:

fast/css/crash-corner-present.html

when test clicks and scrollbar is hidden.

[commit-bot: I haz the power, 2016-04-05 19:15:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-05 19:15:02]

Dry run: This issue passed the CQ dry run.

[wkorman, 2016-04-05 20:12:53]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2016-04-05 20:12:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1830333002/#ps120001
(title: "Don't create a LayoutScrollbartPart if we've no scrollable area.")

[commit-bot: I haz the power, 2016-04-05 20:13:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1830333002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1830333002/120001

[commit-bot: I haz the power, 2016-04-05 20:19:31]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-04-05 20:20:41]

Description was changed from

==========
Correct visual rects for scroll corner/resizer and non-frame composited scroll
bars.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938
==========

to

==========
Correct visual rects for scroll corner/resizer and non-frame composited scroll
bars.

Non-composited scroll corner visual rects are now determined by
their associated ScrollableArea, per updated logic in
LayoutScrollbarPart.

Change the DisplayItemClient for non-frame composited ScrollbarArea
painted content to be the GraphicsLayer to which it belongs.

BUG=529938

Committed: https://crrev.com/398e024db042ebd7d81b71dfd4e5abb8419fd5de
Cr-Commit-Position: refs/heads/master@{#385270}
==========

[commit-bot: I haz the power, 2016-04-05 20:20:43]

Patchset 7 (id:??) landed as
https://crrev.com/398e024db042ebd7d81b71dfd4e5abb8419fd5de
Cr-Commit-Position: refs/heads/master@{#385270}