Fix RTL scroll bars being misrendered on Android.

Fix RTL scroll bars being misrendered on Android.

In https://chromiumcodereview.appspot.com/23734003 , I tried to
include information about whether to use a left side scrollbar by
adding a textDirection method to WebScrollbar.  It missed some cases
when there is no horizontal scrolling.

I also renamed to textDirection() and isRightToLeft() to
shouldPlaceVerticalScrollbarOnLeft() because writing mode direction can
also cause the scrollbar to be on the left.

Fix the bug in ScrollbarThemeOverlay::paintThumb to take the left side
scrollbars into account.

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

[jamesr, 2013-09-05 23:53:05]

It feels weird to ask a Scrollbar if it should place a vertical scrollbar on the
left.  Scrollbars don't place themselves, they're placed by someone else (the
ScrollView or RenderLayer).  I think it'd make more sense to have the function
that asks where a scrollbar should be placed on ScrollableArea and have that
plumb a bit to the scrollbar telling it where it went.

[tony, 2013-09-06 02:06:00]

On 2013/09/05 23:53:05, jamesr wrote:
> It feels weird to ask a Scrollbar if it should place a vertical scrollbar on
the
> left.  Scrollbars don't place themselves, they're placed by someone else (the
> ScrollView or RenderLayer).

Maybe the function name is causing the confusion.  How about:
isLeftSideVerticalScrollbar() instead?

> I think it'd make more sense to have the function
> that asks where a scrollbar should be placed on ScrollableArea and have that
> plumb a bit to the scrollbar telling it where it went.

I could do that, but then we're using an extra unnecessary byte on every
scrollbar.  As long as the function name on Scrollbar is clear, why does it
matter whether the implementation is a member variable or asks ScrollableArea
each time?

If you think it's better to use the memory, I could have Scrollbar::create() ask
the ScrollableArea if it's a left side scrollbar and set a member variable on
Scrollbar at that time.

[tony, 2013-09-06 19:34:58]

ping, should I rename the function, add a member variable to scrollbar or some
combination of the two?

[jamesr, 2013-09-06 22:15:10]

Sorry about the delay.  I care more about the name of the function than storage
- having a Scrollbar::isLeftSideVerticalScrollbar() implemented by asking its
ScrollableArea sounds fine.

[tony, 2013-09-06 22:35:25]

On 2013/09/06 22:15:10, jamesr wrote:
> Sorry about the delay.  I care more about the name of the function than
storage
> - having a Scrollbar::isLeftSideVerticalScrollbar() implemented by asking its
> ScrollableArea sounds fine.

I renamed to Scrollbar::isLeftSideVerticalScrollbar() and left
ScrollableArea::shouldPlaceVerticalScrollbarOnLeft() since that seems like a
question for the scrollable area (ScrollView or RenderLayer).

[jamesr, 2013-09-06 22:46:56]

lgtm

https://codereview.chromium.org/23480037/diff/11001/Source/web/ScrollbarGroup...
File Source/web/ScrollbarGroup.cpp (right):

https://codereview.chromium.org/23480037/diff/11001/Source/web/ScrollbarGroup...
Source/web/ScrollbarGroup.cpp:270: return false;
interesting, i guess we never RTL-flip scrollbars in the pdf plugin

[commit-bot: I haz the power, 2013-09-06 23:12:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tony@chromium.org/23480037/11001

[commit-bot: I haz the power, 2013-09-07 00:18:35]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2013-09-07 00:54:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tony@chromium.org/23480037/11001

[commit-bot: I haz the power, 2013-09-07 03:25:01]

Change committed as 157406