Fix EventDispatcherDelegate::DispatchEvent() to correctly set EventDispatchDetails::target_destroyed

BUG=342867
TEST=EventDispatcherTest.*

Review URL: https://codereview.chromium.org/169963005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253459 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 44 insertions, 35 deletions

diff --git a/ui/aura/window_event_dispatcher.cc b/ui/aura/window_event_dispatcher.cc
index 3fbfdba..cfc43c1 100644
--- a/ui/aura/window_event_dispatcher.cc
+++ b/ui/aura/window_event_dispatcher.cc
@@ -578,7 +578,7 @@ ui::EventDispatchDetails WindowEventDispatcher::PostDispatchEvent(
     ui::EventTarget* target,
     const ui::Event& event) {
   DispatchDetails details;
-  if (target != event_dispatch_target_)
+  if (!target || target != event_dispatch_target_)
     details.target_destroyed = true;
   event_dispatch_target_ = old_dispatch_target_;
   old_dispatch_target_ = NULL;
diff --git a/ui/events/event_dispatcher.cc b/ui/events/event_dispatcher.cc
index a1fbafa..ca24cde 100644
--- a/ui/events/event_dispatcher.cc
+++ b/ui/events/event_dispatcher.cc
@@ -51,11 +51,18 @@ EventDispatchDetails EventDispatcherDelegate::DispatchEvent(EventTarget* target,
   dispatch_helper.set_result(ER_UNHANDLED);
 
   EventDispatchDetails details = PreDispatchEvent(target, event);
-  if (!event->handled() && !details.dispatcher_destroyed)
+  if (!event->handled() &&
+      !details.dispatcher_destroyed &&
+      !details.target_destroyed) {
     details = DispatchEventToTarget(target, event);
-  if (!details.dispatcher_destroyed)
-    details = PostDispatchEvent(target, *event);
+  }
+  bool target_destroyed_during_dispatch = details.target_destroyed;
+  if (!details.dispatcher_destroyed) {
+    details = PostDispatchEvent(target_destroyed_during_dispatch ?
+        NULL : target, *event);
+  }
 
+  details.target_destroyed |= target_destroyed_during_dispatch;
   return details;
 }
 
@@ -81,7 +88,11 @@ EventDispatchDetails EventDispatcherDelegate::DispatchEventToTarget(
   else if (old_dispatcher)
     old_dispatcher->OnDispatcherDelegateDestroyed();
 
-  return dispatcher.details();
+  EventDispatchDetails details;
+  details.dispatcher_destroyed = dispatcher.delegate_destroyed();
+  details.target_destroyed =
+      (!details.dispatcher_destroyed && !CanDispatchToTarget(target));
+  return details;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -128,14 +139,9 @@ void EventDispatcher::ProcessEvent(EventTarget* target, Event* event) {
       return;
   }
 
-  if (!delegate_)
+  if (!delegate_ || !delegate_->CanDispatchToTarget(target))
     return;
 
-  if (!delegate_->CanDispatchToTarget(target)) {
-    details_.target_destroyed = true;
-    return;
-  }
-
   handler_list_.clear();
   target->GetPostTargetHandlers(&handler_list_);
   dispatch_helper.set_phase(EP_POSTTARGET);
@@ -143,7 +149,6 @@ void EventDispatcher::ProcessEvent(EventTarget* target, Event* event) {
 }
 
 void EventDispatcher::OnDispatcherDelegateDestroyed() {
-  details_.dispatcher_destroyed = true;
   delegate_ = NULL;
 }
 
@@ -175,7 +180,6 @@ void EventDispatcher::DispatchEventToEventHandlers(EventHandlerList* list,
 void EventDispatcher::DispatchEvent(EventHandler* handler, Event* event) {
   // If the target has been invalidated or deleted, don't dispatch the event.
   if (!delegate_->CanDispatchToTarget(event->target())) {
-    details_.target_destroyed = true;
     if (event->cancelable())
       event->StopPropagation();
     return;
diff --git a/ui/events/event_dispatcher.h b/ui/events/event_dispatcher.h
index 9f3a450..a8c985e 100644
--- a/ui/events/event_dispatcher.h
+++ b/ui/events/event_dispatcher.h
@@ -54,6 +54,7 @@ class EVENTS_EXPORT EventDispatcherDelegate {
       Event* event) WARN_UNUSED_RESULT;
 
   // This is called right after the event dispatch is completed.
+  // |target| is NULL if the target was deleted during dispatch.
   virtual EventDispatchDetails PostDispatchEvent(
       EventTarget* target,
       const Event& event) WARN_UNUSED_RESULT;
@@ -80,7 +81,6 @@ class EVENTS_EXPORT EventDispatcher {
   Event* current_event() { return current_event_; }
 
   bool delegate_destroyed() const { return !delegate_; }
-  const EventDispatchDetails& details() const { return details_; }
 
   void OnHandlerDestroyed(EventHandler* handler);
   void OnDispatcherDelegateDestroyed();
@@ -99,8 +99,6 @@ class EVENTS_EXPORT EventDispatcher {
 
   EventHandlerList handler_list_;
 
-  EventDispatchDetails details_;
-
   DISALLOW_COPY_AND_ASSIGN(EventDispatcher);
 };
 
diff --git a/ui/events/event_dispatcher_unittest.cc b/ui/events/event_dispatcher_unittest.cc
index 3b9f1db..248784a 100644
--- a/ui/events/event_dispatcher_unittest.cc
+++ b/ui/events/event_dispatcher_unittest.cc
@@ -209,10 +209,8 @@ class TestEventDispatcher : public EventDispatcherDelegate {
 
   virtual ~TestEventDispatcher() {}
 
-  void ProcessEvent(EventTarget* target, Event* event) {
-    EventDispatchDetails details = DispatchEvent(target, event);
-    if (details.dispatcher_destroyed)
-      return;
+  EventDispatchDetails ProcessEvent(EventTarget* target, Event* event) {
+    return DispatchEvent(target, event);
   }
 
  private:
@@ -365,8 +363,8 @@ TEST(EventDispatcherTest, EventDispatcherDestroyedDuringDispatch) {
 
     MouseEvent mouse(ui::ET_MOUSE_MOVED, gfx::Point(3, 4),
                      gfx::Point(3, 4), 0, 0);
-    Event::DispatcherApi event_mod(&mouse);
-    dispatcher->ProcessEvent(&target, &mouse);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &mouse);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_EQ(ER_CONSUMED, mouse.result());
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
@@ -391,8 +389,8 @@ TEST(EventDispatcherTest, EventDispatcherDestroyedDuringDispatch) {
     h2.set_expect_pre_target(false);
 
     NonCancelableEvent event;
-    Event::DispatcherApi event_mod(&event);
-    dispatcher->ProcessEvent(&target, &event);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &event);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
     EXPECT_EQ(5, target.handler_list()[1]);
@@ -417,8 +415,8 @@ TEST(EventDispatcherTest, EventDispatcherDestroyedDuringDispatch) {
 
     MouseEvent mouse(ui::ET_MOUSE_MOVED, gfx::Point(3, 4),
                      gfx::Point(3, 4), 0, 0);
-    Event::DispatcherApi event_mod(&mouse);
-    dispatcher->ProcessEvent(&target, &mouse);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &mouse);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_EQ(ER_CONSUMED, mouse.result());
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
@@ -443,8 +441,8 @@ TEST(EventDispatcherTest, EventDispatcherDestroyedDuringDispatch) {
     h2.set_expect_post_target(false);
 
     NonCancelableEvent event;
-    Event::DispatcherApi event_mod(&event);
-    dispatcher->ProcessEvent(&target, &event);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &event);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
     EXPECT_EQ(5, target.handler_list()[1]);
@@ -471,7 +469,9 @@ TEST(EventDispatcherTest, EventDispatcherInvalidateTarget) {
 
   MouseEvent mouse(ui::ET_MOUSE_MOVED, gfx::Point(3, 4), gfx::Point(3, 4), 0,
                    0);
-  dispatcher.ProcessEvent(&target, &mouse);
+  EventDispatchDetails details = dispatcher.ProcessEvent(&target, &mouse);
+  EXPECT_FALSE(details.dispatcher_destroyed);
+  EXPECT_TRUE(details.target_destroyed);
   EXPECT_FALSE(target.valid());
   EXPECT_TRUE(mouse.stopped_propagation());
   EXPECT_EQ(2U, target.handler_list().size());
@@ -481,9 +481,10 @@ TEST(EventDispatcherTest, EventDispatcherInvalidateTarget) {
   // Test for non-cancelable event.
   target.Reset();
   NonCancelableEvent event;
-  dispatcher.ProcessEvent(&target, &event);
+  details = dispatcher.ProcessEvent(&target, &event);
+  EXPECT_FALSE(details.dispatcher_destroyed);
+  EXPECT_TRUE(details.target_destroyed);
   EXPECT_FALSE(target.valid());
-  EXPECT_TRUE(mouse.stopped_propagation());
   EXPECT_EQ(2U, target.handler_list().size());
   EXPECT_EQ(1, target.handler_list()[0]);
   EXPECT_EQ(2, target.handler_list()[1]);
@@ -511,7 +512,9 @@ TEST(EventDispatcherTest, EventHandlerDestroyedDuringDispatch) {
 
     MouseEvent mouse(ui::ET_MOUSE_MOVED, gfx::Point(3, 4), gfx::Point(3, 4), 0,
                      0);
-    dispatcher.ProcessEvent(&target, &mouse);
+    EventDispatchDetails details = dispatcher.ProcessEvent(&target, &mouse);
+    EXPECT_FALSE(details.dispatcher_destroyed);
+    EXPECT_FALSE(details.target_destroyed);
     EXPECT_FALSE(mouse.stopped_propagation());
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
@@ -535,7 +538,9 @@ TEST(EventDispatcherTest, EventHandlerDestroyedDuringDispatch) {
     h3->set_expect_pre_target(false);
 
     NonCancelableEvent event;
-    dispatcher.ProcessEvent(&target, &event);
+    EventDispatchDetails details = dispatcher.ProcessEvent(&target, &event);
+    EXPECT_FALSE(details.dispatcher_destroyed);
+    EXPECT_FALSE(details.target_destroyed);
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
     EXPECT_EQ(2, target.handler_list()[1]);
@@ -565,7 +570,8 @@ TEST(EventDispatcherTest, EventHandlerAndDispatcherDestroyedDuringDispatch) {
 
     MouseEvent mouse(ui::ET_MOUSE_MOVED, gfx::Point(3, 4), gfx::Point(3, 4), 0,
                      0);
-    dispatcher->ProcessEvent(&target, &mouse);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &mouse);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_TRUE(mouse.stopped_propagation());
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
@@ -592,7 +598,8 @@ TEST(EventDispatcherTest, EventHandlerAndDispatcherDestroyedDuringDispatch) {
     h3->set_expect_pre_target(false);
 
     NonCancelableEvent event;
-    dispatcher->ProcessEvent(&target, &event);
+    EventDispatchDetails details = dispatcher->ProcessEvent(&target, &event);
+    EXPECT_TRUE(details.dispatcher_destroyed);
     EXPECT_EQ(2U, target.handler_list().size());
     EXPECT_EQ(1, target.handler_list()[0]);
     EXPECT_EQ(2, target.handler_list()[1]);