Fix the wrong non-element node handling in EventHanlder and MouseEventManager

third_party/WebKit/Source/core/input/GestureManager.cpp

Index: third_party/WebKit/Source/core/input/GestureManager.cpp
diff --git a/third_party/WebKit/Source/core/input/GestureManager.cpp b/third_party/WebKit/Source/core/input/GestureManager.cpp
index cf16ec673be0a424729e5399b0516fef9380a62f..fc5e77e15e04f627eb85c484621056606348a958 100644
--- a/third_party/WebKit/Source/core/input/GestureManager.cpp
+++ b/third_party/WebKit/Source/core/input/GestureManager.cpp
@@ -185,17 +185,17 @@ WebInputEventResult GestureManager::HandleGestureTap(
   }
 
   // Capture data for showUnhandledTapUIIfNeeded.
-  Node* tapped_node = current_hit_test.InnerNode();
   IntPoint tapped_position =
       FlooredIntPoint(gesture_event.PositionInRootFrame());
-  Node* tapped_non_text_node = tapped_node;
   UserGestureIndicator gesture_indicator(DocumentUserGestureToken::Create(
-      tapped_node ? &tapped_node->GetDocument() : nullptr));
-
-  if (tapped_non_text_node && tapped_non_text_node->IsTextNode())
-    tapped_non_text_node = FlatTreeTraversal::Parent(*tapped_non_text_node);
+      current_hit_test.InnerNode()
+          ? &current_hit_test.InnerNode()->GetDocument()
+          : nullptr));
 
-  mouse_event_manager_->SetClickNode(tapped_non_text_node);
+  Node* tapped_node = current_hit_test.InnerNode();
+  Element* tapped_element =
+      EventHandlingUtil::ParentElementIfNeeded(tapped_node);
+  mouse_event_manager_->SetClickElement(tapped_element);
 
   WebMouseEvent fake_mouse_down(
       WebInputEvent::kMouseDown, gesture_event,
@@ -262,7 +262,7 @@ WebInputEventResult GestureManager::HandleGestureTap(
                 EventTypeNames::mouseup, fake_mouse_up);
 
   WebInputEventResult click_event_result = WebInputEventResult::kNotHandled;
-  if (tapped_non_text_node) {
+  if (tapped_element) {
     if (current_hit_test.InnerNode()) {
       // Updates distribution because a mouseup (or mousedown) event listener
       // can make the tree dirty at dispatchMouseEvent() invocation above.
@@ -270,15 +270,15 @@ WebInputEventResult GestureManager::HandleGestureTap(
       // tappedNonTextNode and currentHitTest.innerNode()) don't need to be
       // updated because commonAncestor() will exit early if their documents are
       // different.
-      tapped_non_text_node->UpdateDistribution();
+      tapped_element->UpdateDistribution();
       Node* click_target_node = current_hit_test.InnerNode()->CommonAncestor(
-          *tapped_non_text_node, EventHandlingUtil::ParentForClickEvent);
+          *tapped_element, EventHandlingUtil::ParentForClickEvent);
       click_event_result =
           mouse_event_manager_->SetMousePositionAndDispatchMouseEvent(
               click_target_node, String(), EventTypeNames::click,
               fake_mouse_up);
     }
-    mouse_event_manager_->SetClickNode(nullptr);
+    mouse_event_manager_->SetClickElement(nullptr);
   }
 
   if (mouse_up_event_result == WebInputEventResult::kNotHandled)
@@ -290,7 +290,7 @@ WebInputEventResult GestureManager::HandleGestureTap(
       EventHandlingUtil::MergeEventResult(mouse_down_event_result,
                                           mouse_up_event_result),
       click_event_result);
-  if (event_result == WebInputEventResult::kNotHandled && tapped_node &&
+  if (event_result == WebInputEventResult::kNotHandled && tapped_element &&

[Donn Denman, 2017/04/12 17:12:19]

Nit: I wonder if it would be better to test the tapped_node here instead of the
tapped_element since that's what we're going to pass to the client.  I suppose
it's not possible for one to be null without the other right now, but seems
better and cleaner the other way.

[hayato, 2017/04/13 05:01:40]

Nice catch. tapped_node should be better.
Done.

       frame_->GetPage()) {
     bool dom_tree_changed = pre_dispatch_dom_tree_version !=
                             frame_->GetDocument()->DomTreeVersion();