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

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

Index: third_party/WebKit/Source/core/input/MouseEventManager.cpp
diff --git a/third_party/WebKit/Source/core/input/MouseEventManager.cpp b/third_party/WebKit/Source/core/input/MouseEventManager.cpp
index ea204d0016d5ff04befd546cf3e5d6fee1e68c65..2bcfcdc8f6cda002675d3fbeb66e368beedc4c9a 100644
--- a/third_party/WebKit/Source/core/input/MouseEventManager.cpp
+++ b/third_party/WebKit/Source/core/input/MouseEventManager.cpp
@@ -258,13 +258,16 @@ WebInputEventResult MouseEventManager::DispatchMouseClickIfNeeded(
         IsLinkSelection(mev));
   if (should_dispatch_click_event) {
     Node* click_target_node = nullptr;
-    // Updates distribution because a 'mouseup' event listener can make the
-    // tree dirty at dispatchMouseEvent() invocation above.
-    // Unless distribution is updated, commonAncestor would hit ASSERT.
-    if (click_node_ == mev.InnerNode()) {
+    // A click event should be fired even when a text node has been removed in
+    // mouseup event.
+    if ((mev.InnerNode()->IsTextNode() && click_node_->isConnected() &&

[tkent, 2017/04/11 04:16:14]

I think this code has the following unexpected behavior.

1. Open fast/events/remove-text-node-in-mouseup.html
2. Mousedown on a Text child of the first <p> element
3. Move mouse cursor onto the second <div> element
4. Mouseup 

In this case, click_node_ is <p>, mev.InnerNode() is the removed Text node, and
this code will dispatch 'click' on <p>.

[tkent, 2017/04/11 04:24:15]

Maybe a logic would be:

Node* mouseup_node = mev.InnerNode()->isTextNode() ?
mev.InnerNode()->parentNode() : mev.InnerNode();
if (click_node_ == mouseup_node) {
  ...

+         !mev.InnerNode()->isConnected()) ||

[kochi, 2017/04/11 04:25:00]

(I am not sure and just checking) - Isn't click_node_->isConnected()
already guaranteed somewhere above?  (I vaguely assumed that
click_node_->isConnected() is implied in |should_dispatch_click_event|,
but it seems not.)
If click_node_ is not connected, still click event should be dispatched
to it?

For readability, I prefer the following order:
 "mev.InnerNode()->IsTextNode() && !mev.InnerNode()->isConnected() &&
  click_node_->isConnected()"
but it's up to you.

+        click_node_ == mev.InnerNode()) {

[kochi, 2017/04/11 04:25:00]

I'd guess "click_node_ == mev.InnerNode()" is the more common case,
so in that case the condition may be better checked first?

       click_target_node = click_node_;
-      click_target_node->UpdateDistribution();
     } else if (click_node_->GetDocument() == mev.InnerNode()->GetDocument()) {
+      // Updates distribution because a 'mouseup' event listener can make the
+      // tree dirty at dispatchMouseEvent() invocation above.
+      // Unless distribution is updated, commonAncestor would hit ASSERT.
       click_node_->UpdateDistribution();
       mev.InnerNode()->UpdateDistribution();
       click_target_node = mev.InnerNode()->CommonAncestor(