Adding synthetic touch/mouse drag [Part1]

content/renderer/gpu/gpu_benchmarking_extension.cc

Index: content/renderer/gpu/gpu_benchmarking_extension.cc
diff --git a/content/renderer/gpu/gpu_benchmarking_extension.cc b/content/renderer/gpu/gpu_benchmarking_extension.cc
index 68221886255898ee802d9565de7841ad6f73cf27..76e999f5be42298893714235a40bcddaffc5db45 100644
--- a/content/renderer/gpu/gpu_benchmarking_extension.cc
+++ b/content/renderer/gpu/gpu_benchmarking_extension.cc
@@ -14,6 +14,7 @@
 #include "cc/layers/layer.h"
 #include "content/common/input/synthetic_gesture_params.h"
 #include "content/common/input/synthetic_pinch_gesture_params.h"
+#include "content/common/input/synthetic_smooth_drag_gesture_params.h"
 #include "content/common/input/synthetic_smooth_scroll_gesture_params.h"
 #include "content/common/input/synthetic_tap_gesture_params.h"
 #include "content/public/child/v8_value_converter.h"
@@ -381,6 +382,44 @@ bool BeginSmoothScroll(v8::Isolate* isolate,
   return true;
 }
 
+bool BeginSmoothDrag(v8::Isolate* isolate,
+                     float start_x,
+                     float start_y,
+                     float end_x,
+                     float end_y,
+                     v8::Handle<v8::Function> callback,
+                     int gesture_source_type,
+                     int speed_in_pixels_s) {

[picksi, 2015/02/19 11:50:19]

Wow! Lots of parameters. In general it would be better to construct a struct (or
class) an element at a time and pass that instead.

[ssid, 2015/02/20 10:58:55]

This change is moved to second part of the CL.

+  GpuBenchmarkingContext context;
+  if (!context.Init(false))

[picksi, 2015/02/19 11:50:18]

What does this false parameter mean? This looks odd to a reader who doesn't know
what the context init is all about. Could we replace the false with a const that
says what it means?

[ssid, 2015/02/20 10:58:55]

It is the bit copied from previous function. Will add it to the list for future

+    return false;
+  scoped_refptr<CallbackAndContext> callback_and_context =
+      new CallbackAndContext(isolate, callback,
+                             context.web_frame()->mainWorldScriptContext());
+
+  scoped_ptr<SyntheticSmoothDragGestureParams> gesture_params(
+      new SyntheticSmoothDragGestureParams);
+
+  // Convert coordinates from CSS pixels to density independent pixels (DIPs).
+  float page_scale_factor = context.web_view()->pageScaleFactor();
+
+  gesture_params->start_point.SetPoint(start_x * page_scale_factor,

[picksi, 2015/02/19 11:50:18]

Not sure what code exists for this stuff, but it would be neater to use vector
manipulation here rather than doing the hand multiplication & x/y subtraction.

e.g. Vector2d startPosition(start_x,start_y)
     Vector2d endPosition(end_x,end_y)
     Vector2d delta = endPosition-startPosition
     
     ScaleVector(startPosition, page_scale_factor)
     ScaleVector(delta, page_scale_factor)

[ssid, 2015/02/20 10:58:55]

Done.

+                                       start_y * page_scale_factor);
+  gfx::Vector2d distance((end_x - start_x) * page_scale_factor,
+                         (end_y - start_y) * page_scale_factor);

[picksi, 2015/02/20 11:51:18]

I'd still like to see the *page_scale_factor performed on the vectors, not the
individual floats.

e.g.

distance = distance * page_scale_factor;
start_point = start_point * page_scale_factor;

As this makes the intention of the code conceptually closer to what we want to
do (which is scale the vectors)... and could end up being faster on some
architectures; Although it probably won't in reality :)

+  gesture_params->distances.push_back(distance);
+  gesture_params->speed_in_pixels_s = speed_in_pixels_s * page_scale_factor;
+  gesture_params->gesture_source_type =
+      static_cast<SyntheticGestureParams::GestureSourceType>(
+          gesture_source_type);
+
+  context.render_view_impl()->QueueSyntheticGesture(

[Sami, 2015/02/19 11:59:06]

Please copy the same TODO(nduca) comment from above here too.

[ssid, 2015/02/20 10:58:55]

Done.

+      gesture_params.Pass(),
+      base::Bind(&OnSyntheticGestureCompleted, callback_and_context));
+
+  return true;
+}
+
 }  // namespace
 
 gin::WrapperInfo GpuBenchmarking::kWrapperInfo = {gin::kEmbedderNativeGin};
@@ -425,6 +464,7 @@ gin::ObjectTemplateBuilder GpuBenchmarking::GetObjectTemplateBuilder(
       .SetMethod("gestureSourceTypeSupported",
                  &GpuBenchmarking::GestureSourceTypeSupported)
       .SetMethod("smoothScrollBy", &GpuBenchmarking::SmoothScrollBy)
+      .SetMethod("smoothDrag", &GpuBenchmarking::SmoothDrag)
       .SetMethod("swipe", &GpuBenchmarking::Swipe)
       .SetMethod("scrollBounce", &GpuBenchmarking::ScrollBounce)
       // TODO(dominikg): Remove once JS interface changes have rolled into
@@ -529,6 +569,39 @@ bool GpuBenchmarking::SmoothScrollBy(gin::Arguments* args) {
                            start_y);
 }
 
+bool GpuBenchmarking::SmoothDrag(gin::Arguments* args) {
+  GpuBenchmarkingContext context;
+  if (!context.Init(true))
+    return false;
+
+  float start_x;
+  float start_y;
+  float end_x;
+  float end_y;
+  v8::Handle<v8::Function> callback;
+  int gesture_source_type = 0;  // Default input

[picksi, 2015/02/19 11:50:19]

is there an enum for this '0'?

[picksi, 2015/02/20 11:51:18]

Would still like to see this use the correct enum to prevent it going wrong when
new enums are added in the future.

+  int speed_in_pixels_s = 800;
+
+  if (!GetArg(args, &start_x) ||
+      !GetArg(args, &start_y) ||
+      !GetArg(args, &end_x) ||
+      !GetArg(args, &end_y) ||
+      !GetOptionalArg(args, &callback) ||
+      !GetOptionalArg(args, &gesture_source_type) ||
+      !GetOptionalArg(args, &speed_in_pixels_s)) {
+    return false;
+  }
+
+  return BeginSmoothDrag(args->isolate(),
+                         start_x,
+                         start_y,
+                         end_x,
+                         end_y,
+                         callback,
+                         gesture_source_type,
+                         speed_in_pixels_s);
+}
+
 bool GpuBenchmarking::Swipe(gin::Arguments* args) {
   GpuBenchmarkingContext context;
   if (!context.Init(true))