Consider isPrimary and pointerType when preventing compat mouse

components/test_runner/event_sender.cc

Index: components/test_runner/event_sender.cc
diff --git a/components/test_runner/event_sender.cc b/components/test_runner/event_sender.cc
index 393572482957e5cc3f4162a6fbb5fb366717c278..a690fced43f9114017f5f260e1f36c65d275cf2d 100644
--- a/components/test_runner/event_sender.cc
+++ b/components/test_runner/event_sender.cc
@@ -58,6 +58,35 @@ namespace test_runner {
 
 namespace {
 
+const char* kPointerTypeStringUnknown = "";
+const char* kPointerTypeStringMouse = "mouse";
+const char* kPointerTypeStringPen = "pen";
+const char* kPointerTypeStringTouch = "touch";
+
+bool getPointerType(gin::Arguments* args,
+                    blink::WebPointerProperties::PointerType &t) {
+    if (!args->PeekNext().IsEmpty()) {
+      std::string pointer_type_string;
+      if (!args->GetNext(&pointer_type_string)) {
+        args->ThrowError();
+        return true;
+      }
+      if (pointer_type_string == kPointerTypeStringUnknown) {
+        t = WebMouseEvent::PointerType::Unknown;
+      } else if (pointer_type_string == kPointerTypeStringMouse) {
+        t = WebMouseEvent::PointerType::Mouse;
+      } else if (pointer_type_string == kPointerTypeStringPen) {
+        t = WebMouseEvent::PointerType::Pen;
+      } else if (pointer_type_string == kPointerTypeStringTouch) {
+        t = WebMouseEvent::PointerType::Touch;
+      } else {
+        args->ThrowError();
+        return true;
+      }
+    }
+    return false;
+}
+
 WebMouseEvent::Button GetButtonTypeFromButtonNumber(int button_code) {
   switch (button_code) {
     case -1:
@@ -103,6 +132,8 @@ void InitMouseEvent(WebInputEvent::Type t,
                     double time_stamp,
                     int click_count,
                     int modifiers,
+                    blink::WebPointerProperties::PointerType pointerType,
+                    int pointerId,
                     WebMouseEvent* e) {
   e->type = t;
   e->button = b;
@@ -111,7 +142,8 @@ void InitMouseEvent(WebInputEvent::Type t,
   e->y = pos.y;
   e->globalX = pos.x;
   e->globalY = pos.y;
-  e->pointerType = blink::WebPointerProperties::PointerType::Mouse;
+  e->pointerType = pointerType;
+  e->id = pointerId;
   e->timeStampSeconds = time_stamp;
   e->clickCount = click_count;
 }
@@ -455,11 +487,6 @@ bool GetScrollUnits(gin::Arguments* args, WebGestureEvent::ScrollUnits* units) {
 const char* kSourceDeviceStringTouchpad = "touchpad";
 const char* kSourceDeviceStringTouchscreen = "touchscreen";
 
-const char* kPointerTypeStringUnknown = "";
-const char* kPointerTypeStringMouse = "mouse";
-const char* kPointerTypeStringPen = "pen";
-const char* kPointerTypeStringTouch = "touch";
-
 }  // namespace
 
 class EventSenderBindings : public gin::Wrappable<EventSenderBindings> {
@@ -978,12 +1005,26 @@ void EventSenderBindings::MouseDown(gin::Arguments* args) {
 
   int button_number = 0;
   int modifiers = 0;
+  blink::WebPointerProperties::PointerType type =
+      blink::WebPointerProperties::PointerType::Mouse;
+  int pointerId = 0;
   if (!args->PeekNext().IsEmpty()) {
     args->GetNext(&button_number);
-    if (!args->PeekNext().IsEmpty())
+    if (!args->PeekNext().IsEmpty()) {
       modifiers = GetKeyModifiersFromV8(args->isolate(), args->PeekNext());
+      args->Skip();
+    }
   }
-  sender_->MouseDown(button_number, modifiers);
+  getPointerType(args, type);
+  if (!args->PeekNext().IsEmpty())
+    args->GetNext(&pointerId);
+
+  // Only allow pen or mouse through this API;
+  if (type != blink::WebPointerProperties::PointerType::Mouse
+      && type != blink::WebPointerProperties::PointerType::Pen)
+      type = blink::WebPointerProperties::PointerType::Mouse;
+
+  sender_->MouseDown(button_number, modifiers, type, pointerId);
 }
 
 void EventSenderBindings::MouseUp(gin::Arguments* args) {
@@ -992,12 +1033,26 @@ void EventSenderBindings::MouseUp(gin::Arguments* args) {
 
   int button_number = 0;
   int modifiers = 0;
+  blink::WebPointerProperties::PointerType type =
+      blink::WebPointerProperties::PointerType::Mouse;
+  int pointerId = 0;
   if (!args->PeekNext().IsEmpty()) {
     args->GetNext(&button_number);
-    if (!args->PeekNext().IsEmpty())
+    if (!args->PeekNext().IsEmpty()) {
       modifiers = GetKeyModifiersFromV8(args->isolate(), args->PeekNext());
+      args->Skip();
+    }
   }
-  sender_->MouseUp(button_number, modifiers);
+  getPointerType(args, type);

[dtapuska, 2016/04/05 19:21:25]

This API returns a bool... should we handle the return?

[Navid Zolghadr, 2016/04/05 20:17:25]

I wanted to do that too. However this function was not error checking itself for
the button_number. So I thought it would be odd to only handle it here. I added
the error check for the previous field as well. Is that okay?

[dtapuska, 2016/04/05 20:30:40]

looks good.

+  if (!args->PeekNext().IsEmpty())
+    args->GetNext(&pointerId);
+
+  // Only allow pen or mouse through this API;
+  if (type != blink::WebPointerProperties::PointerType::Mouse

[dtapuska, 2016/04/05 19:21:24]

Personally I think a switch is easier to read. Also comment is an incomplete
sentence terminating with a ";"

[Navid Zolghadr, 2016/04/05 20:17:25]

I changed the way I handle this. I moved it to getPointerType function and
return error in any other case rather than silently overriding it. Is that
better or am I misusing that error here?

[dtapuska, 2016/04/05 20:30:40]

Seems reasonable to me.

+      && type != blink::WebPointerProperties::PointerType::Pen)
+      type = blink::WebPointerProperties::PointerType::Mouse;
+
+  sender_->MouseUp(button_number, modifiers, type, pointerId);
 }
 
 void EventSenderBindings::SetMouseButtonState(gin::Arguments* args) {
@@ -1203,6 +1258,7 @@ void EventSender::Reset() {
   current_drag_effects_allowed_ = blink::WebDragOperationNone;
   if (view_ && pressed_button_ != WebMouseEvent::ButtonNone)
     view_->mouseCaptureLost();
+  stateOfPen.clear();
   pressed_button_ = WebMouseEvent::ButtonNone;
   current_buttons_ = 0;
   modifiers_ = 0;
@@ -1264,6 +1320,8 @@ void EventSender::DoDragDrop(const WebDragData& drag_data,
                  GetCurrentEventTimeSec(),
                  click_count_,
                  modifiers_,
+                 blink::WebPointerProperties::PointerType::Mouse,
+                 0,
                  &event);
   WebPoint client_point(event.x, event.y);
   WebPoint screen_point(event.globalX, event.globalY);
@@ -1280,7 +1338,8 @@ void EventSender::DoDragDrop(const WebDragData& drag_data,
   ReplaySavedEvents();
 }
 
-void EventSender::MouseDown(int button_number, int modifiers) {
+void EventSender::MouseDown(int button_number, int modifiers,
+    blink::WebPointerProperties::PointerType pointerType, int pointerId) {
   if (force_layout_on_events_)
     view_->updateAllLifecyclePhases();
 
@@ -1289,25 +1348,45 @@ void EventSender::MouseDown(int button_number, int modifiers) {
   WebMouseEvent::Button button_type =
       GetButtonTypeFromButtonNumber(button_number);
 
-  UpdateClickCountForButton(button_type);
+  WebMouseEvent event;
+  if (pointerType == blink::WebPointerProperties::PointerType::Mouse) {
+    UpdateClickCountForButton(button_type);
 
-  pressed_button_ = button_type;
-  current_buttons_ |= GetWebMouseEventModifierForButton(pressed_button_);
-  modifiers_ = modifiers;
+    pressed_button_ = button_type;
+    current_buttons_ |= GetWebMouseEventModifierForButton(pressed_button_);
+    modifiers_ = modifiers;
+    InitMouseEvent(WebInputEvent::MouseDown,
+                   pressed_button_,

[dtapuska, 2016/04/05 19:21:25]

Would it make sense to store some of this in the stateOfPen map? but just have a
special pointerId value for the mouse?

[Navid Zolghadr, 2016/04/05 20:17:25]

I'd like to do that too as it is pretty much the same state. Although they are
all static but I don't think there is any problem removing their static
modifier.
If I do that I need to either prevent this API to get that particular mouse id
for the pen or just remove the id from the input altogether and assign the ids
myself in event_sender similar to the touch ids. Which way do you think is
better?

[dtapuska, 2016/04/05 20:30:40]

The current_pen_state_ is a unordered_map; so I'd use an integer that isn't
expected to be a pointer id.. Is there an explicit range? It's up to you if you
allow pointerIds to be assigned by the event sender; I don't have strong
feelings either way.

+                   current_buttons_,
+                   last_mouse_pos_,
+                   GetCurrentEventTimeSec(),
+                   click_count_,
+                   modifiers_,
+                   pointerType,
+                   0,
+                   &event);
+  } else {
+    stateOfPen[pointerId].pressed_button_ = button_type;
+    stateOfPen[pointerId].current_buttons_ |=
+      GetWebMouseEventModifierForButton(pressed_button_);
+    stateOfPen[pointerId].modifiers_ = modifiers;
+    InitMouseEvent(WebInputEvent::MouseDown,
+                   stateOfPen[pointerId].pressed_button_,
+                   stateOfPen[pointerId].current_buttons_,
+                   stateOfPen[pointerId].last_pos_,
+                   GetCurrentEventTimeSec(),
+                   0,
+                   modifiers_,
+                   pointerType,
+                   pointerId,
+                   &event);
+  }
 
-  WebMouseEvent event;
-  InitMouseEvent(WebInputEvent::MouseDown,
-                 pressed_button_,
-                 current_buttons_,
-                 last_mouse_pos_,
-                 GetCurrentEventTimeSec(),
-                 click_count_,
-                 modifiers_,
-                 &event);
   HandleInputEventOnViewOrPopup(event);
 }
 
-void EventSender::MouseUp(int button_number, int modifiers) {
+void EventSender::MouseUp(int button_number, int modifiers,
+    blink::WebPointerProperties::PointerType pointerType, int pointerId) {
   if (force_layout_on_events_)
     view_->updateAllLifecyclePhases();
 
@@ -1316,27 +1395,48 @@ void EventSender::MouseUp(int button_number, int modifiers) {
   WebMouseEvent::Button button_type =
       GetButtonTypeFromButtonNumber(button_number);
 
-  if (is_drag_mode_ && !replaying_saved_events_) {
-    SavedEvent saved_event;
-    saved_event.type = SavedEvent::TYPE_MOUSE_UP;
-    saved_event.button_type = button_type;
-    saved_event.modifiers = modifiers;
-    mouse_event_queue_.push_back(saved_event);
-    ReplaySavedEvents();
+  if (pointerType == blink::WebPointerProperties::PointerType::Mouse) {
+    if (is_drag_mode_ && !replaying_saved_events_) {
+      SavedEvent saved_event;
+      saved_event.type = SavedEvent::TYPE_MOUSE_UP;
+      saved_event.button_type = button_type;
+      saved_event.modifiers = modifiers;
+      mouse_event_queue_.push_back(saved_event);
+      ReplaySavedEvents();
+    } else {
+      current_buttons_ &= ~GetWebMouseEventModifierForButton(button_type);
+      pressed_button_ = WebMouseEvent::ButtonNone;
+
+      WebMouseEvent event;
+      InitMouseEvent(WebInputEvent::MouseUp,
+                     button_type,
+                     current_buttons_,
+                     last_mouse_pos_,
+                     GetCurrentEventTimeSec(),
+                     click_count_,
+                     modifiers,
+                     pointerType,
+                     0,
+                     &event);
+      DoMouseUp(event);
+    }
   } else {
-    current_buttons_ &= ~GetWebMouseEventModifierForButton(button_type);
-    pressed_button_ = WebMouseEvent::ButtonNone;
-
-    WebMouseEvent event;
-    InitMouseEvent(WebInputEvent::MouseUp,
-                   button_type,
-                   current_buttons_,
-                   last_mouse_pos_,
-                   GetCurrentEventTimeSec(),
-                   click_count_,
-                   modifiers,
-                   &event);
-    DoMouseUp(event);
+      stateOfPen[pointerId].current_buttons_ &=
+          ~GetWebMouseEventModifierForButton(button_type);
+      stateOfPen[pointerId].pressed_button_ = WebMouseEvent::ButtonNone;
+
+      WebMouseEvent event;
+      InitMouseEvent(WebInputEvent::MouseUp,
+                     button_type,
+                     stateOfPen[pointerId].current_buttons_,
+                     stateOfPen[pointerId].last_pos_,
+                     GetCurrentEventTimeSec(),
+                     0,
+                     modifiers,
+                     pointerType,
+                     pointerId,
+                     &event);
+      HandleInputEventOnViewOrPopup(event);
   }
 }
 
@@ -1550,6 +1650,8 @@ void EventSender::KeyDown(const std::string& code_str,
                    GetCurrentEventTimeSec(),
                    click_count_,
                    0,
+                   blink::WebPointerProperties::PointerType::Mouse,
+                   0,
                    &event);
     FinishDragAndDrop(event, blink::WebDragOperationNone);
   }
@@ -1605,6 +1707,8 @@ std::vector<std::string> EventSender::ContextClick() {
                  GetCurrentEventTimeSec(),
                  click_count_,
                  0,
+                 blink::WebPointerProperties::PointerType::Mouse,
+                 0,
                  &event);
   HandleInputEventOnViewOrPopup(event);
 
@@ -1620,6 +1724,8 @@ std::vector<std::string> EventSender::ContextClick() {
                  GetCurrentEventTimeSec(),
                  click_count_,
                  0,
+                 blink::WebPointerProperties::PointerType::Mouse,
+                 0,
                  &event);
   HandleInputEventOnViewOrPopup(event);
 #endif
@@ -1973,6 +2079,9 @@ void EventSender::MouseMoveTo(gin::Arguments* args) {
 
   double x;
   double y;
+  blink::WebPointerProperties::PointerType pointerType =
+      blink::WebPointerProperties::PointerType::Mouse;
+  int pointerId = 0;
   if (!args->GetNext(&x) || !args->GetNext(&y)) {
     args->ThrowError();
     return;
@@ -1980,27 +2089,53 @@ void EventSender::MouseMoveTo(gin::Arguments* args) {
   WebPoint mouse_pos(static_cast<int>(x), static_cast<int>(y));
 
   int modifiers = 0;
-  if (!args->PeekNext().IsEmpty())
+  if (!args->PeekNext().IsEmpty()) {
     modifiers = GetKeyModifiersFromV8(args->isolate(), args->PeekNext());
+    args->Skip();
+  }
 
-  if (is_drag_mode_ && pressed_button_ == WebMouseEvent::ButtonLeft &&
-      !replaying_saved_events_) {
-    SavedEvent saved_event;
-    saved_event.type = SavedEvent::TYPE_MOUSE_MOVE;
-    saved_event.pos = mouse_pos;
-    saved_event.modifiers = modifiers;
-    mouse_event_queue_.push_back(saved_event);
+  getPointerType(args, pointerType);

[dtapuska, 2016/04/05 19:21:25]

same comments regarding return type.

[Navid Zolghadr, 2016/04/05 20:17:25]

Done.

+
+  if (!args->PeekNext().IsEmpty())
+    args->GetNext(&pointerId);
+
+  if (pointerType == blink::WebPointerProperties::PointerType::Mouse) {
+    if (is_drag_mode_ && pressed_button_ == WebMouseEvent::ButtonLeft &&
+        !replaying_saved_events_) {
+      SavedEvent saved_event;
+      saved_event.type = SavedEvent::TYPE_MOUSE_MOVE;
+      saved_event.pos = mouse_pos;
+      saved_event.modifiers = modifiers;
+      mouse_event_queue_.push_back(saved_event);
+    } else {
+      WebMouseEvent event;
+      InitMouseEvent(WebInputEvent::MouseMove,
+                     pressed_button_,
+                     current_buttons_,
+                     mouse_pos,
+                     GetCurrentEventTimeSec(),
+                     click_count_,
+                     modifiers,
+                     pointerType,
+                     0,
+                     &event);
+      DoMouseMove(event);
+    }
   } else {
-    WebMouseEvent event;
-    InitMouseEvent(WebInputEvent::MouseMove,
-                   pressed_button_,
-                   current_buttons_,
-                   mouse_pos,
-                   GetCurrentEventTimeSec(),
-                   click_count_,
-                   modifiers,
-                   &event);
-    DoMouseMove(event);
+      stateOfPen[pointerId].last_pos_ = mouse_pos;
+
+      WebMouseEvent event;
+      InitMouseEvent(WebInputEvent::MouseMove,
+                     stateOfPen[pointerId].pressed_button_,
+                     stateOfPen[pointerId].current_buttons_,
+                     mouse_pos,
+                     GetCurrentEventTimeSec(),
+                     0,
+                     modifiers,
+                     pointerType,
+                     pointerId,
+                     &event);
+      HandleInputEventOnViewOrPopup(event);
   }
 }
 
@@ -2016,6 +2151,8 @@ void EventSender::MouseLeave() {
                  GetCurrentEventTimeSec(),
                  click_count_,
                  0,
+                 blink::WebPointerProperties::PointerType::Mouse,
+                 0,
                  &event);
   HandleInputEventOnViewOrPopup(event);
 }
@@ -2333,6 +2470,8 @@ void EventSender::GestureEvent(WebInputEvent::Type type,
                    GetCurrentEventTimeSec(),
                    click_count_,
                    modifiers_,
+                   blink::WebPointerProperties::PointerType::Mouse,
+                   0,
                    &mouse_event);
 
     FinishDragAndDrop(mouse_event, blink::WebDragOperationNone);
@@ -2404,6 +2543,8 @@ void EventSender::InitMouseWheelEvent(gin::Arguments* args,
                  GetCurrentEventTimeSec(),
                  click_count_,
                  modifiers,
+                 blink::WebPointerProperties::PointerType::Mouse,
+                 0,
                  event);
   event->wheelTicksX = static_cast<float>(horizontal);
   event->wheelTicksY = static_cast<float>(vertical);
@@ -2466,24 +2607,8 @@ void EventSender::InitPointerProperties(gin::Arguments* args,
     e->tiltY = tiltY;
   }
 
-  if (!args->PeekNext().IsEmpty()) {
-    std::string pointer_type_string;
-    if (!args->GetNext(&pointer_type_string)) {
-      args->ThrowError();
-      return;
-    }
-    if (pointer_type_string == kPointerTypeStringUnknown) {
-      e->pointerType = WebMouseEvent::PointerType::Unknown;
-    } else if (pointer_type_string == kPointerTypeStringMouse) {
-      e->pointerType = WebMouseEvent::PointerType::Mouse;
-    } else if (pointer_type_string == kPointerTypeStringPen) {
-      e->pointerType = WebMouseEvent::PointerType::Pen;
-    } else if (pointer_type_string == kPointerTypeStringTouch) {
-      e->pointerType = WebMouseEvent::PointerType::Touch;
-    } else {
-      args->ThrowError();
+  if (getPointerType(args, e->pointerType)) {
       return;
-    }
   }
 }
 
@@ -2562,6 +2687,8 @@ void EventSender::ReplaySavedEvents() {
                        GetCurrentEventTimeSec(),
                        click_count_,
                        e.modifiers,
+                       blink::WebPointerProperties::PointerType::Mouse,
+                       0,
                        &event);
         DoMouseMove(event);
         break;
@@ -2581,6 +2708,8 @@ void EventSender::ReplaySavedEvents() {
                        GetCurrentEventTimeSec(),
                        click_count_,
                        e.modifiers,
+                       blink::WebPointerProperties::PointerType::Mouse,
+                       0,
                        &event);
         DoMouseUp(event);
         break;