Change hittest node to be scrollbar's parent when hittest include a scrollbar

third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Index: third_party/WebKit/Source/web/tests/WebFrameTest.cpp
diff --git a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
index 146f8e217958b470fd382db5997a64925a30600d..ec2ecd886fa5d66006f1db8c8e3b6c4fe48a4d18 100644
--- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
+++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
@@ -10907,12 +10907,12 @@ TEST_F(WebFrameTest, MouseOverLinkAndOverlayScrollbar) {
   Document* document = webView->mainFrameImpl()->frame()->document();
   Element* aTag = document->getElementById("a");
 
-  // Ensure hittest has scrollbar and link
+  // Ensure hittest only has scrollbar.

[bokan, 2017/03/31 20:07:00]

What changed here? Shouldn't the hit test set the scrollbar's ScrollableArea's
associated Element as the innerElement?

   HitTestResult hitTestResult =
       webView->coreHitTestResultAt(WebPoint(18, aTag->offsetTop()));
 
-  EXPECT_TRUE(hitTestResult.URLElement());
-  EXPECT_TRUE(hitTestResult.innerElement());
+  EXPECT_FALSE(hitTestResult.URLElement());
+  EXPECT_FALSE(hitTestResult.innerElement());
   EXPECT_TRUE(hitTestResult.scrollbar());
   EXPECT_FALSE(hitTestResult.scrollbar()->isCustomScrollbar());
 
@@ -11047,6 +11047,73 @@ TEST_F(WebFrameTest, MouseOverCustomScrollbar) {
   EXPECT_EQ(hitTestResult.scrollbar()->hoveredPart(), ScrollbarPart::ThumbPart);
 }
 
+// Makes sure that mouse hover over an overlay scrollbar doesn't hover iframe
+// below.
+TEST_F(WebFrameTest, MouseOverScrollbarAndIFrame) {
+  registerMockedHttpURLLoad("scrollbar-and-iframe-hover.html");
+  FrameTestHelpers::WebViewHelper webViewHelper;
+  WebViewImpl* webView = webViewHelper.initializeAndLoad(
+      m_baseURL + "scrollbar-and-iframe-hover.html");
+
+  webViewHelper.resize(WebSize(200, 200));
+
+  webView->updateAllLifecyclePhases();
+
+  Document* document = toLocalFrame(webView->page()->mainFrame())->document();
+
+  // Ensure hittest only has IFRAME.
+  HitTestResult hitTestResult = webView->coreHitTestResultAt(WebPoint(5, 5));
+
+  EXPECT_TRUE(hitTestResult.innerElement());
+  EXPECT_FALSE(hitTestResult.scrollbar());
+
+  // Mouse over IFRAME.
+  WebMouseEvent mouseMoveOverIFrame(
+      WebInputEvent::MouseMove, WebFloatPoint(5, 5), WebFloatPoint(5, 5),
+      WebPointerProperties::Button::NoButton, 0, WebInputEvent::NoModifiers,
+      TimeTicks::Now().InSeconds());
+  mouseMoveOverIFrame.setFrameScale(1);
+  document->frame()->eventHandler().handleMouseMoveEvent(
+      mouseMoveOverIFrame, Vector<WebMouseEvent>());
+
+  // IFRAME hover.
+  EXPECT_EQ(document->hoverNode()->nodeName(), "IFRAME");

[bokan, 2017/03/31 20:07:00]

Give the iframe an id, save document.getElelementById("iframe") and compare the
pointers here.

+
+  // Ensure hittest has scrollbar.
+  hitTestResult = webView->coreHitTestResultAt(WebPoint(195, 5));
+  EXPECT_FALSE(hitTestResult.innerElement());
+  EXPECT_TRUE(hitTestResult.scrollbar());
+  EXPECT_TRUE(hitTestResult.scrollbar()->enabled());
+
+  // Mouse over scrollbar.
+  WebMouseEvent mouseMoveOverIFrameAndScrollbar(
+      WebInputEvent::MouseMove, WebFloatPoint(195, 5), WebFloatPoint(195, 5),
+      WebPointerProperties::Button::NoButton, 0, WebInputEvent::NoModifiers,
+      TimeTicks::Now().InSeconds());
+  mouseMoveOverIFrameAndScrollbar.setFrameScale(1);
+  document->frame()->eventHandler().handleMouseMoveEvent(
+      mouseMoveOverIFrameAndScrollbar, Vector<WebMouseEvent>());
+
+  // IFRAME not hover.
+  EXPECT_FALSE(document->hoverNode());

[bokan, 2017/03/31 20:07:00]

The iframe shouldn't be hovered, but shouldn't the main frame's documentElement?

[chaopeng, 2017/04/03 17:50:26]

For root scrollbar: scrollableArea->layoutBox()->node()  is #document not
documentElement and #document is not Element that is why hoverNode is null.

[bokan, 2017/04/04 15:18:01]

Ok, that behavior is unchanged from before this patch? i.e. today hovering a
root scrollbar sets hoverNode to null? If so, the original check (EXPECT_FALSE)
is fine.

[chaopeng, 2017/04/04 18:20:36]

Yes, we dont change the behavior.

+
+  // Disable the Scrollbar.
+  webView->mainFrameImpl()->frameView()->setScrollbarsHidden(true);
+
+  // Ensure hittest has IFRAME and no scrollbar.
+  hitTestResult = webView->coreHitTestResultAt(WebPoint(195, 5));
+
+  EXPECT_TRUE(hitTestResult.innerElement());
+  EXPECT_FALSE(hitTestResult.scrollbar());
+
+  // Mouse over disabled scrollbar.
+  document->frame()->eventHandler().handleMouseMoveEvent(
+      mouseMoveOverIFrameAndScrollbar, Vector<WebMouseEvent>());

[bokan, 2017/03/31 20:07:00]

For safety, I'd change the coordinate here by at least a pixel. It's not out of
the question that a event handler might one day add some logic to not do
anything if consecutive mouse moves have the same coordinate.

+
+  // IFRAME hover.
+  EXPECT_EQ(document->hoverNode()->nodeName(), "IFRAME");
+}
+
 // Makes sure that mouse hover over a scrollbar also hover the element owns the
 // scrollbar.
 TEST_F(WebFrameTest, MouseOverScrollbarAndParentElement) {