Add ScrollDirectionPhysical enum in Scroll types.

Add ScrollDirectionPhysical enum in Scroll types.

Currently ScrollDirection enum contains both physical directions and logical
directions to remove code duplication. Some scrolling methods can take only
physical directions. In these method, caller had to check the style for writing
mode and convert a logical scroll to the physical direction.

This CL eleminate potential error by using ScrollDirectionPhysical enum which
contains physical directions only.

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

[hyup, 2015-05-21 13:03:03]

sh53.lee@samsung.com changed reviewers:
+ bokan@chromium.org

[hyup, 2015-05-21 13:09:05]

PTAL

[bokan, 2015-05-21 18:54:43]

Looks good overall, just suggesting some some improvements.

https://codereview.chromium.org/1147283002/diff/1/Source/core/frame/FrameView...
File Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1147283002/diff/1/Source/core/frame/FrameView...
Source/core/frame/FrameView.cpp:3266: bool
FrameView::scroll(ScrollDirectionPhysical direction, ScrollGranularity
granularity, float delta)
Just remove FrameView::scroll and let callers use the base class version
directly.

https://codereview.chromium.org/1147283002/diff/1/Source/platform/scroll/Scro...
File Source/platform/scroll/ScrollTypes.h (right):

https://codereview.chromium.org/1147283002/diff/1/Source/platform/scroll/Scro...
Source/platform/scroll/ScrollTypes.h:46: ScrollPhysicalUp,
I'd make these ones ScrollUp, ScrollLeft, etc and the physical directions in the
above enum ScrollUpIgnoringWritingMode, ScrollLeftIgnoringWritingMode, etc.

https://codereview.chromium.org/1147283002/diff/1/Source/web/WebPluginScrollb...
File Source/web/WebPluginScrollbarImpl.cpp (right):

https://codereview.chromium.org/1147283002/diff/1/Source/web/WebPluginScrollb...
Source/web/WebPluginScrollbarImpl.cpp:361: return
m_group->scroll(toPhysicalDirection(scrollDirection, true, true),
scrollGranularity);
I'd rather make WebViewImpl::mapKeyCodeForScroll use a ScrollDirectionPhysical.
You'll have to also update WebViewImpl::scrollViewWithKeyboard  which is the
other caller of mapKeyCodeForScroll. That method calls
EventHandler::bubblingScroll which takes ScrollDirection so I'd add a
toScrollDirection function that converts a PhysicalScrollDirection to The
ScrollLeft/Right/UpDown enums of ScrollDirection.

[hyup, 2015-05-22 02:03:57]

Bokan, Thanks for your nice review. I updated CL. PTAL

[bokan, 2015-05-22 02:15:01]

bokan@chromium.org changed reviewers:
+ vollick@chromium.org

[bokan, 2015-05-22 02:15:02]

lgtm, thanks for the clean up!

+vollick@ for Source/platform, Source/core/(layout|editing|page)

[Ian Vollick, 2015-05-22 02:58:57]

On 2015/05/22 at 02:15:02, bokan wrote:
> lgtm, thanks for the clean up!
> 
> +vollick@ for Source/platform, Source/core/(layout|editing|page)

lgtm

[hyup, 2015-05-22 04:07:42]

The CQ bit was checked by sh53.lee@samsung.com

[commit-bot: I haz the power, 2015-05-22 04:07:50]

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

[commit-bot: I haz the power, 2015-05-22 04:12:23]

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

[commit-bot: I haz the power, 2015-05-22 04:12:24]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/56094)

[hyup, 2015-05-22 08:23:25]

bogan, rollick, Thanks for your review. I fixed a confilct error. Should I get
review again?

[bokan, 2015-05-22 14:26:20]

On 2015/05/22 08:23:25, hyup wrote:
> bogan, rollick, Thanks for your review. I fixed a confilct error. Should I get
> review again?

No. In general, you'll often need to rebase before committing. Unless there are
significant conflicts during the rebase that clash with your changes, you don't
need re-review.

[hyup, 2015-05-22 15:02:35]

The CQ bit was checked by sh53.lee@samsung.com

[hyup, 2015-05-22 15:02:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
vollick@chromium.org, bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/1147283002/#ps60001
(title: "Fixed a conflict on EventHandler")

[commit-bot: I haz the power, 2015-05-22 15:02:57]

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

[commit-bot: I haz the power, 2015-05-22 16:45:59]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195796