Delete joystick scroll/zoom support.

Delete joystick scroll/zoom support.

This task-posting gamepad scrolling and pinching support has proven
impossible to extend or override by either WebView embedders or
Javascript, so we unfortunately need to go back to the drawing board on
it.  Remove the behavior and report joystick stick/shoulder events as
unhandled, to allow OEM frameworks to create their own behavior instead
(which could be the same behavior that it's doing now, or something
different).

BUG=454355, 624930

[aelias_OOO_until_Jul13, 2016-08-19 01:03:38]

The CQ bit was checked by aelias@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 01:04:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[aelias_OOO_until_Jul13, 2016-08-19 01:06:53]

aelias@chromium.org changed reviewers:
+ tdresser@chromium.org, tedchoc@chromium.org

[aelias_OOO_until_Jul13, 2016-08-19 01:06:59]

PTAL.

[commit-bot: I haz the power, 2016-08-19 02:03:59]

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

[commit-bot: I haz the power, 2016-08-19 02:04:00]

Dry run: This issue passed the CQ dry run.

[aelias_OOO_until_Jul13, 2016-08-19 21:23:17]

Hi Nvidia folks, I didn't hear from you about the proposal to delete one way or
the other, so I'm planning to move forward with it.

(Needs lgt'm from tedchoc@ and tdresser@ to land.)

[davebytes, 2016-08-20 02:49:58]

On 2016/08/19 21:23:17, aelias wrote:
> Hi Nvidia folks, I didn't hear from you about the proposal to delete one way
or
> the other, so I'm planning to move forward with it.
> 
> (Needs lgt'm from tedchoc@ and tdresser@ to land.)

Can you hold on this?

I unfortunately signed in through personal Gmail not work, i don't watch it
closely and thus have not seen further thread on this. More important though,
I'm not a member of the browser team which worked hard to get this integration
in the first place, they should be involved in any such decision (and lack of
response is likely due to many key people not even being on the bug, at least
that's my guess...)

My last recollection is that I had reviewed code again (now a few weeks back?)
and it looked like maybe I was mistaken and overrode the wrong view method. 
Have been focused on other projects, would want to go back and look at this next
week, and have someone from browser team jump in and comment here or the
original thread.

Thanks, and apologies for confusion here.

[aelias_OOO_until_Jul13, 2016-08-20 02:59:39]

OK, thanks for responding.  Well, sshelke@nvidia.com who wrote all of this has
been cc'ed on all the relevant bugs as well as this patch, and I haven't heard
from him at all recently.

If you're worried this proposed deletion is purely because of your bug report,
it's not.  The lack of portability to non-Android Chrome platforms, the lack of
JS overridability, the difference from how every other kind of input event is
handled, and the fact that its tests have been flaky, I'd been increasingly
thinking it probably was a mistake to allow in this code.  Particularly since it
actually can be implemented on the framework side and doesn't really require
Chromium to do anything special.

Anyway, I'm not in a huge hurry so I can wait another week for sshelke@ or
someone else on his team to comment.

[sshelke, 2016-08-22 11:08:43]

> sshelke@nvidia.com who wrote all of this has
> been cc'ed on all the relevant bugs as well as this patch, and I haven't heard
> from him at all recently.
Sorry for delayed response, I was on leave for almost 2 weeks and could not get
chance to go through bug list. I should have mentioned ooto in my profile name.


> If you're worried this proposed deletion is purely because of your bug report,
> it's not.  The lack of portability to non-Android Chrome platforms, the lack
of
> JS overridability, the difference from how every other kind of input event is
> handled, 
When I started discussion with Jared regarding implementation of this feature,
we wanted to implement for all platforms but due to some issues and we wanted 
to try and experiment on android first, so we moved ahead and 
implemented on android.Please refer,
https://bugs.chromium.org/p/chromium/issues/detail?id=454355#c12

+jdduke@chromium.org

>> and the fact that its tests have been flaky, 
I would not recommend to delete feature just because test is flaky. 
We can work towards removing flakiness from test.

>> I'd been increasingly thinking it probably was a mistake to allow in this
code.  
If you think its really great mistake to allow this feature in 
chromium repositorty.It would be helpful if you provide me list 
of issue these features have contributed to and I will work towards 
solving those issues.

> Particularly since it actually can be implemented on the framework side 
> and doesn't really require Chromium to do anything special.
If you have alternate way of achieving same thing, please let me know I would be

happy to think and work in that direction.

Regarding issue raised by @davebytes, we have already discussed about 
solution and we are trying to resolve issue at our end, 
no action as such required from chromium developers.

Even if we decide to remove smooth scrolling and LS should fall back 
to default operation i.e LS to DPAD mapping (which is android specific).
I just want to know whether there is any standard specification defined 
which puts restriction on LS to be used for navigation only,if yes, 
then I would not mind removing this feature. 

Lastly, its up to chromium developers and reviewers to decide whether
we want DPAD buttons and Left joystick(LS) to perform same operation 
i.e. navigation (LS becomes redundant)or we want to utilize LS to add new 
feature i.e. smooth scrolling in this case.Thanks!!

[aelias_OOO_until_Jul13, 2016-08-22 21:27:54]

On 2016/08/22 at 11:08:43, sshelke wrote:
> > If you're worried this proposed deletion is purely because of your bug
report,
> > it's not.  The lack of portability to non-Android Chrome platforms, the lack
of
> > JS overridability, the difference from how every other kind of input event
is
> > handled, 
> When I started discussion with Jared regarding implementation of this feature,
> we wanted to implement for all platforms but due to some issues and we wanted 
> to try and experiment on android first, so we moved ahead and 
> implemented on android.Please refer,
> https://bugs.chromium.org/p/chromium/issues/detail?id=454355#c12
> 
> +jdduke@chromium.org

Jared is no longer on Chromium-team.  Anyway, this leaves the feature on
a provisional partially-supported basis and we typically aim not to have
features in this limbo.

> 
> >> and the fact that its tests have been flaky, 
> I would not recommend to delete feature just because test is flaky. 
> We can work towards removing flakiness from test.

Well, you haven't made any move to address past bugs about test flakiness you
were cc'ed on like http://crbug.com/605944, http://crbug.com/538781, and
http://crbug.com/635812.  Ultimately the burden of deflaking tests always
falls on project owners.

> 
> >> I'd been increasingly thinking it probably was a mistake to allow in this
code.  
> If you think its really great mistake to allow this feature in 
> chromium repositorty.It would be helpful if you provide me list 
> of issue these features have contributed to and I will work towards 
> solving those issues.
> 
> > Particularly since it actually can be implemented on the framework side 
> > and doesn't really require Chromium to do anything special.
> If you have alternate way of achieving same thing, please let me know I would
be 
> happy to think and work in that direction.

I sent you an email with the suggestion last week, posting again here:
"Here's my suggestion for sshelke@.  I presume he is also part of a team with
control
over the Android framework itself.  Chromium will commit to reporting gamepad
events
as unhandled, and sshelke@'s desired behavior can be written as part of the
Android
framework as follows: generate synthetic touchpad scroll and pinch events from
them
and send them to Chrome.  Note that I do mean touch*pad* not touchscreen:
touchpads
are a better match for gamepad inputs because they are positionless and will
affect
whatever element is currently focused."

I think such framework-side approach would be beneficial in that left-stick
could
potentially scroll on *all* Android apps, not just Chromium.  But apps could
also mark
joystick events as handled and do something different if needed.

I just ordered an NVidia Shield portable to see what default behavior it
already has today for unhandled joystick events.

> Even if we decide to remove smooth scrolling and LS should fall back 
> to default operation i.e LS to DPAD mapping (which is android specific).
> I just want to know whether there is any standard specification defined 
> which puts restriction on LS to be used for navigation only,if yes, 
> then I would not mind removing this feature. 

There's no specification about it, and that's one of the problems.  We were
quick to deploy a particular behavior on one single Chrome platform, but we're
not really sure yet if this will be the best behavior in the long term, nor
is any way specified for Javascript to override it (which in general we
want to provide for all input events).  Until these design questions are
resolved we should probably just be neutral.

[sshelke, 2016-08-23 06:24:03]

>> Well, you haven't made any move to address past bugs about test flakiness you
>> were cc'ed on like http://crbug.com/605944, http://crbug.com/538781, and
>> http://crbug.com/635812.  Ultimately the burden of deflaking tests always
>> falls on project owners.
I am really sorry for not updating bugs in past regarding test flakiness. But 
when I check testJoystickScroll on NVIDIA platforms, it gets passed.
So I don't find a way to resolve the issue. Next time, I will verify on other 
platforms and try to fix it ASAP.


>> I just ordered an NVidia Shield portable to see what default behavior it
>> already has today for unhandled joystick events.
I am not part Android framework team but I have done some analysis on unhandled
joystick events.
If false is return from onGenericMotion handler, through chain of views it is
landed to ViewRootImpl where logic it written to convert LS to DPAD.
Please refer 
https://chromium.googlesource.com/android_tools/+/18728e9dd5dd66d4f5edf1b792e...
So Android framework is already generating synthetic DPAD events for unhandled
LS events.

>> Until these design questions are resolved we should probably just be neutral.
We have added these features based on requirements where devices might not
have touch screen or key board but have gamepad attached to it. For ex,
NVIDIA Shield TV, NVIDIA shield or any gaming device.On these devices user 
should be able to browse web pages (either using browser or app based on android
web view)
using gamepad buttons/axes. Since DPAD was already mapped for navigation, 
so I thought smooth scrolling through LS might be useful.

[aelias_OOO_until_Jul13, 2016-08-23 19:26:00]

On 2016/08/23 at 06:24:03, sshelke wrote:
> >> Until these design questions are resolved we should probably just be
neutral.
> We have added these features based on requirements where devices might not
> have touch screen or key board but have gamepad attached to it. For ex,
> NVIDIA Shield TV, NVIDIA shield or any gaming device.On these devices user 
> should be able to browse web pages (either using browser or app based on
android web view)
> using gamepad buttons/axes. Since DPAD was already mapped for navigation, 
> so I thought smooth scrolling through LS might be useful.

Wouldn't this reasoning wouldn't apply to all Android apps, not just
Chrome/WebView?
That makes the web browser inconsistent with the rest of the system.  In
general we aim to behave like any other scrollable Android View as much as
possible.
Your desired behavior makes sense, but I'd still argue it shouldn't be
implemented in
Chromium but just be in the framework.

[sshelke, 2016-08-24 06:49:31]

Users generally prefer browser or webview based apps for browsing, so
browser is perfect use case for these features.
AFAIK, LS to DPAD mapping is fallback operation which Android framework 
supports, so Android framework allows application (browser in our case) 
first to intercept and implement own customized implementation.So,
I guess we are not breaking any functionality here.

>> That makes the web browser inconsistent with the rest of the system
I don't think browser is inconsistent since all app developers have 
their own implementation for LS.

Again, I understand that browser is intercepting LS and android framework
is not able to map it to DPAD, so everyone thinks we are breaking or
regressing previous functionality. I am still not sure why we are removing
zooming feature? Is there any fallback provided for LT/RT by android 
framework? Is zooming feature regressing any previous functionality?

[sshelke, 2016-08-24 08:57:45]

One more thing, I have added testJoystickScroll test only, inside 
ContentViewScrollingTest.java. But you trying to delete entire scrolling 
test class.Please check!!

[aelias_OOO_until_Jul13, 2016-09-06 22:38:41]

OK, we had some more internal discussion about it (note: we'll aim to make all
of the discussion public in the future-- I realize my communication pattern may
have appeared very intermittent because some of it was unnecessarily going on
privately) and I think we have a long-term plan for how this behavior can
probably be maintained without undue burden: I just filed
http://crbug.com/644488 about it.  Furthermore, I checked the tests are
currently not flaky anymore.  So there is no very strong reason to push for
immediate deletion of the behavior, and we'll put it on our backlog for an
eventual rewrite instead.

I'm closing this patch without landing for now, thanks for the feedback.