Update EventSender::GestureTap to return a bool (if handled).

Update EventSender::GestureTap to return a bool (if handled).

BUG=355154
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274357

[donnd, 2014-05-29 04:45:51]

Rick, PTAL at this update.  I'm unsure about the return values under error
conditions, and also when a drag has been undertaken.

Regarding running the tests, there's good news and bad news.  Jared pointed me
to the "cr" tool, so I can easily build and run linux from the same code base as
my Clank development.  Yea!  The bad news is I still can't run the tests, so the
only verification this CL has had is compiling.  I'll hopefully address that
tomorrow.

[Rick Byers, 2014-05-29 20:57:47]

Seems reasonable overall - we'll just want to test that it actually works as
intended with a LayoutTest before landing...

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/event_sender.cc (right):

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:748: return false; //
TODO(donnd): true?
yeah this is goofy (should probably throw instead of silently return), but don't
worry about it - just returning false is fine.

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1695: bool
EventSender::GestureTap(gin::Arguments* args) {
I'm new to gin (gin is a new lightweight javascript<->C++ bindings system) but
it looks like the right way to set a return value to JS in this case is probably
with args->Return.  Maybe your direct way will automatically work too, and if so
that's fine but you'll need to test.  If not then try args->Return(...) to set
the return value.

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1932: return false;
If the right way to set a return value to JS is args->Return then all these
extra statements can go away...

[donnd, 2014-05-31 03:14:57]

That args->Return(bool) worked!  Now this whole CL is just a few lines.

I still can't run the Layout tests on Android, but I've verified it works on
Linux, and I can't imagine this change would cause trouble on other platforms.

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/event_sender.cc (right):

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:748: return false; //
TODO(donnd): true?
On 2014/05/29 20:57:47, Rick Byers wrote:
> yeah this is goofy (should probably throw instead of silently return), but
don't
> worry about it - just returning false is fine.

Done.

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1695: bool
EventSender::GestureTap(gin::Arguments* args) {
On 2014/05/29 20:57:47, Rick Byers wrote:
> I'm new to gin (gin is a new lightweight javascript<->C++ bindings system) but
> it looks like the right way to set a return value to JS in this case is
probably
> with args->Return.  Maybe your direct way will automatically work too, and if
so
> that's fine but you'll need to test.  If not then try args->Return(...) to set
> the return value.

Done.

https://codereview.chromium.org/301103002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/event_sender.cc:1932: return false;
On 2014/05/29 20:57:47, Rick Byers wrote:
> If the right way to set a return value to JS is args->Return then all these
> extra statements can go away...

Done.

[Rick Byers, 2014-05-31 03:23:39]

LGTM

> I still can't run the Layout tests on Android, but I've verified it works on
> Linux, and I can't imagine this change would cause trouble on other platforms.

Yep running your tests just on Linux (and passing the bots once you start using
this) is enough.

[Rick Byers, 2014-05-31 03:24:27]

+jochen for OWNERS

[jochen (gone - plz use gerrit), 2014-06-02 07:04:06]

lgtm

[donnd, 2014-06-02 17:07:02]

The CQ bit was checked by donnd@google.com

[commit-bot: I haz the power, 2014-06-02 17:08:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/donnd@chromium.org/301103002/20001

[commit-bot: I haz the power, 2014-06-02 19:53:23]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-02 22:36:38]

Change committed as 274357