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

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

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 10889 matching lines @@
                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
                      "</a>"
                      "<div style='position: absolute; top: 1000px'>end</div>");
 
   webView->updateAllLifecyclePhases();
 
   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());
 
   // Mouse over link. Mouse cursor should be hand.
   WebMouseEvent mouseMoveOverLinkEvent(
       WebInputEvent::MouseMove,
       WebFloatPoint(aTag->offsetLeft(), aTag->offsetTop()),
       WebFloatPoint(aTag->offsetLeft(), aTag->offsetTop()),
       WebPointerProperties::Button::NoButton, 0, WebInputEvent::NoModifiers,
       TimeTicks::Now().InSeconds());
@@ skipping 114 matching lines @@
       TimeTicks::Now().InSeconds());
   mouseMoveOverDivAndScrollbar.setFrameScale(1);
   document->frame()->eventHandler().handleMouseMoveEvent(
       mouseMoveOverDivAndScrollbar, Vector<WebMouseEvent>());
 
   // Custom not change the DIV :hover
   EXPECT_EQ(document->hoverNode(), scrollbarDiv);
   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) {
   registerMockedHttpURLLoad("scrollbar-and-element-hover.html");
   FrameTestHelpers::WebViewHelper webViewHelper;
   RuntimeEnabledFeatures::setOverlayScrollbarsEnabled(false);
   WebViewImpl* webView = webViewHelper.initializeAndLoad(
       m_baseURL + "scrollbar-and-element-hover.html");
 
   webViewHelper.resize(WebSize(200, 200));
@@ skipping 499 matching lines @@
   EXPECT_FALSE(testSelectAll("<textarea></textarea>"));
   EXPECT_TRUE(testSelectAll("<textarea>nonempty</textarea>"));
   EXPECT_FALSE(testSelectAll("<input>"));
   EXPECT_TRUE(testSelectAll("<input value='nonempty'>"));
   // TODO(amaralp): Empty contenteditable should not have select all enabled.
   EXPECT_TRUE(testSelectAll("<div contenteditable></div>"));
   EXPECT_TRUE(testSelectAll("<div contenteditable>nonempty</div>"));
 }
 
 }  // namespace blink