Remove scroll_delta flooring hack

Remove scroll_delta flooring hack

Now that fractional scroll offset can pass through cc main
thread and Blink, we can remove the scroll_delta flooring hack
and send the full fractional scroll_delta from impl to main.

Note that this will break the old pinch viewport code path (
which is to be removed soon) since it still only expects integer
scroll offset.

BUG=414283
Committed: https://crrev.com/7a2217569bc1a7596ad6671513249e241b3491c3
Cr-Commit-Position: refs/heads/master@{#314626}

[Yufeng Shen (Slow to review), 2015-01-28 19:38:06]

miletus@chromium.org changed reviewers:
+ aelias@chromium.org, bokan@chromium.org

[aelias_OOO_until_Jul13, 2015-01-28 19:53:34]

Looks OK per se, just not very useful to land on its own.  How about making this
patch complete and working with the assumption that the Blink side will land
first?

[Yufeng Shen (Slow to review), 2015-01-28 22:53:07]

On 2015/01/28 19:53:34, aelias wrote:
> Looks OK per se, just not very useful to land on its own.  How about making
this
> patch complete and working with the assumption that the Blink side will land
> first?

hmmm, what do you mean by "complete" ? This CL is meant to be just mechanical
change for preparation of we will send fully fractional scroll_delta from impl
to main. Do you mean land this together with turning on sending full fractional
scroll delta from impl to main ?

[bokan, 2015-01-28 22:54:52]

You'll to change a few more descendants that wont get built by default (so you
wouldn't see it locally):

src/mojo/services/html_viewer/weblayertreeview_impl.cc
src/android_webview/browser/hardware_renderer.h

possibly others.

[aelias_OOO_until_Jul13, 2015-01-28 22:57:54]

On 2015/01/28 at 22:53:07, miletus wrote:
> On 2015/01/28 19:53:34, aelias wrote:
> > Looks OK per se, just not very useful to land on its own.  How about making
this
> > patch complete and working with the assumption that the Blink side will land
> > first?
> 
> hmmm, what do you mean by "complete" ? This CL is meant to be just mechanical
> change for preparation of we will send fully fractional scroll_delta from impl
> to main. Do you mean land this together with turning on sending full
fractional
> scroll delta from impl to main ?

Right.  Note that personally I typically prefer to review larger patches, it
makes it easier to reason about the whole picture.

[Yufeng Shen (Slow to review), 2015-02-03 22:21:52]

On 2015/01/28 22:57:54, aelias wrote:
> On 2015/01/28 at 22:53:07, miletus wrote:
> > On 2015/01/28 19:53:34, aelias wrote:
> > > Looks OK per se, just not very useful to land on its own.  How about
making
> this
> > > patch complete and working with the assumption that the Blink side will
land
> > > first?
> > 
> > hmmm, what do you mean by "complete" ? This CL is meant to be just
mechanical
> > change for preparation of we will send fully fractional scroll_delta from
impl
> > to main. Do you mean land this together with turning on sending full
> fractional
> > scroll delta from impl to main ?
> 
> Right.  Note that personally I typically prefer to review larger patches, it
> makes it easier to reason about the whole picture.

Alright, the new patchset removes the scroll_delta flooring hack. PTAL, thanks.

[aelias_OOO_until_Jul13, 2015-02-03 22:24:34]

lgtm

[Yufeng Shen (Slow to review), 2015-02-04 00:37:34]

New patchsets have been uploaded after l-g-t-m from aelias@chromium.org

[Yufeng Shen (Slow to review), 2015-02-04 01:21:36]

miletus@chromium.org changed reviewers:
+ boliu@chromium.org, danakj@chromium.org, jamesr@chromium.org,
sievers@chromium.org

[Yufeng Shen (Slow to review), 2015-02-04 01:23:25]

Add OWNERS:

boliu@    for  android_webview/*
sievers@  for  content/*
danakj@   for  ui/compositor/*
jamesr@   for  mojo/*

[jamesr, 2015-02-04 01:25:08]

mojo/services/html_viewer/ lgtm

[danakj, 2015-02-04 01:32:22]

ui/compositor LGTM

[boliu, 2015-02-04 02:23:55]

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:383:
final int maxScrollXPix = (int) Math.floor(maxScrollXCss * deviceDIPScale);
assertScrollInJs above needs to do a floor (ie cast to int) is because of the
floor here. testUiScrollReflectedInJs (which also calls assertScrollInJs)
doesn't need that behavior.

You can change assertScrollInJs to take doubles, and then skip the floor here.
That should work I think?

[Yufeng Shen (Slow to review), 2015-02-04 02:39:21]

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:383:
final int maxScrollXPix = (int) Math.floor(maxScrollXCss * deviceDIPScale);
On 2015/02/04 02:23:55, boliu wrote:
> assertScrollInJs above needs to do a floor (ie cast to int) is because of the
> floor here. testUiScrollReflectedInJs (which also calls assertScrollInJs)
> doesn't need that behavior.
> 
> You can change assertScrollInJs to take doubles, and then skip the floor here.
> That should work I think?

yeah, my concern is that here you get a native double value and in
assertScrollInJs you get a string represented double generated from JS, do you
prefer to compare those ?

[boliu, 2015-02-04 02:55:34]

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:383:
final int maxScrollXPix = (int) Math.floor(maxScrollXCss * deviceDIPScale);
On 2015/02/04 02:39:21, Yufeng Shen wrote:
> On 2015/02/04 02:23:55, boliu wrote:
> > assertScrollInJs above needs to do a floor (ie cast to int) is because of
the
> > floor here. testUiScrollReflectedInJs (which also calls assertScrollInJs)
> > doesn't need that behavior.
> > 
> > You can change assertScrollInJs to take doubles, and then skip the floor
here.
> > That should work I think?
> 
> yeah, my concern is that here you get a native double value and in
> assertScrollInJs you get a string represented double generated from JS, do you
> prefer to compare those ?

I was thinking parse to double, then abs(a-b) < epsilon

[Yufeng Shen (Slow to review), 2015-02-04 03:21:03]

New patchsets have been uploaded after l-g-t-m from
jamesr@chromium.org,danakj@chromium.org

[Yufeng Shen (Slow to review), 2015-02-04 03:22:30]

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/870363005/diff/60001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:383:
final int maxScrollXPix = (int) Math.floor(maxScrollXCss * deviceDIPScale);
On 2015/02/04 02:55:34, boliu wrote:
> On 2015/02/04 02:39:21, Yufeng Shen wrote:
> > On 2015/02/04 02:23:55, boliu wrote:
> > > assertScrollInJs above needs to do a floor (ie cast to int) is because of
> the
> > > floor here. testUiScrollReflectedInJs (which also calls assertScrollInJs)
> > > doesn't need that behavior.
> > > 
> > > You can change assertScrollInJs to take doubles, and then skip the floor
> here.
> > > That should work I think?
> > 
> > yeah, my concern is that here you get a native double value and in
> > assertScrollInJs you get a string represented double generated from JS, do
you
> > prefer to compare those ?
> 
> I was thinking parse to double, then abs(a-b) < epsilon

Done.

[boliu, 2015-02-04 03:33:45]

Thanks. android_webview lgtm

[Yufeng Shen (Slow to review), 2015-02-04 19:28:01]

New patchsets have been uploaded after l-g-t-m from boliu@chromium.org

[no sievers, 2015-02-04 19:53:41]

content lgtm

[Yufeng Shen (Slow to review), 2015-02-04 20:25:08]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2015-02-04 20:26:05]

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

[commit-bot: I haz the power, 2015-02-04 20:45:19]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-04 20:46:56]

Patchset 6 (id:??) landed as
https://crrev.com/7a2217569bc1a7596ad6671513249e241b3491c3
Cr-Commit-Position: refs/heads/master@{#314626}