mus: Fix handled status in UI event ack, add MessageLoop::NestingObserver

ui/views/mus/platform_window_mus.cc

Index: ui/views/mus/platform_window_mus.cc
diff --git a/ui/views/mus/platform_window_mus.cc b/ui/views/mus/platform_window_mus.cc
index 4b72979b40ce92857c8d88346fcdc320a7a2eb10..f1d58a37dabfda16ce900e8ed835e42ad868bd28 100644
--- a/ui/views/mus/platform_window_mus.cc
+++ b/ui/views/mus/platform_window_mus.cc
@@ -4,6 +4,7 @@
 
 #include "ui/views/mus/platform_window_mus.h"
 
+#include "base/message_loop/message_loop.h"
 #include "build/build_config.h"
 #include "components/bitmap_uploader/bitmap_uploader.h"
 #include "components/mus/public/cpp/property_type_converters.h"
@@ -14,11 +15,40 @@
 #include "ui/platform_window/platform_window_delegate.h"
 #include "ui/views/mus/window_manager_connection.h"
 
+using mus::mojom::EventResult;
+
 namespace views {
 
 namespace {
+
 static uint32_t accelerated_widget_count = 1;
 
+// Acknowledges an input event immediately if a nested message loop starts.
+// Needed because otherwise we appear unresponsive for the life of the nested
+// message loop.
+class EventAckNestingObserver : public base::MessageLoop::NestingObserver {
+ public:
+  explicit EventAckNestingObserver(
+      std::unique_ptr<base::Callback<void(EventResult)>>* ack_callback)

[sky, 2016/04/27 17:49:48]

Did you consider making this take a std::unique_ptr, not a unique_ptr* and have
the destructor run the callback if it wasn't run already? Seems a bit simpler
and no pointers being passed around that way.

I would also be inclined to make this class add/remove itself from the
messageloop.

[James Cook, 2016/04/27 18:11:14]

Done.

+      : ack_callback_(ack_callback) {
+    DCHECK(ack_callback_);
+  }
+  ~EventAckNestingObserver() override {}
+
+  // base::MessageLoop::NestingObserver:
+  void OnBeginNestedMessageLoop() override {
+    if (*ack_callback_) {
+      (*ack_callback_)->Run(EventResult::HANDLED);
+      ack_callback_->reset();  // Signal that the event was ack'd.
+    }
+  }
+
+ private:
+  std::unique_ptr<base::Callback<void(EventResult)>>* ack_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(EventAckNestingObserver);
+};
+
 }  // namespace
 
 PlatformWindowMus::PlatformWindowMus(ui::PlatformWindowDelegate* delegate,
@@ -222,17 +252,29 @@ void PlatformWindowMus::OnRequestClose(mus::Window* window) {
 
 void PlatformWindowMus::OnWindowInputEvent(
     mus::Window* view,
-    const ui::Event& event,
-    std::unique_ptr<base::Callback<void(mus::mojom::EventResult)>>*
-        ack_callback) {
-  // It's possible dispatching the event will spin a nested message loop. Ack
-  // the callback now, otherwise we appear unresponsive for the life of the
-  // nested message loop.
-  (*ack_callback)->Run(mus::mojom::EventResult::HANDLED);
-  ack_callback->reset();
-  // TODO(moshayedi): Avoid cloning after updating PlatformWindowDelegate to
-  // accept constant pointers.
-  delegate_->DispatchEvent(ui::Event::Clone(event).get());
+    const ui::Event& event_in,
+    std::unique_ptr<base::Callback<void(EventResult)>>* ack_callback_in) {
+  // Take ownership of the callback, indicating that we will handle it.
+  std::unique_ptr<base::Callback<void(EventResult)>> ack_callback =
+      std::move(*ack_callback_in);
+
+  // If dispatching the event creates a nested message loop we need to ack the
+  // event immediately. There might be multiple nested message loops; each one
+  // needs its own observer.
+  EventAckNestingObserver nesting_observer(&ack_callback);
+  base::MessageLoop::current()->AddNestingObserver(&nesting_observer);
+
+  std::unique_ptr<ui::Event> event = ui::Event::Clone(event_in);
+  delegate_->DispatchEvent(event.get());
+  // NOTE: |this| may be deleted.
+
+  base::MessageLoop::current()->RemoveNestingObserver(&nesting_observer);
+
+  // If the callback wasn't already called by the message loop observer then
+  // ack it with the handled state.
+  if (ack_callback)
+    ack_callback->Run(event->handled() ? EventResult::HANDLED
+                                       : EventResult::UNHANDLED);
 }
 
 }  // namespace views