Make ScrollbarThemeAura selectively invalidate scrollbar parts.

Make ScrollbarThemeAura selectively invalidate scrollbar parts.

In particular:
- when hovering or pressing the thumb, repaint only the thumb
- when thumb position changes, repaint the track only if it changes
  (e.g. due to a button being disabled upon reaching the end)

BUG=549277
Committed: https://crrev.com/0e8396ada46f085c23343450d850d81751053c94
Cr-Commit-Position: refs/heads/master@{#367082}

[jbroman, 2015-12-22 20:32:36]

jbroman@chromium.org changed reviewers:
+ ccameron@chromium.org, skobes@chromium.org

[jbroman, 2015-12-22 20:32:37]

This makes ScrollbarThemeAura tell the Scrollbar and ScrollableArea when parts
of the
scrollbar need to be repainted, and all that's happened is the thumb moving
(which
doesn't require repainting if the scrollbar is composited).

Tweaks are made to the Mac path to adjust for these changes. As a side effect,
it also
stops the repaint earlier in that case (the GraphicsLayer isn't invalidated at
all).

On both Aura and Mac, most scrolling does not trigger paint flashing in the
scrollbar.
(Exceptions are cases where the thumb or track must be repainted, of course.)

Unit tests included.

[jbroman, 2015-12-22 20:33:08]

Also: the rebaselines requested in TestExpectations are progressions in repaint
tests, where a composited scrollbar is no longer invalidated.

[ccameron, 2015-12-23 01:20:37]

Mac parts lgtm (though you can drop the TODO -- we don't plan "to do" much Mac
OS 10.6 optimization).

https://codereview.chromium.org/1525803002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm
(right):

https://codereview.chromium.org/1525803002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm:279: //
TODO(jbroman): Does this also apply to ScrollbarThemeMacNonOverlayAPI?
In principle it could be "just the thumb", but, in the interest of not breaking
things, just stick with the superclass's "all" behavior.

[jbroman, 2015-12-23 16:10:53]

On 2015/12/23 at 01:20:37, ccameron wrote:
> Mac parts lgtm (though you can drop the TODO -- we don't plan "to do" much Mac
OS 10.6 optimization).

Removed the TODO.

[skobes, 2015-12-28 17:48:39]

Have you tested custom scrollbars (which will still require invalidations)?

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/FrameView.cpp:2077:
scrollbar->setNeedsPaintInvalidation(static_cast<ScrollbarPart>(~ThumbPart));
Why do we exclude ThumbPart here?

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scroll/Scrollbar.h (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scroll/Scrollbar.h:101: // whether the thumb
is painted in a separate layer.
Technically the thumb is never a separate layer.  Perhaps this should read "the
scrollbar is unaware of whether its layer can move the thumb without
repainting"?

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp:53: struct
PartPaintingParams {
This could use a comment or two.

[jbroman, 2015-12-29 16:45:14]

On 2015/12/28 at 17:48:39, skobes wrote:
> Have you tested custom scrollbars (which will still require invalidations)?

Yes, they still work. They wind up using LayoutScrollbarTheme (rather than
ScrollbarThemeAura), which doesn't opt into this more efficient mode, and
instead continues to repaint the scrollbar each frame. Possibly someone could
find a set of cases where it could opt into the no-repainting path, but that's
future work if someone wants to pursue it.

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/FrameView.cpp:2077:
scrollbar->setNeedsPaintInvalidation(static_cast<ScrollbarPart>(~ThumbPart));
On 2015/12/28 at 17:48:39, skobes wrote:
> Why do we exclude ThumbPart here?

The previous version of the code invalidates the track, but not the thumb. (From
cc's perspective, the scrollbar consists of a track quad and a thumb quad.)
There are a variety of scrollbar parts that all end up in the track quad (and
it's not important to specify which of these draws the tickmarks), but I'd like
not to invalidate the thumb here, since the existing code doesn't.

If you'd rather also invalidate the thumb, I can do that.

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scroll/Scrollbar.h (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scroll/Scrollbar.h:101: // whether the thumb
is painted in a separate layer.
On 2015/12/28 at 17:48:39, skobes wrote:
> Technically the thumb is never a separate layer.  Perhaps this should read
"the scrollbar is unaware of whether its layer can move the thumb without
repainting"?

reworded to "...unaware of whether the thumb can be moved without repainting".

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp (right):

https://codereview.chromium.org/1525803002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scroll/ScrollbarThemeAura.cpp:53: struct
PartPaintingParams {
On 2015/12/28 at 17:48:39, skobes wrote:
> This could use a comment or two.

Done.

[skobes, 2015-12-29 16:47:02]

lgtm

[jbroman, 2015-12-29 16:51:35]

The CQ bit was checked by jbroman@chromium.org

[jbroman, 2015-12-29 16:51:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/1525803002/#ps180001
(title: "comments per skobes")

[commit-bot: I haz the power, 2015-12-29 16:51:52]

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

[commit-bot: I haz the power, 2015-12-29 18:16:10]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-12-29 18:17:05]

Description was changed from

==========
Make ScrollbarThemeAura selectively invalidate scrollbar parts.

In particular:
- when hovering or pressing the thumb, repaint only the thumb
- when thumb position changes, repaint the track only if it changes
  (e.g. due to a button being disabled upon reaching the end)

BUG=549277
==========

to

==========
Make ScrollbarThemeAura selectively invalidate scrollbar parts.

In particular:
- when hovering or pressing the thumb, repaint only the thumb
- when thumb position changes, repaint the track only if it changes
  (e.g. due to a button being disabled upon reaching the end)

BUG=549277

Committed: https://crrev.com/0e8396ada46f085c23343450d850d81751053c94
Cr-Commit-Position: refs/heads/master@{#367082}
==========

[commit-bot: I haz the power, 2015-12-29 18:17:06]

Patchset 10 (id:??) landed as
https://crrev.com/0e8396ada46f085c23343450d850d81751053c94
Cr-Commit-Position: refs/heads/master@{#367082}

[jbroman, 2015-12-29 20:00:42]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1550983002/ by jbroman@chromium.org.

The reason for reverting is: Oilpan unit test failure:
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac%20Oilpan/bu....