Fixes bug in accelerator handling in mus

Fixes bug in accelerator handling in mus

Here's the problematic case:
. Add a post accelerator.
. Event comes in matching accelerator.
* Target window responds with handled.
. Different event comes in.
. Target window response with unhandled.
. Accelerator matching earlier event is notified.

The accelerator should not have been notified at that last
step. Things went wrong at * because post_target_accelerator_ was not
getting reset.

BUG=612331
TEST=covered by tests
R=sadrul@chromium.org

Committed: https://crrev.com/99106ee359c78c25120afe69e2d6964a3cd8a593
Cr-Commit-Position: refs/heads/master@{#408170}

[sky, 2016-07-26 22:02:31]

The CQ bit was checked by sky@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 22:03:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 22:55:12]

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

[commit-bot: I haz the power, 2016-07-26 22:55:12]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-07-27 16:31:47]

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_state.cc (right):

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_state.cc:222: std::unique_ptr<ui::Event> event =
std::move(event_awaiting_input_ack_);
Should we always reset |event_awaiting_input_ack_| too?

[sky, 2016-07-27 16:34:49]

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_state.cc (right):

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_state.cc:222: std::unique_ptr<ui::Event> event =
std::move(event_awaiting_input_ack_);
On 2016/07/27 16:31:47, sadrul wrote:
> Should we always reset |event_awaiting_input_ack_| too?

Isn't that what the std::move does? Or did you mean something else?

[sadrul, 2016-07-27 16:36:01]

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_state.cc (right):

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_state.cc:222: std::unique_ptr<ui::Event> event =
std::move(event_awaiting_input_ack_);
On 2016/07/27 16:34:49, sky wrote:
> On 2016/07/27 16:31:47, sadrul wrote:
> > Should we always reset |event_awaiting_input_ack_| too?
> 
> Isn't that what the std::move does? Or did you mean something else?

Yep. I mean, should we do that even if result != UNHANDLED?

[sky, 2016-07-27 16:45:34]

The CQ bit was checked by sky@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 16:45:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sky, 2016-07-27 16:46:01]

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_state.cc (right):

https://codereview.chromium.org/2184613005/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_state.cc:222: std::unique_ptr<ui::Event> event =
std::move(event_awaiting_input_ack_);
On 2016/07/27 16:36:01, sadrul wrote:
> On 2016/07/27 16:34:49, sky wrote:
> > On 2016/07/27 16:31:47, sadrul wrote:
> > > Should we always reset |event_awaiting_input_ack_| too?
> > 
> > Isn't that what the std::move does? Or did you mean something else?
> 
> Yep. I mean, should we do that even if result != UNHANDLED?

Ah, I got it. It doesn't matter because of ProcessNextEventFromQueue. But I do
agree, it's safer to reset always. So, updated.

[sadrul, 2016-07-27 16:48:04]

lgtm

[sky, 2016-07-27 16:49:54]

The CQ bit was unchecked by sky@chromium.org

[sky, 2016-07-27 16:50:00]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-07-27 16:50:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 17:41:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-27 17:43:32]

Description was changed from

==========
Fixes bug in accelerator handling in mus

Here's the problematic case:
. Add a post accelerator.
. Event comes in matching accelerator.
* Target window responds with handled.
. Different event comes in.
. Target window response with unhandled.
. Accelerator matching earlier event is notified.

The accelerator should not have been notified at that last
step. Things went wrong at * because post_target_accelerator_ was not
getting reset.

BUG=612331
TEST=covered by tests
R=sadrul@chromium.org
==========

to

==========
Fixes bug in accelerator handling in mus

Here's the problematic case:
. Add a post accelerator.
. Event comes in matching accelerator.
* Target window responds with handled.
. Different event comes in.
. Target window response with unhandled.
. Accelerator matching earlier event is notified.

The accelerator should not have been notified at that last
step. Things went wrong at * because post_target_accelerator_ was not
getting reset.

BUG=612331
TEST=covered by tests
R=sadrul@chromium.org

Committed: https://crrev.com/99106ee359c78c25120afe69e2d6964a3cd8a593
Cr-Commit-Position: refs/heads/master@{#408170}
==========

[commit-bot: I haz the power, 2016-07-27 17:43:33]

Patchset 3 (id:??) landed as
https://crrev.com/99106ee359c78c25120afe69e2d6964a3cd8a593
Cr-Commit-Position: refs/heads/master@{#408170}