Implement EventSender.mouseWheelTo method....

Implement EventSender.mouseWheelTo method.

WebKit fix is also needed to make the test pass.
http://bugs.webkit.org/show_bug.cgi?id=31292

BUG=24175
TEST=LayoutTests/platform/gtk/scrollbars/overflow-scrollbar-horizontal-wheel-scroll.html

[kinuko (google), 2009-11-11 03:33:57]

Reviewers: levin, Evan Martin, Evan Stade,

Description:
Implement EventSender.mouseWheelTo method.

WebKit fix is also needed to make the test pass.
http://bugs.webkit.org/show_bug.cgi?id=31292

BUG=24175
TEST=LayoutTests/platform/gtk/scrollbars/overflow-scrollbar-horizontal-wheel-scroll.html


Please review this at http://codereview.chromium.org/387002

SVN Base: http://src.chromium.org/svn/trunk/src/

Affected files:
   M     webkit/tools/test_shell/event_sending_controller.h
   M     webkit/tools/test_shell/event_sending_controller.cc


Index: webkit/tools/test_shell/event_sending_controller.cc
===================================================================
--- webkit/tools/test_shell/event_sending_controller.cc	(revision 31504)
+++ webkit/tools/test_shell/event_sending_controller.cc	(working copy)
@@ -56,6 +56,7 @@
  using WebKit::WebInputEvent;
  using WebKit::WebKeyboardEvent;
  using WebKit::WebMouseEvent;
+using WebKit::WebMouseWheelEvent;
  using WebKit::WebPoint;
  using WebKit::WebString;
  using WebKit::WebView;
@@ -106,6 +107,10 @@
  static const double kMultiClickTimeSec = 1;
  static const int kMultiClickRadiusPixels = 5;

+// How much we should scroll per event - the value here is chosen to
+// match the WebKit impl and layout test results.
+static const float kScrollbarPixelsPerTick = 40.0f;
+
  inline bool outside_multiclick_radius(const gfx::Point &a, const  
gfx::Point &b) {
    return ((a.x() - b.x()) * (a.x() - b.x()) + (a.y() - b.y()) * (a.y() -  
b.y())) >
      kMultiClickRadiusPixels * kMultiClickRadiusPixels;
@@ -183,6 +188,7 @@
    BindMethod("mouseUp", &EventSendingController::mouseUp);
    BindMethod("contextClick", &EventSendingController::contextClick);
    BindMethod("mouseMoveTo", &EventSendingController::mouseMoveTo);
+  BindMethod("mouseWheelTo", &EventSendingController::mouseWheelTo);
    BindMethod("leapForward", &EventSendingController::leapForward);
    BindMethod("keyDown", &EventSendingController::keyDown);
    BindMethod("dispatchMessage", &EventSendingController::dispatchMessage);
@@ -404,6 +410,27 @@
    }
  }

+void EventSendingController::mouseWheelTo(
+    const CppArgumentList& args, CppVariant* result) {
+  result->SetNull();
+
+  if (args.size() >= 2 && args[0].isNumber() && args[1].isNumber()) {
+    webview()->layout();
+
+    int horizontal = args[0].ToInt32();
+    int vertical = args[1].ToInt32();
+
+    WebMouseWheelEvent event;
+    InitMouseEvent(WebInputEvent::MouseWheel, pressed_button_,
+                   last_mouse_pos_, &event);
+    event.wheelTicksX = static_cast<float>(horizontal);
+    event.wheelTicksY = static_cast<float>(vertical);
+    event.deltaX = -1 * horizontal * kScrollbarPixelsPerTick;
+    event.deltaY = -1 * vertical * kScrollbarPixelsPerTick;
+    webview()->handleInputEvent(event);
+  }
+}
+
  // static
  void EventSendingController::DoMouseMove(const WebMouseEvent& e) {
    last_mouse_pos_.SetPoint(e.x, e.y);
Index: webkit/tools/test_shell/event_sending_controller.h
===================================================================
--- webkit/tools/test_shell/event_sending_controller.h	(revision 31504)
+++ webkit/tools/test_shell/event_sending_controller.h	(working copy)
@@ -49,6 +49,7 @@
    void mouseDown(const CppArgumentList& args, CppVariant* result);
    void mouseUp(const CppArgumentList& args, CppVariant* result);
    void mouseMoveTo(const CppArgumentList& args, CppVariant* result);
+  void mouseWheelTo(const CppArgumentList& args, CppVariant* result);
    void leapForward(const CppArgumentList& args, CppVariant* result);
    void keyDown(const CppArgumentList& args, CppVariant* result);
    void dispatchMessage(const CppArgumentList& args, CppVariant* result);

[Evan Martin, 2009-11-11 08:08:11]

LGTM

http://codereview.chromium.org/387002/diff/3001/6
File webkit/tools/test_shell/event_sending_controller.cc (right):

http://codereview.chromium.org/387002/diff/3001/6#newcode418
Line 418: webview()->layout();
why is this layout necessary?  maybe add a comment saying why if it is really
needed

[Evan Martin, 2009-11-11 08:08:30]

LGTM


http://codereview.chromium.org/387002/diff/3001/6
File webkit/tools/test_shell/event_sending_controller.cc (right):

http://codereview.chromium.org/387002/diff/3001/6#newcode418
Line 418: webview()->layout();
why is this layout necessary?  maybe add a comment saying why if it is
really needed

http://codereview.chromium.org/387002

[kinuko (google), 2009-11-11 10:49:18]

On 2009/11/11 08:08:30, Evan Martin wrote:
> LGTM
> 
> 
> http://codereview.chromium.org/387002/diff/3001/6
> File webkit/tools/test_shell/event_sending_controller.cc (right):
> 
> http://codereview.chromium.org/387002/diff/3001/6#newcode418
> Line 418: webview()->layout();
> why is this layout necessary?  maybe add a comment saying why if it is
> really needed

Added a comment explaining why it's there (actually the test does
not rely on this).

> 
> http://codereview.chromium.org/387002

[kinuko (google), 2009-11-11 10:49:40]

On 2009/11/11 08:08:30, Evan Martin wrote:
> LGTM


> http://codereview.chromium.org/387002/diff/3001/6
> File webkit/tools/test_shell/event_sending_controller.cc (right):

> http://codereview.chromium.org/387002/diff/3001/6#newcode418
> Line 418: webview()->layout();
> why is this layout necessary?  maybe add a comment saying why if it is
> really needed

Added a comment explaining why it's there (actually the test does
not rely on this).


> http://codereview.chromium.org/387002



http://codereview.chromium.org/387002

[kinuko (google), 2009-11-16 06:31:19]

Updated the test_expectations.txt too as upstream patch has been landed.
Is it OK to submit?

On 2009/11/11 10:49:40, Kinuko Yasuda wrote:
> On 2009/11/11 08:08:30, Evan Martin wrote:
> > LGTM
> 
> 
> > http://codereview.chromium.org/387002/diff/3001/6
> > File webkit/tools/test_shell/event_sending_controller.cc (right):
> 
> > http://codereview.chromium.org/387002/diff/3001/6#newcode418
> > Line 418: webview()->layout();
> > why is this layout necessary?  maybe add a comment saying why if it is
> > really needed
> 
> Added a comment explaining why it's there (actually the test does
> not rely on this).
> 
> 
> > http://codereview.chromium.org/387002
> 
> 
> 
> http://codereview.chromium.org/387002

[Evan Martin, 2009-11-16 08:15:48]

LGTM, minor English suggestions follow

http://codereview.chromium.org/387002/diff/7001/9
File webkit/tools/test_shell/event_sending_controller.cc (right):

http://codereview.chromium.org/387002/diff/7001/9#newcode417
Line 417: // Forces a layout here just to make sure every positioning has been
Forces => Force
"every positioning" => "every position" or "positioning"

http://codereview.chromium.org/387002/diff/7001/9#newcode418
Line 418: // deteremined before we send events (as well as all the other methods
typo: "determined"

http://codereview.chromium.org/387002/diff/7001/9#newcode419
Line 419: // that send an event does). The layout test calling this
does => do  ("all" is plural)

[kinuko (google), 2009-11-16 08:28:15]

Thanks for reviewing! I've corrected the comment.

On 2009/11/16 08:15:48, Evan Martin wrote:
> LGTM, minor English suggestions follow
> 
> http://codereview.chromium.org/387002/diff/7001/9
> File webkit/tools/test_shell/event_sending_controller.cc (right):
> 
> http://codereview.chromium.org/387002/diff/7001/9#newcode417
> Line 417: // Forces a layout here just to make sure every positioning has been
> Forces => Force
> "every positioning" => "every position" or "positioning"
> 
> http://codereview.chromium.org/387002/diff/7001/9#newcode418
> Line 418: // deteremined before we send events (as well as all the other
methods
> typo: "determined"
> 
> http://codereview.chromium.org/387002/diff/7001/9#newcode419
> Line 419: // that send an event does). The layout test calling this
> does => do  ("all" is plural)

[tkent, 2009-11-17 06:07:06]

Committed as r32157.