Update WindowTree::OnWindowInputEventAck to include handled

Update WindowTree::OnWindowInputEventAck to include handled

To support post-target accelerators we plan to have the client application report whether an input event was handled or not, when ack-ing the event.

This change updates WindowTree::OnWindowInputEventAck to include a boolean parameter |handled|.

PlatformWindowMus will always mark events as handled. CompositorMusConnection will base the handled state on the ack state of the renderer.

TEST=WindowTreeClientImplTest, WindowTreeApptest, WindowTreeTest, CompositorMusConnectionTest
BUG=560478
Committed: https://crrev.com/c3385cc179280dd4b09733b2954be7e9ae3b5112
Cr-Commit-Position: refs/heads/master@{#379821}

[jonross, 2016-03-01 19:46:48]

jonross@chromium.org changed reviewers:
+ sky@chromium.org

[jonross, 2016-03-01 19:46:49]

Hi,

I've prepared this patch to expand WindowTree::OnWindowInputEventAck to include
a boolean for if the event was handled.

A future review will use this to decide to notify post-target accelerators.

PTAL.

Thanks,
Jon

[sky, 2016-03-01 23:08:17]

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/i...
File components/mus/public/cpp/input_event_handler.h (right):

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/i...
components/mus/public/cpp/input_event_handler.h:24:
scoped_ptr<base::Callback<void(bool)>>* ack_callback) = 0;
Document what the bool means.

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/l...
File components/mus/public/cpp/lib/window_tree_client_impl.cc (right):

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/l...
components/mus/public/cpp/lib/window_tree_client_impl.cc:834:
ack_callback->Run(false);
Why does this supply false?

[jonross, 2016-03-01 23:26:16]

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/l...
File components/mus/public/cpp/lib/window_tree_client_impl.cc (right):

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/l...
components/mus/public/cpp/lib/window_tree_client_impl.cc:834:
ack_callback->Run(false);
On 2016/03/01 23:08:17, sky wrote:
> Why does this supply false?

There is an undocumented expectation on:
  window->input_event_handler_->OnWindowInputEvent
That if the call location acks the event that it clears the scoped_ptr.

This call is a fallback to make sure the server gets an ack for the event in
case the handler didn't ack. I went with false, as an input_event_handler should
ack with 'true' if handled.

I could document this expectation, or it might be be more clearer if
OnWindowInputEvent returned a bool for if it sent an ack.

[sky, 2016-03-02 03:32:40]

Got it. I like the current behavior in that it seems less prone. How about
documenting it?

LGTM

[jonross, 2016-03-02 15:23:24]

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/i...
File components/mus/public/cpp/input_event_handler.h (right):

https://codereview.chromium.org/1749323002/diff/1/components/mus/public/cpp/i...
components/mus/public/cpp/input_event_handler.h:24:
scoped_ptr<base::Callback<void(bool)>>* ack_callback) = 0;
On 2016/03/01 23:08:17, sky wrote:
> Document what the bool means.

Done.

[jonross, 2016-03-02 15:24:34]

jonross@chromium.org changed reviewers:
+ tdresser@chromium.org

[jonross, 2016-03-02 15:24:35]

tdresser@chromium.org: Please review changes in 

content/renderer/input/*

As discussed offline I've made parts of these classes virtual so that I can
extend them within unittests

[jonross, 2016-03-02 15:25:57]

jonross@chromium.org changed reviewers:
+ ben@chromium.org

[jonross, 2016-03-02 15:25:57]

Hi Ben,

Could you provide an owners review of the changes in: 	 content/renderer/mus/* ?

I'm updating InputEventHandler::OnWindowInputEvent so that the callback takes a
boolean for if it was consumed.

Thanks,
Jon

[tdresser, 2016-03-02 16:04:41]

content/renderer/input/ LGTM

[jonross, 2016-03-04 15:47:37]

Hi Ben,

Could you provide an owner's review of the changes in: 	 content/renderer/mus/*
?

Thanks,
Jon

[Ben Goodger (Google), 2016-03-04 20:24:14]

lgtm

[jonross, 2016-03-04 20:36:26]

The CQ bit was checked by jonross@chromium.org

[jonross, 2016-03-04 20:36:27]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1749323002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-03-04 20:37:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1749323002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1749323002/20001

[commit-bot: I haz the power, 2016-03-04 20:40:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-04 20:40:30]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jonross, 2016-03-07 16:38:53]

The CQ bit was checked by jonross@chromium.org

[jonross, 2016-03-07 16:38:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, ben@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1749323002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-03-07 16:39:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1749323002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1749323002/40001

[commit-bot: I haz the power, 2016-03-07 17:15:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-07 17:15:45]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jonross, 2016-03-08 00:16:19]

The CQ bit was checked by jonross@chromium.org

[jonross, 2016-03-08 00:16:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org, ben@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1749323002/#ps60001
(title: "Missing Component Build DEPS")

[commit-bot: I haz the power, 2016-03-08 00:16:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1749323002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1749323002/60001

[commit-bot: I haz the power, 2016-03-08 02:55:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 02:56:00]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jonross, 2016-03-08 12:56:01]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2016-03-08 12:56:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1749323002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1749323002/60001

[commit-bot: I haz the power, 2016-03-08 14:03:36]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-08 14:04:50]

Description was changed from

==========
Update WindowTree::OnWindowInputEventAck to include handled

To support post-target accelerators we plan to have the client application
report whether an input event was handled or not, when ack-ing the event.

This change updates WindowTree::OnWindowInputEventAck to include a boolean
parameter |handled|.

PlatformWindowMus will always mark events as handled. CompositorMusConnection
will base the handled state on the ack state of the renderer.

TEST=WindowTreeClientImplTest, WindowTreeApptest, WindowTreeTest,
CompositorMusConnectionTest
BUG=560478
==========

to

==========
Update WindowTree::OnWindowInputEventAck to include handled

To support post-target accelerators we plan to have the client application
report whether an input event was handled or not, when ack-ing the event.

This change updates WindowTree::OnWindowInputEventAck to include a boolean
parameter |handled|.

PlatformWindowMus will always mark events as handled. CompositorMusConnection
will base the handled state on the ack state of the renderer.

TEST=WindowTreeClientImplTest, WindowTreeApptest, WindowTreeTest,
CompositorMusConnectionTest
BUG=560478

Committed: https://crrev.com/c3385cc179280dd4b09733b2954be7e9ae3b5112
Cr-Commit-Position: refs/heads/master@{#379821}
==========

[commit-bot: I haz the power, 2016-03-08 14:04:52]

Patchset 4 (id:??) landed as
https://crrev.com/c3385cc179280dd4b09733b2954be7e9ae3b5112
Cr-Commit-Position: refs/heads/master@{#379821}

[Nico, 2016-03-08 14:59:14]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-03-08 14:59:15]

I believe this broke component builds on Windows with GN, see e.g.
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dbg%29/buil...

Please take a look, and if it can't be fixed quickly, please revert.

[jonross, 2016-03-08 15:07:15]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1774973002/ by jonross@chromium.org.

The reason for reverting is: This broke the Windows component build.

https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dbg%29/buil....