Introduce a kSetEventMaskCommand, some cleanups in windows eventhandler

Introduce a kSetEventMaskCommand, some cleanups in windows eventhandler

R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43547

[kustermann, 2015-02-06 00:41:54]

kustermann@google.com changed reviewers:
+ sgjesse@google.com

[Søren Gjesse, 2015-02-06 08:09:26]

LGTM with comments

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
File dart/runtime/bin/eventhandler.h (right):

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler.h:45: #define IS_EVENT(data, command_bit) \
command_bit -> event_bit

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
File dart/runtime/bin/eventhandler_android.cc (right):

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:172: } else {
Maybe also add an assert that (msg[i].data & COMMAND_MASK != 0). It should be
caught by the UNREACHABLE further down, but would make it explicit that a
command bit it needed.

Maybe even better would be to asset that exactly one command bit is set.

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:174: msg[i].id, (msg[i].data & (1 <<
kListeningSocket)) != 0);
Maybe add LISTENING_MASK (defined as (1 << kListeningSocket))

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:202: ASSERT(0 == (events & ~(1 <<
kInEvent) & ~(1 << kOutEvent)));
I think

(events & ~(1 << kInEvent) | 1 << kOutEvent))

reads better

[kustermann, 2015-02-06 09:28:59]

New patchsets have been uploaded after l-g-t-m from sgjesse@google.com

[kustermann, 2015-02-06 09:36:47]

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
File dart/runtime/bin/eventhandler.h (right):

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler.h:45: #define IS_EVENT(data, command_bit) \
On 2015/02/06 08:09:25, Søren Gjesse wrote:
> command_bit -> event_bit

Done.

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
File dart/runtime/bin/eventhandler_android.cc (right):

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:172: } else {
On 2015/02/06 08:09:25, Søren Gjesse wrote:
> Maybe also add an assert that (msg[i].data & COMMAND_MASK != 0). It should be
> caught by the UNREACHABLE further down, but would make it explicit that a
> command bit it needed.
> 
> Maybe even better would be to asset that exactly one command bit is set.

Done - although I find the UNREACHABLE enough.

The IS_COMMAND checks that it is exactly that command, so setting multiple
commands will fall through to UNREACHABLE.

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:174: msg[i].id, (msg[i].data & (1 <<
kListeningSocket)) != 0);
On 2015/02/06 08:09:25, Søren Gjesse wrote:
> Maybe add LISTENING_MASK (defined as (1 << kListeningSocket))

I defined a IS_LISTENING_SOCKET.

https://codereview.chromium.org/900363004/diff/20001/dart/runtime/bin/eventha...
dart/runtime/bin/eventhandler_android.cc:202: ASSERT(0 == (events & ~(1 <<
kInEvent) & ~(1 << kOutEvent)));
On 2015/02/06 08:09:25, Søren Gjesse wrote:
> I think
> 
> (events & ~(1 << kInEvent) | 1 << kOutEvent))
> 
> reads better

Done.

[kustermann, 2015-02-06 09:37:19]

Committed patchset #3 (id:40001) manually as 43547.