Add ability to create solid color scrollbars in ScrollingCoordinator.

Add ability to create solid color scrollbars in ScrollingCoordinator.

This CL adds the ability to create solid color scrollbars within ScrollingCoordinator,
based on the current value of the settings useOverlayScrollbars and useSolidColorScrollbars.
This functionality is made available in the class interface to facilitate
access from WebViewImpl in subsequent pinch-zoom CLs.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157254

[wjmaclean, 2013-09-03 22:57:10]

Let me know if this approach looks as the next step in the scrollbar refactor
work.

[jamesr, 2013-09-03 23:06:19]

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
Source/core/page/scrolling/ScrollingCoordinator.cpp:284: bool
shouldCreateSolidColorScrollbar = settings->useSolidColorScrollbars() &&
settings->usesOverlayScrollbars();
why are you checking usesSolidColor... *and* usesOverlay?  When would
usesSolidColor..() be true but not usesOverlay..()?

[enne (OOO), 2013-09-03 23:47:33]

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
Source/core/page/scrolling/ScrollingCoordinator.cpp:284: bool
shouldCreateSolidColorScrollbar = settings->useSolidColorScrollbars() &&
settings->usesOverlayScrollbars();
On 2013/09/03 23:06:19, jamesr wrote:
> why are you checking usesSolidColor... *and* usesOverlay?  When would
> usesSolidColor..() be true but not usesOverlay..()?

My thought was that Android would set both of these flags.  Are you suggesting
you'd prefer just checking solid color (and ASSERTing overlay, maybe)?

[wjmaclean, 2013-09-04 13:27:59]

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
Source/core/page/scrolling/ScrollingCoordinator.cpp:284: bool
shouldCreateSolidColorScrollbar = settings->useSolidColorScrollbars() &&
settings->usesOverlayScrollbars();
On 2013/09/03 23:47:33, enne wrote:
> On 2013/09/03 23:06:19, jamesr wrote:
> > why are you checking usesSolidColor... *and* usesOverlay?  When would
> > usesSolidColor..() be true but not usesOverlay..()?
> 
> My thought was that Android would set both of these flags.  Are you suggesting
> you'd prefer just checking solid color (and ASSERTing overlay, maybe)?

I can confirm that Android does currently set both of these flags. That being
said, it's also reasonable that if the solid color scrollbar setting is true,
then overlay scrollbars should also be true, since the solid color scrollbars
don't do any drawing outside the thumb. So, we could check for
useSolidColorScrollbars() and assert usesOverlayScrollbars().

Let me know which is preferred.

[enne (OOO), 2013-09-04 17:51:40]

Please give jamesr a chance to chime in if he still objects, but if Android
currently sets both then this lgtm.

[jamesr, 2013-09-04 20:41:16]

Sorry, either way lgtm.  I think I have a slight preference for enforcing that
the settings are consistent with an ASSERT() just to try to limit the number of
settings we support.

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):

https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
Source/core/page/scrolling/ScrollingCoordinator.cpp:284: bool
shouldCreateSolidColorScrollbar = settings->useSolidColorScrollbars() &&
settings->usesOverlayScrollbars();
On 2013/09/04 13:27:59, wjmaclean wrote:
> On 2013/09/03 23:47:33, enne wrote:
> > On 2013/09/03 23:06:19, jamesr wrote:
> > > why are you checking usesSolidColor... *and* usesOverlay?  When would
> > > usesSolidColor..() be true but not usesOverlay..()?
> > 
> > My thought was that Android would set both of these flags.  Are you
suggesting
> > you'd prefer just checking solid color (and ASSERTing overlay, maybe)?
> 
> I can confirm that Android does currently set both of these flags. That being
> said, it's also reasonable that if the solid color scrollbar setting is true,
> then overlay scrollbars should also be true, since the solid color scrollbars
> don't do any drawing outside the thumb. So, we could check for
> useSolidColorScrollbars() and assert usesOverlayScrollbars().
> 
> Let me know which is preferred.

I think either would be fine - either say that one setting requires the other
and ASSERT() or only check the useSolidColorScrollbars() setting and assume the
embedder has set up the settings in a reasonable way.  Let's just pick one.

[commit-bot: I haz the power, 2013-09-04 20:50:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wjmaclean@chromium.org/23620023/12001

[wjmaclean, 2013-09-04 20:50:27]

On 2013/09/04 20:41:16, jamesr wrote:
> Sorry, either way lgtm.  I think I have a slight preference for enforcing that
> the settings are consistent with an ASSERT() just to try to limit the number
of
> settings we support.
> 
>
https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
> File Source/core/page/scrolling/ScrollingCoordinator.cpp (right):
> 
>
https://codereview.chromium.org/23620023/diff/1/Source/core/page/scrolling/Sc...
> Source/core/page/scrolling/ScrollingCoordinator.cpp:284: bool
> shouldCreateSolidColorScrollbar = settings->useSolidColorScrollbars() &&
> settings->usesOverlayScrollbars();
> On 2013/09/04 13:27:59, wjmaclean wrote:
> > On 2013/09/03 23:47:33, enne wrote:
> > > On 2013/09/03 23:06:19, jamesr wrote:
> > > > why are you checking usesSolidColor... *and* usesOverlay?  When would
> > > > usesSolidColor..() be true but not usesOverlay..()?
> > > 
> > > My thought was that Android would set both of these flags.  Are you
> suggesting
> > > you'd prefer just checking solid color (and ASSERTing overlay, maybe)?
> > 
> > I can confirm that Android does currently set both of these flags. That
being
> > said, it's also reasonable that if the solid color scrollbar setting is
true,
> > then overlay scrollbars should also be true, since the solid color
scrollbars
> > don't do any drawing outside the thumb. So, we could check for
> > useSolidColorScrollbars() and assert usesOverlayScrollbars().
> > 
> > Let me know which is preferred.
> 
> I think either would be fine - either say that one setting requires the other
> and ASSERT() or only check the useSolidColorScrollbars() setting and assume
the
> embedder has set up the settings in a reasonable way.  Let's just pick one.

Done, thanks!

[commit-bot: I haz the power, 2013-09-05 01:19:43]

Change committed as 157254