Adding NOTREACHED in Read() in Event StructTraits to add clarity to the code.

ui/events/mojo/event_struct_traits.cc

Index: ui/events/mojo/event_struct_traits.cc
diff --git a/ui/events/mojo/event_struct_traits.cc b/ui/events/mojo/event_struct_traits.cc
index 21ae98f1201a0488c4c0b4fd7379e4eaef77198c..7b4c537bd6fd2d2920bd4f104d5e57417a1933d2 100644
--- a/ui/events/mojo/event_struct_traits.cc
+++ b/ui/events/mojo/event_struct_traits.cc
@@ -308,6 +308,7 @@ bool StructTraits<ui::mojom::EventDataView, EventUniquePtr>::Read(
       return false;
   }
 
+  NOTREACHED();

[sadrul, 2017/03/02 00:00:38]

I am not sure if NOTREACHED() is good here. Because this means a sender can
trigger a crash to a remote app by sending it an invalid event, which I think is
something we should avoid.

[mfomitchev, 2017/03/02 00:03:41]

If the event is invalid, it's type will be UNKNOWN, and we will return false,
no?

[sadrul, 2017/03/02 02:22:31]

I think it's possible for the sender to send an event with an undefined action
(i.e. none of the valid ui::mojom::EventTypes)?

[mfomitchev, 2017/03/02 03:05:45]

Hmm.. at this point we have EventDataView object, and not just some random blob
of binary data. EventDataView::action() is returning a value of type
::ui::mojom::EventType, which I think means that it should be one of the valid
EventType values. If we somehow received an invalid serialized value, I think it
would be handled deeper in the guts of Mojo? Here we have a typed value
already..

As you may have guessed, I've put this up merely as a precursor/sanity check for
https://codereview.chromium.org/2728883002/. If we indeed have a case where we
can legitimately fall through to this return, we should probably structure the
code in that CL a little differently.

[Tom Sepez, 2017/03/02 18:54:37]

Doesn't NOTREACHED() only fire in the debug code?  If so, its fine to leave it
as the release build avoids it, and we don't mind crashing in debug.

   return false;
 }