Set the coordinates of the synthetic touch event correctly in SyntheticGestureTargetAura

content/browser/renderer_host/input/synthetic_gesture_target_aura.cc

Index: content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
diff --git a/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc b/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
index 64e1c16b9f72172561d84f1da9cb712e9c378fa6..3f86b069143db2eae640a3c85bb11aa27447f682 100644
--- a/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
+++ b/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
@@ -11,6 +11,7 @@
 #include "content/browser/renderer_host/ui_events_helper.h"
 #include "third_party/WebKit/public/platform/WebScreenInfo.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/events/event_processor.h"
 #include "ui/events/event_utils.h"
@@ -34,8 +35,6 @@ void SyntheticGestureTargetAura::DispatchWebTouchEventToPlatform(
     const ui::LatencyInfo& latency_info) {
   TouchEventWithLatencyInfo touch_with_latency(web_touch, latency_info);
   for (size_t i = 0; i < touch_with_latency.event.touchesLength; i++) {
-    touch_with_latency.event.touches[i].position.x *= device_scale_factor_;
-    touch_with_latency.event.touches[i].position.y *= device_scale_factor_;

[bokan, 2016/08/25 12:36:51]

Why did this get removed in the latest patch?

[lanwei, 2016/08/25 15:19:57]

If we use local coordinates, then we will use event.touches[i].position as the
position in the ui::TouchEvent, when we create ui::TouchEvent from WebTouchEvent
in MakeUITouchEventsFromWebTouchEvents. Then we do not need to apply the
device_scale_factor_, because later we do not converted back as I set transform
event to false, host->dispatcher()->set_transform_events(false).

I think here the order is wrong which makes the coordinates is not the same as
we pass at beginning.
Here when we get a pair of x, y from gpu_benchmarking.tap function, we apply the
device_scale_factor and then ConvertLocationToTarget, but in
EventProcessor::OnEventFromSource, it tries to transform the event position back
from device scale factor. Later when it is finding the target for the event, it
tries to convert point to target, then it gets a wrong coordinate. I feel it is
easy we do not device_scale_factor at all, and we can make sure we get the right
coordinate. What do you think?

 

[bokan, 2016/08/25 18:53:07]

Hmm, in that case, why did we need this in the first place? In other words, if
this works, why wouldn't leaving the |*= device_scale_factor| here and removing
the |set_transform_events(false)| work? Shouldn't it be equivalent? There's
something I'm not understanding here...
The comment at
https://cs.chromium.org/chromium/src/ui/aura/window_event_dispatcher.h?dr=CSs...
seems to say that the dispatcher gets events in physical pixel coordinates (i.e.
with device_scale_factor applied. [DeviceIndependentPixels *
device_scale_factor_ = physical pixels]).

The web_touch we get as a parameter in this method comes as input from tests so
it's in DIPs (device independent pixels). So to me it seems we *should* be
multiplying by device_scale_factor_ to get it into physical pixels and then
handing it off to the EventDispatcher which then *should* divide by
device_scale_factor (in  WindowEventDispatcher::OnEventProcessingStarted the
TransformEventForDeviceScaleFactor the function divides by DSF). Any idea where
this chain of events is breaking?

     touch_with_latency.event.touches[i].radiusX *= device_scale_factor_;
     touch_with_latency.event.touches[i].radiusY *= device_scale_factor_;
   }
@@ -46,6 +45,11 @@ void SyntheticGestureTargetAura::DispatchWebTouchEventToPlatform(
 
   aura::Window* window = GetWindow();
   aura::WindowTreeHost* host = window->GetHost();
+
+  // Synthetic touch events will be transformed here before being sent to the
+  // EventProcessor, so they should not be transformed again later in
+  // WindowEventDispatcher.
+  host->dispatcher()->set_transform_events(false);
   for (ScopedVector<ui::TouchEvent>::iterator iter = events.begin(),
       end = events.end(); iter != end; ++iter) {
     (*iter)->ConvertLocationToTarget(window, host->window());