Fixes bug in accelerator handling in mus

services/ui/ws/window_manager_state.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "services/ui/ws/window_manager_state.h"
 
 #include "base/memory/weak_ptr.h"
 #include "services/shell/public/interfaces/connector.mojom.h"
 #include "services/ui/common/event_matcher_util.h"
 #include "services/ui/ws/accelerator.h"
@@ skipping 197 matching lines @@
                                     mojom::EventResult result) {
   if (tree_awaiting_input_ack_ != tree ||
       event_dispatch_phase_ != EventDispatchPhase::TARGET) {
     // TODO(sad): The ack must have arrived after the timeout. We should do
     // something here, and in OnEventAckTimeout().
     return;
   }
   tree_awaiting_input_ack_ = nullptr;
   event_ack_timer_.Stop();
 
-  if (result == mojom::EventResult::UNHANDLED && post_target_accelerator_) {
-    OnAccelerator(post_target_accelerator_->id(), *event_awaiting_input_ack_,
+  base::WeakPtr<Accelerator> post_target_accelerator = post_target_accelerator_;
+  post_target_accelerator_.reset();
+
+  if (result == mojom::EventResult::UNHANDLED && post_target_accelerator) {
+    std::unique_ptr<ui::Event> event = std::move(event_awaiting_input_ack_);

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

Should we always reset |event_awaiting_input_ack_| too?

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

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

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

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

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

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

+    OnAccelerator(post_target_accelerator->id(), *event,
                   AcceleratorPhase::POST);
   }
 
   event_dispatch_phase_ = EventDispatchPhase::NONE;
   ProcessNextEventFromQueue();
 }
 
 void WindowManagerState::OnAcceleratorAck(mojom::EventResult result) {
   if (event_dispatch_phase_ != EventDispatchPhase::PRE_TARGET_ACCELERATOR) {
     // TODO(sad): The ack must have arrived after the timeout. We should do
     // something here, and in OnEventAckTimeout().
     return;
   }
 
   tree_awaiting_input_ack_ = nullptr;
   event_ack_timer_.Stop();
   event_dispatch_phase_ = EventDispatchPhase::NONE;
 
   if (result == mojom::EventResult::UNHANDLED) {
+    std::unique_ptr<ui::Event> event = std::move(event_awaiting_input_ack_);
     event_dispatcher_.ProcessEvent(
-        *event_awaiting_input_ack_,
-        EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
+        *event, EventDispatcher::AcceleratorMatchPhase::POST_ONLY);
   } else {
     // We're not going to process the event any further, notify event observers.
     // We don't do this first to ensure we don't send an event twice to clients.
-    window_server()->SendToEventObservers(*event_awaiting_input_ack_, user_id(),
-                                          nullptr);
+    std::unique_ptr<ui::Event> event = std::move(event_awaiting_input_ack_);
+    window_server()->SendToEventObservers(*event, user_id(), nullptr);
     ProcessNextEventFromQueue();
   }
 }
 
 const WindowServer* WindowManagerState::window_server() const {
   return window_tree_->window_server();
 }
 
 WindowServer* WindowManagerState::window_server() {
   return window_tree_->window_server();
@@ skipping 269 matching lines @@
   return display_root ? display_root->root() : nullptr;
 }
 
 void WindowManagerState::OnEventTargetNotFound(const ui::Event& event) {
   window_server()->SendToEventObservers(event, user_id(),
                                         nullptr /* ignore_tree */);
 }
 
 }  // namespace ws
 }  // namespace ui