Fix for select PopupMenuList appearance in rtl mode with overlay-scrollbar enabled

Fix for select PopupMenuList appearance in rtl mode with overlay-scrollbar enabled.

The content should take total window width while overlay-scrollbar is enabled. 

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

[pals, 2014-06-12 14:12:31]

I am not sure how to test this change. There are unit tests for PopupMenuList
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

But this change needs pixel tests, this capability is missing I guess.

PTAL.

[leviw_travelin_and_unemployed, 2014-06-12 19:26:34]

I'm not a Source/web owner, but this seems reasonable.

Doing a quick log on the file doesn't show a clear owner, but I've CC-ed some
folks who can help.

[tkent, 2014-06-13 00:07:46]

We had better have tests.
We can't make layout tests for SELECT popups. C++ unittest is preferable.  Refer
to web/tests/PopupContainerTest.cpp and web/tests/PopupMenuTest.cpp.

[pals, 2014-06-13 01:49:05]

On 2014/06/13 00:07:46, tkent wrote:
> We had better have tests.
> We can't make layout tests for SELECT popups. C++ unittest is preferable. 
Refer
> to web/tests/PopupContainerTest.cpp and web/tests/PopupMenuTest.cpp.

But this change needs pixel tests, this capability is missing I guess.

[tkent, 2014-06-13 01:59:14]

On 2014/06/13 01:49:05, sanjoy.pal wrote:
> On 2014/06/13 00:07:46, tkent wrote:
> > We had better have tests.
> > We can't make layout tests for SELECT popups. C++ unittest is preferable. 
> Refer
> > to web/tests/PopupContainerTest.cpp and web/tests/PopupMenuTest.cpp.
> 
> But this change needs pixel tests, this capability is missing I guess.

Yes, we have no ways to make pixel tests for this.  So we should have unit
tests.

For example, you add new function for "shouldPlaceVerticalScrollbarOnLeft() &&
verticalScrollbar() && !verticalScrollbar()->isOverlayScrollbar())," and you can
write unit test for the function.

[pals, 2014-06-13 02:03:17]

On 2014/06/13 01:49:05, sanjoy.pal wrote:
> On 2014/06/13 00:07:46, tkent wrote:
> > We had better have tests.
> > We can't make layout tests for SELECT popups. C++ unittest is preferable. 
> Refer
> > to web/tests/PopupContainerTest.cpp and web/tests/PopupMenuTest.cpp.
> 
> But this change needs pixel tests, this capability is missing I guess.

I think we can write unittests. I shall update the patch with a test. Thanks.

[pals, 2014-06-16 09:02:50]

Added unit tests. Please take another look.

[tkent, 2014-06-16 23:31:28]

lgtm.  Thanks!

[tkent, 2014-06-16 23:31:31]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-06-16 23:31:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sanjoy.pal@samsung.com/333633003/40001

[commit-bot: I haz the power, 2014-06-17 01:00:24]

Change committed as 176261

[haraken, 2014-06-17 12:48:38]

On 2014/06/17 01:00:24, I haz the power (commit-bot) wrote:
> Change committed as 176261

Reverted this CL in r176309 since it broken webkit_unit_tests in Android bots:
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/14135