Use the flat tree for hover handling.

third_party/WebKit/Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All
  * rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
@@ skipping 6127 matching lines @@
       !m_snapCoordinator)
     m_snapCoordinator = SnapCoordinator::create();
 
   return m_snapCoordinator.get();
 }
 
 void Document::setContextFeatures(ContextFeatures& features) {
   m_contextFeatures = &features;
 }
 
-static LayoutObject* nearestCommonHoverAncestor(LayoutObject* obj1,
-                                                LayoutObject* obj2) {
-  if (!obj1 || !obj2)
-    return 0;
-
-  for (LayoutObject* currObj1 = obj1; currObj1;
-       currObj1 = currObj1->hoverAncestor()) {
-    for (LayoutObject* currObj2 = obj2; currObj2;
-         currObj2 = currObj2->hoverAncestor()) {
-      if (currObj1 == currObj2)
-        return currObj1;
-    }
-  }
-
-  return 0;
-}
-
 // TODO(mustaq) |request| parameter maybe a misuse of HitTestRequest in
 // updateHoverActiveState() since the function doesn't bother with hit-testing.
 void Document::updateHoverActiveState(const HitTestRequest& request,
                                       Element* innerElement,
                                       Scrollbar* hitScrollbar) {
   DCHECK(!request.readOnly());
 
   if (request.active() && m_frame)
     m_frame->eventHandler().notifyElementActivated();
 
@@ skipping 57 matching lines @@
 
   // Check to see if the hovered node has changed.
   // If it hasn't, we do not need to do anything.
   Node* newHoverNode = innerElementInDocument;
   while (newHoverNode && !newHoverNode->layoutObject())
     newHoverNode = newHoverNode->parentOrShadowHostNode();
 
   // Update our current hover node.
   setHoverNode(newHoverNode);
 
-  // We have two different objects. Fetch their layoutObjects.
-  LayoutObject* oldHoverObj =
-      oldHoverNode ? oldHoverNode->layoutObject() : nullptr;
-  LayoutObject* newHoverObj =
-      newHoverNode ? newHoverNode->layoutObject() : nullptr;
+  // Update the old hover node distribution if needed, in case it's been
+  // detached.
+  //
+  // TODO(ecobos@igalia.com): Probably detached nodes should never appear here.
+  if (oldHoverNode)
+    oldHoverNode->updateDistribution();

[rune, 2017/03/30 12:25:19]

I just noticed, there a call to Document::updateDistribution() further up in
this method. I don't think updateDistribution() should be called for
disconnected nodes. Instead, we should not try to flat-tree-traverse nodes which
are !isConnected(). The ultimate fix, though, would be to update the old hover
node and clear the hover state of any disconnected ancestors as well.

I thought removing nodes should make use clear the hover in
hoveredNodeDetached() called from Element::detachLayoutTree(). Is the
performingReattach check there what's causing problems?

 
-  // Locate the common ancestor layout object for the two layoutObjects.
-  LayoutObject* ancestor = nearestCommonHoverAncestor(oldHoverObj, newHoverObj);
-  Node* ancestorNode(ancestor ? ancestor->node() : nullptr);
+  Node* ancestor =
+      (oldHoverNode && newHoverNode)
+          ? FlatTreeTraversal::commonAncestor(*oldHoverNode, *newHoverNode)
+          : nullptr;
 
   HeapVector<Member<Node>, 32> nodesToRemoveFromChain;
   HeapVector<Member<Node>, 32> nodesToAddToChain;
 
-  if (oldHoverObj != newHoverObj) {
-    // If the old hovered node is not nil but it's layoutObject is, it was
-    // probably detached as part of the :hover style (for instance by setting
-    // display:none in the :hover pseudo-class). In this case, the old hovered
-    // element (and its ancestors) must be updated, to ensure it's normal style
-    // is re-applied.
-    if (oldHoverNode && !oldHoverObj) {
-      for (Node& node : NodeTraversal::inclusiveAncestorsOf(*oldHoverNode)) {
-        if (!mustBeInActiveChain ||
-            (node.isElementNode() && toElement(node).inActiveChain()))
-          nodesToRemoveFromChain.push_back(node);
-      }
-    }
-
+  if (oldHoverNode != newHoverNode) {
     // The old hover path only needs to be cleared up to (and not including) the
     // common ancestor;
-    for (LayoutObject* curr = oldHoverObj; curr && curr != ancestor;
-         curr = curr->hoverAncestor()) {
-      if (curr->node() && !curr->isText() &&
-          (!mustBeInActiveChain || curr->node()->inActiveChain()))
-        nodesToRemoveFromChain.push_back(curr->node());
+    for (Node* curr = oldHoverNode; curr && curr != ancestor;
+         curr = FlatTreeTraversal::parent(*curr)) {
+      if (!curr->isTextNode() &&
+          (!mustBeInActiveChain || curr->inActiveChain()))
+        nodesToRemoveFromChain.push_back(curr);
     }
-
-    // TODO(mustaq): The two loops above may push a single node twice into
-    // nodesToRemoveFromChain. There must be a better way.
   }
 
   // Now set the hover state for our new object up to the root.
-  for (LayoutObject* curr = newHoverObj; curr; curr = curr->hoverAncestor()) {
-    if (curr->node() && !curr->isText() &&
-        (!mustBeInActiveChain || curr->node()->inActiveChain()))
-      nodesToAddToChain.push_back(curr->node());
+  for (Node* curr = newHoverNode; curr;
+       curr = FlatTreeTraversal::parent(*curr)) {
+    if (!curr->isTextNode() && (!mustBeInActiveChain || curr->inActiveChain()))
+      nodesToAddToChain.push_back(curr);
   }
 
-  size_t removeCount = nodesToRemoveFromChain.size();
-  for (size_t i = 0; i < removeCount; ++i) {
-    nodesToRemoveFromChain[i]->setHovered(false);
-  }
+  for (Node* node : nodesToRemoveFromChain)
+    node->setHovered(false);
 
   bool sawCommonAncestor = false;
-  size_t addCount = nodesToAddToChain.size();
-  for (size_t i = 0; i < addCount; ++i) {
+  for (Node* node : nodesToAddToChain) {
     // Elements past the common ancestor do not change hover state, but might
     // change active state.
-    if (ancestorNode && nodesToAddToChain[i] == ancestorNode)
+    if (node == ancestor)
       sawCommonAncestor = true;
     if (allowActiveChanges)
-      nodesToAddToChain[i]->setActive(true);
-    if (!sawCommonAncestor || nodesToAddToChain[i] == m_hoverNode) {
-      nodesToAddToChain[i]->setHovered(true);
-    }
+      node->setActive(true);
+    if (!sawCommonAncestor || node == m_hoverNode)
+      node->setHovered(true);
   }
 }
 
 bool Document::haveScriptBlockingStylesheetsLoaded() const {
   return m_styleEngine->haveScriptBlockingStylesheetsLoaded();
 }
 
 bool Document::haveRenderBlockingStylesheetsLoaded() const {
   if (RuntimeEnabledFeatures::cssInBodyDoesNotBlockPaintEnabled())
     return m_styleEngine->haveRenderBlockingStylesheetsLoaded();
@@ skipping 347 matching lines @@
 }
 
 void showLiveDocumentInstances() {
   WeakDocumentSet& set = liveDocumentSet();
   fprintf(stderr, "There are %u documents currently alive:\n", set.size());
   for (blink::Document* document : set)
     fprintf(stderr, "- Document %p URL: %s\n", document,
             document->url().getString().utf8().data());
 }
 #endif