Implement elementsFromPoint

Source/core/rendering/HitTestResult.cpp

Index: Source/core/rendering/HitTestResult.cpp
diff --git a/Source/core/rendering/HitTestResult.cpp b/Source/core/rendering/HitTestResult.cpp
index f7c4da96be03ea356a2807ed7e798af5a1050ed3..20d004c1900939b2a525ab46b11f4dc5bcf7d964 100644
--- a/Source/core/rendering/HitTestResult.cpp
+++ b/Source/core/rendering/HitTestResult.cpp
@@ -82,8 +82,8 @@ HitTestResult::HitTestResult(const HitTestResult& other)
     , m_scrollbar(other.scrollbar())
     , m_isOverWidget(other.isOverWidget())
 {
-    // Only copy the NodeSet in case of rect hit test.
-    m_rectBasedTestResult = adoptPtrWillBeNoop(other.m_rectBasedTestResult ? new NodeSet(*other.m_rectBasedTestResult) : 0);
+    // Only copy the NodeSet in case of list hit test.
+    m_listBasedTestResult = adoptPtrWillBeNoop(other.m_listBasedTestResult ? new NodeSet(*other.m_listBasedTestResult) : 0);
 }
 
 HitTestResult::~HitTestResult()
@@ -102,8 +102,8 @@ HitTestResult& HitTestResult::operator=(const HitTestResult& other)
     m_scrollbar = other.scrollbar();
     m_isOverWidget = other.isOverWidget();
 
-    // Only copy the NodeSet in case of rect hit test.
-    m_rectBasedTestResult = adoptPtrWillBeNoop(other.m_rectBasedTestResult ? new NodeSet(*other.m_rectBasedTestResult) : 0);
+    // Only copy the NodeSet in case of list hit test.
+    m_listBasedTestResult = adoptPtrWillBeNoop(other.m_listBasedTestResult ? new NodeSet(*other.m_listBasedTestResult) : 0);
 
     return *this;
 }
@@ -116,7 +116,7 @@ void HitTestResult::trace(Visitor* visitor)
     visitor->trace(m_innerURLElement);
     visitor->trace(m_scrollbar);
 #if ENABLE(OILPAN)
-    visitor->trace(m_rectBasedTestResult);
+    visitor->trace(m_listBasedTestResult);
 #endif
 }
 
@@ -371,35 +371,43 @@ bool HitTestResult::isContentEditable() const
     return m_innerNonSharedNode->hasEditableStyle();
 }
 
-bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const HitTestRequest& request, const HitTestLocation& locationInContainer, const LayoutRect& rect)
+bool HitTestResult::addNodeToListBasedTestResult(Node* node, const HitTestRequest& request, const HitTestLocation& locationInContainer, const LayoutRect& rect)
 {
-    // If it is not a rect-based hit test, this method has to be no-op.
-    // Return false, so the hit test stops.
-    if (!isRectBasedTest())
+    // If not a list-based test, this function should be a no-op.
+    if (!request.listBased()) {
+        ASSERT(!isRectBasedTest());
         return false;
+    }
 
     // If node is null, return true so the hit test can continue.
     if (!node)
         return true;
 
-    mutableRectBasedTestResult().add(node);
+    mutableListBasedTestResult().add(node);
+
+    if (request.penetrating())
+        return true;
 
     bool regionFilled = rect.contains(locationInContainer.boundingBox());
     return !regionFilled;
 }
 
-bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const HitTestRequest& request, const HitTestLocation& locationInContainer, const FloatRect& rect)
+bool HitTestResult::addNodeToListBasedTestResult(Node* node, const HitTestRequest& request, const HitTestLocation& locationInContainer, const FloatRect& rect)
 {
-    // If it is not a rect-based hit test, this method has to be no-op.
-    // Return false, so the hit test stops.
-    if (!isRectBasedTest())
+    // If not a list-based test, this function should be a no-op.
+    if (!request.listBased()) {
+        ASSERT(!isRectBasedTest());
         return false;
+    }
 
     // If node is null, return true so the hit test can continue.
     if (!node)
         return true;
 
-    mutableRectBasedTestResult().add(node);
+    mutableListBasedTestResult().add(node);
+
+    if (request.penetrating())
+        return true;
 
     bool regionFilled = rect.contains(locationInContainer.boundingBox());
     return !regionFilled;
@@ -407,8 +415,6 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const HitTestReques
 
 void HitTestResult::append(const HitTestResult& other)
 {
-    ASSERT(isRectBasedTest() && other.isRectBasedTest());

[Rick Byers, 2015/02/04 10:34:03]

why don't you want to assert both are list-based hit-tests?  "Appending" doesn't
really make sense otherwise, right?

Related: I kind of hate that we have m_innerNode etc. for list-based tests - I
think it's value isn't really meaningful (just one of the set in the list). 
I've seen several bugs where we accidental use the value for something and in
edge cases do the wrong thing.

[pdr., 2015/02/17 04:10:14]

I've added a const HitTestRequest& parameter to append and put an assert that
we're a list in append() instead of a bunch of error-prone asserts like I had
before.
I agree :/ I filed crbug.com/451914 for one of these.

[Rick Byers, 2015/02/20 20:02:23]

Seems reasonable, thanks.

I was naively thinking we could tell from the result whether it was list-based
(without having the request), but of course an empty result looks the same
whether it's list-based or not.  I don't see any good reason why the result
shouldn't keep a copy of the request (it has the location, which is logically as
much a part of the "request" as the HitTestRequest is).  If we did that, I think
it would clean some of this up (since we used to be able to tell just from the
result if it was expected to produce a list or not).  I can do that in a
follow-up CL though if you think it's a reasonable idea but don't want to mix it
in here.

-
     if (!m_scrollbar && other.scrollbar()) {
         setScrollbar(other.scrollbar());
     }
@@ -423,25 +429,25 @@ void HitTestResult::append(const HitTestResult& other)
         m_isOverWidget = other.isOverWidget();
     }
 
-    if (other.m_rectBasedTestResult) {
-        NodeSet& set = mutableRectBasedTestResult();
-        for (NodeSet::const_iterator it = other.m_rectBasedTestResult->begin(), last = other.m_rectBasedTestResult->end(); it != last; ++it)
+    if (other.m_listBasedTestResult) {
+        NodeSet& set = mutableListBasedTestResult();
+        for (NodeSet::const_iterator it = other.m_listBasedTestResult->begin(), last = other.m_listBasedTestResult->end(); it != last; ++it)
             set.add(it->get());
     }
 }
 
-const HitTestResult::NodeSet& HitTestResult::rectBasedTestResult() const
+const HitTestResult::NodeSet& HitTestResult::listBasedTestResult() const
 {
-    if (!m_rectBasedTestResult)
-        m_rectBasedTestResult = adoptPtrWillBeNoop(new NodeSet);
-    return *m_rectBasedTestResult;
+    if (!m_listBasedTestResult)
+        m_listBasedTestResult = adoptPtrWillBeNoop(new NodeSet);
+    return *m_listBasedTestResult;
 }
 
-HitTestResult::NodeSet& HitTestResult::mutableRectBasedTestResult()
+HitTestResult::NodeSet& HitTestResult::mutableListBasedTestResult()
 {
-    if (!m_rectBasedTestResult)
-        m_rectBasedTestResult = adoptPtrWillBeNoop(new NodeSet);
-    return *m_rectBasedTestResult;
+    if (!m_listBasedTestResult)
+        m_listBasedTestResult = adoptPtrWillBeNoop(new NodeSet);
+    return *m_listBasedTestResult;
 }
 
 void HitTestResult::resolveRectBasedTest(Node* resolvedInnerNode, const LayoutPoint& resolvedPointInMainFrame)
@@ -453,7 +459,7 @@ void HitTestResult::resolveRectBasedTest(Node* resolvedInnerNode, const LayoutPo
     m_innerNode = nullptr;
     m_innerNonSharedNode = nullptr;
     m_innerPossiblyPseudoNode = nullptr;
-    m_rectBasedTestResult = nullptr;
+    m_listBasedTestResult = nullptr;
 
     // Update the HitTestResult as if the supplied node had been hit in normal point-based hit-test.
     // Note that we don't know the local point after a rect-based hit-test, but we never use