Added checks in TouchDevice.java to avoid NPEs.

Added checks in TouchDevice.java to avoid NPEs.

It is possible that an input device has been removed from the system
right after grabbing the device ids using InputDevice.getDeviceIds().
This CL adds checks in TouchDevice.java to avoid NPEs in such cases.

BUG=480619
Committed: https://crrev.com/6d91546a0756a6b2171ad146a0e02ee28fce1975
Cr-Commit-Position: refs/heads/master@{#326837}

[mustaq, 2015-04-24 16:14:31]

mustaq@chromium.org changed reviewers:
+ jdduke@chromium.org

[mustaq, 2015-04-24 16:14:32]

ptal.

[jdduke (slow), 2015-04-24 16:17:32]

lgtm with nits.

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
File ui/android/java/src/org/chromium/ui/base/TouchDevice.java (right):

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/base/TouchDevice.java:57: int
pointerTypesVal = 0;
Why not initialize this to PointerType.NONE, and get rid of the (if
pointerTypesVal == 0) cleanup at the end? Same below in availableHoverTypes.

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/base/TouchDevice.java:61: if (inputDevice ==
null)
Nit: For Java we either require braces for conditionals with a line break, or if
it's a single statement that fits on the conditional line, just move it there
(without braces), so this becomes:

if (inputDevice == null) continue;

Same below.

[mustaq, 2015-04-24 16:38:49]

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
File ui/android/java/src/org/chromium/ui/base/TouchDevice.java (right):

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/base/TouchDevice.java:57: int
pointerTypesVal = 0;
On 2015/04/24 16:17:32, jdduke wrote:
> Why not initialize this to PointerType.NONE, and get rid of the (if
> pointerTypesVal == 0) cleanup at the end? Same below in availableHoverTypes.

Unfortunately this intuitive suggestion won't work here because the value of
NONE is 1, in both repos, for a historical mistake in spec interpretation. I had
planned for the cleanup, but didn't want to face commits across chromium-blink
boundary for this simple thing :(

https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
ui/android/java/src/org/chromium/ui/base/TouchDevice.java:61: if (inputDevice ==
null)
Ooops, forgot the rule.

[jdduke (slow), 2015-04-24 16:42:30]

On 2015/04/24 16:38:49, mustaq wrote:
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> File ui/android/java/src/org/chromium/ui/base/TouchDevice.java (right):
> 
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> ui/android/java/src/org/chromium/ui/base/TouchDevice.java:57: int
> pointerTypesVal = 0;
> On 2015/04/24 16:17:32, jdduke wrote:
> > Why not initialize this to PointerType.NONE, and get rid of the (if
> > pointerTypesVal == 0) cleanup at the end? Same below in availableHoverTypes.
> 
> Unfortunately this intuitive suggestion won't work here because the value of
> NONE is 1, in both repos, for a historical mistake in spec interpretation. I
had
> planned for the cleanup, but didn't want to face commits across chromium-blink
> boundary for this simple thing :(

I guess I still don't understand what the problem is? If you initialize to 0,
then you'll return 1. If you initialize to 1, then you'll return 1. So the net
result should be the same? What am I missing?

[mustaq, 2015-04-24 16:59:02]

On 2015/04/24 16:42:30, jdduke wrote:
> On 2015/04/24 16:38:49, mustaq wrote:
> >
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> > File ui/android/java/src/org/chromium/ui/base/TouchDevice.java (right):
> > 
> >
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> > ui/android/java/src/org/chromium/ui/base/TouchDevice.java:57: int
> > pointerTypesVal = 0;
> > On 2015/04/24 16:17:32, jdduke wrote:
> > > Why not initialize this to PointerType.NONE, and get rid of the (if
> > > pointerTypesVal == 0) cleanup at the end? Same below in
availableHoverTypes.
> > 
> > Unfortunately this intuitive suggestion won't work here because the value of
> > NONE is 1, in both repos, for a historical mistake in spec interpretation. I
> had
> > planned for the cleanup, but didn't want to face commits across
chromium-blink
> > boundary for this simple thing :(
> 
> I guess I still don't understand what the problem is? If you initialize to 0,
> then you'll return 1. If you initialize to 1, then you'll return 1. So the net
> result should be the same? What am I missing?

Again NONE=1 is the confusing thing here: non-NONE values can combine with each
other but not with NONE. So it's okay to return FINE|COARSE here (when a system
has both mouse and touch), but not NONE|COARSE.

Since we have a bit reserved for NONE, we have to reset it if any other bit is
set before returning the value.

[jdduke (slow), 2015-04-24 17:00:25]

On 2015/04/24 16:59:02, mustaq wrote:
> On 2015/04/24 16:42:30, jdduke wrote:
> > On 2015/04/24 16:38:49, mustaq wrote:
> > >
> >
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> > > File ui/android/java/src/org/chromium/ui/base/TouchDevice.java (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1087263004/diff/1/ui/android/java/src/org/chr...
> > > ui/android/java/src/org/chromium/ui/base/TouchDevice.java:57: int
> > > pointerTypesVal = 0;
> > > On 2015/04/24 16:17:32, jdduke wrote:
> > > > Why not initialize this to PointerType.NONE, and get rid of the (if
> > > > pointerTypesVal == 0) cleanup at the end? Same below in
> availableHoverTypes.
> > > 
> > > Unfortunately this intuitive suggestion won't work here because the value
of
> > > NONE is 1, in both repos, for a historical mistake in spec interpretation.
I
> > had
> > > planned for the cleanup, but didn't want to face commits across
> chromium-blink
> > > boundary for this simple thing :(
> > 
> > I guess I still don't understand what the problem is? If you initialize to
0,
> > then you'll return 1. If you initialize to 1, then you'll return 1. So the
net
> > result should be the same? What am I missing?
> 
> Again NONE=1 is the confusing thing here: non-NONE values can combine with
each
> other but not with NONE. So it's okay to return FINE|COARSE here (when a
system
> has both mouse and touch), but not NONE|COARSE.
> 
> Since we have a bit reserved for NONE, we have to reset it if any other bit is
> set before returning the value.

Oh I see, well yeah, that is confusing :) lgtm.

[mustaq, 2015-04-24 17:15:43]

The CQ bit was checked by mustaq@chromium.org

[commit-bot: I haz the power, 2015-04-24 17:16:10]

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

[commit-bot: I haz the power, 2015-04-24 18:07:19]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-24 18:08:57]

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