Fix link's hover state if the link under scrollbar

Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
Committed: https://crrev.com/524f6ac4628d3d5249d291314c6100bd0a337053
Cr-Commit-Position: refs/heads/master@{#430141}

[chaopeng, 2016-10-04 00:11:49]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 00:13:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chaopeng, 2016-10-04 00:13:22]

Description was changed from

==========
add clear hit test to fix

BUG=332227
==========

to

==========
fix link's hover state if the link under scrollbar

BUG=332227
==========

[commit-bot: I haz the power, 2016-10-04 00:23:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-04 00:23:42]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[chaopeng, 2016-10-04 02:56:56]

Description was changed from

==========
fix link's hover state if the link under scrollbar

BUG=332227
==========

to

==========
fix link's hover state if the link under scrollbar

BUG=636436
==========

[chaopeng, 2016-10-05 19:31:56]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 19:33:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 19:36:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 19:36:58]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chaopeng, 2016-10-05 19:47:36]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-05 19:48:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 21:20:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 21:20:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chaopeng, 2016-10-18 20:06:50]

chaopeng@chromium.org changed reviewers:
+ bokan@chromium.org

[chaopeng, 2016-11-01 15:46:41]

PTAL Thank you

[bokan, 2016-11-01 21:09:38]

I'm happy with the approach, I'd just like to make the test less manual.

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Document.h:729: bool hasScrollbar);
Nit: hasScrollbar -> hitScrollbar describes this better.

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:221: if (!s->enabled())
Hmm, in the case of regular, non-overlay scrollbars, the HitTestResult sets the
scrollbar? That's probably ok as long as the tests pass...

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/ChromeClientImpl.cpp (right):

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/ChromeClientImpl.cpp:631: // Ignore URL if hitTest
include scrollbar
I'add add to this comment " since we might have both a scrollbar and an element
in the case of overlay scrollbars."

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10334: TEST_F(WebFrameTest,
MouseOnScrollbarsAndHyperlink) {
Nit: Better name would be MouseOverLinkAndOverlayScrollar. I also like to add a
comment above the test describing what the test is checking for - in this case:
makes sure that mouse hover over an overlay scrollbar doesn't activate elements
below unless the scrollbar is faded out.

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10366:
document->frame()->eventHandler().selectCursorTypeForTest(hitTestResult);
We should avoid these type of "ForTest" methods unless there's really no other
way. In this case, we already have:

frame()->chromeClient().lastSetCursorForTesting()

Which means that you'd just need to figure out how to really simulate
interacting the mouse with the scrollbars here (rather than manually creating
scrollbars and doing hittests).

I found out these tests already use the "ScrollbarThemeMockOverlay" which are
overlay scrollbars, but they don't fade out. So I think if you make your content
big enough to have scrollbars, they should show up. You can then manually "fade
out" the scrollbars with frameView->setScrollbarsHidden(true|false). Then it's
just a matter of manually calling the mouse move/down methods in event handler.

That way, this test runs in a more realistic environment and so it's more likely
to catch real regressions.

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10375: // put disabled
scrollbar to hitTestResult, hitTestResult.scrollbar() should
Nit: Capitalize "put" -> "Put"

https://codereview.chromium.org/2389073002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10382: // put enabled
scrollbar to hitTestResult, activeHoverElement() should be null
Nit: ditto here and period at end. (We're pretty fussy with comments)

[bokan, 2016-11-02 00:09:54]

By the way, I'm running into issues with this layout test:

plugins/overlay-scrollbar-mouse-capture.html

And I think your patch will trip it too. I think it's wrong since it's testing
that the plugin gets the mouse down and mouse up events which is exactly what we
don't want to happen.

I'm making some changes to it on my end for a patch of mine but I think you
should delete it and rewrite the test as a unit test (should be fairly easy).
We'll chat tomorrow.

[chaopeng, 2016-11-02 19:05:38]

On 2016/11/02 00:09:54, bokan wrote:
> By the way, I'm running into issues with this layout test:
> 
> plugins/overlay-scrollbar-mouse-capture.html
> 
> And I think your patch will trip it too. I think it's wrong since it's testing
> that the plugin gets the mouse down and mouse up events which is exactly what
we
> don't want to happen.
> 
> I'm making some changes to it on my end for a patch of mine but I think you
> should delete it and rewrite the test as a unit test (should be fairly easy).
> We'll chat tomorrow.

PTAL. Thank you.

This patch did not break `plugins/overlay-scrollbar-mouse-capture.html` test,
but the Patch Set 2 would. And Patch Set 2 would break about 10 layout tests.

[chaopeng, 2016-11-02 19:05:46]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-02 19:06:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 20:24:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-02 20:24:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[bokan, 2016-11-02 21:36:34]

Looks like you've still got a failure in
fast/overflow/scrollbar-click-retains-focus.html, I haven't looked at it so
you'll need to investigate. Otherwise, everything looks great, lgtm once you fix
that test (assuming it doesn't require a significant change to the patch).

https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):

https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:220: return
(m_scrollbar.get() && m_scrollbar.get()->enabled()) ? m_scrollbar.get()
So I actually found out this already exists in
Scrollbar::shouldParticipateInHitTesting for Mac overlays but it requires a
little more plumbing that I did in https://codereview.chromium.org/2467693002/.
So leave this in but add a TODO(bokan) to remove and I'll remove it once my
patch lands and we can use that instead.

[chaopeng, 2016-11-03 02:56:11]

On 2016/11/02 21:36:34, bokan wrote:
> Looks like you've still got a failure in
> fast/overflow/scrollbar-click-retains-focus.html, I haven't looked at it so
> you'll need to investigate. Otherwise, everything looks great, lgtm once you
fix
> that test (assuming it doesn't require a significant change to the patch).
> 
>
https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):
> 
>
https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/HitTestResult.cpp:220: return
> (m_scrollbar.get() && m_scrollbar.get()->enabled()) ? m_scrollbar.get()
> So I actually found out this already exists in
> Scrollbar::shouldParticipateInHitTesting for Mac overlays but it requires a
> little more plumbing that I did in
https://codereview.chromium.org/2467693002/.
> So leave this in but add a TODO(bokan) to remove and I'll remove it once my
> patch lands and we can use that instead.

This test failed is caused by the <section> has scrollbar but disable, and we
add enable() check to hitTestResult::getScrollbar() that change the
focusedElement. So I add a div in section to make the scrollbar enable.

[chaopeng, 2016-11-03 02:57:18]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 02:57:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 04:50:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 04:50:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[bokan, 2016-11-03 11:53:01]

On 2016/11/03 02:56:11, chaopeng wrote:
> On 2016/11/02 21:36:34, bokan wrote:
> > Looks like you've still got a failure in
> > fast/overflow/scrollbar-click-retains-focus.html, I haven't looked at it so
> > you'll need to investigate. Otherwise, everything looks great, lgtm once you
> fix
> > that test (assuming it doesn't require a significant change to the patch).
> > 
> >
>
https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2389073002/diff/100001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/layout/HitTestResult.cpp:220: return
> > (m_scrollbar.get() && m_scrollbar.get()->enabled()) ? m_scrollbar.get()
> > So I actually found out this already exists in
> > Scrollbar::shouldParticipateInHitTesting for Mac overlays but it requires a
> > little more plumbing that I did in
> https://codereview.chromium.org/2467693002/.
> > So leave this in but add a TODO(bokan) to remove and I'll remove it once my
> > patch lands and we can use that instead.
> 
> This test failed is caused by the <section> has scrollbar but disable, and we
> add enable() check to hitTestResult::getScrollbar() that change the
> focusedElement. So I add a div in section to make the scrollbar enable.

Ah, actually, I think that's a real failure since the scrollbars aren't overlay.
In your check in HittestResult, only return nullptr on disabled if the scrollbar
is overlay.

[bokan, 2016-11-03 11:53:20]

https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html
(right):

https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html:122:

Nit: remove space

[chaopeng, 2016-11-03 13:27:20]

The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 13:27:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chaopeng, 2016-11-03 13:48:55]

The CQ bit was unchecked by chaopeng@chromium.org

[bokan, 2016-11-03 16:49:45]

On 2016/11/03 11:53:20, bokan wrote:
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html
> (right):
> 
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html:122:
> 
> Nit: remove space

Oh, one more thing: please elaborate the commit message as to what this patch is
changing

[chaopeng, 2016-11-03 17:59:33]

On 2016/11/03 16:49:45, bokan wrote:
> On 2016/11/03 11:53:20, bokan wrote:
> >
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
> > File
> >
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
> >
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html:122:
> > 
> > Nit: remove space
> 
> Oh, one more thing: please elaborate the commit message as to what this patch
is
> changing

Hi David, the newest patch (remove if-statement in hitTestResult::scrollbar and
merge your patch) can pass plugins/overlay-scrollbar-mouse-capture.html and
WebFrameTest.MouseOverLinkAndOverlayScrollbar on OSX. Can I land it?

[bokan, 2016-11-03 18:00:12]

On 2016/11/03 17:59:33, chaopeng wrote:
> On 2016/11/03 16:49:45, bokan wrote:
> > On 2016/11/03 11:53:20, bokan wrote:
> > >
> >
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
> > > File
> > >
> >
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2389073002/diff/120001/third_party/WebKit/Lay...
> > >
> >
>
third_party/WebKit/LayoutTests/fast/overflow/scrollbar-click-retains-focus.html:122:
> > > 
> > > Nit: remove space
> > 
> > Oh, one more thing: please elaborate the commit message as to what this
patch
> is
> > changing
> 
> Hi David, the newest patch (remove if-statement in hitTestResult::scrollbar
and
> merge your patch) can pass plugins/overlay-scrollbar-mouse-capture.html and
> WebFrameTest.MouseOverLinkAndOverlayScrollbar on OSX. Can I land it?

I'll take a look in a few minutes, please wait for now.

[bokan, 2016-11-03 18:23:10]

still lgtm

[chaopeng, 2016-11-03 18:55:13]

Description was changed from

==========
fix link's hover state if the link under scrollbar

BUG=636436
==========

to

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

[chaopeng, 2016-11-03 18:56:00]

Description was changed from

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

to

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

[chaopeng, 2016-11-03 18:56:17]

Description was changed from

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

to

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

[chaopeng, 2016-11-03 18:56:30]

The CQ bit was checked by chaopeng@chromium.org

[commit-bot: I haz the power, 2016-11-03 18:57:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bokan, 2016-11-03 18:57:51]

Description was changed from

==========
fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

to

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

[commit-bot: I haz the power, 2016-11-03 20:25:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 20:25:24]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chaopeng, 2016-11-03 22:07:25]

The CQ bit was checked by chaopeng@chromium.org

[commit-bot: I haz the power, 2016-11-03 22:08:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 02:01:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 02:01:09]

Failed to apply patch for third_party/WebKit/Source/web/tests/WebFrameTest.cpp:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/web/tests/WebFrameTest.cpp:79
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/web/tests/WebFrameTest.cpp' with
conflicts.
  U third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Patch:       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
676f6d6ac30b0f28c2dd8a3d4d7caf43757960a9..978cdebe62784199e06dde16365ff7c34492f3e9
100644
--- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
+++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
@@ -79,13 +79,18 @@
 #include "core/testing/NullExecutionContext.h"
 #include "modules/mediastream/MediaStream.h"
 #include "modules/mediastream/MediaStreamRegistry.h"
+#include "platform/Cursor.h"
 #include "platform/DragImage.h"
+#include "platform/PlatformMouseEvent.h"
 #include "platform/PlatformResourceLoader.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/UserGestureIndicator.h"
 #include "platform/geometry/FloatRect.h"
 #include "platform/network/ResourceError.h"
+#include "platform/scroll/Scrollbar.h"
+#include "platform/scroll/ScrollbarTestSuite.h"
 #include "platform/scroll/ScrollbarTheme.h"
+#include "platform/scroll/ScrollbarThemeMock.h"
 #include "platform/scroll/ScrollbarThemeOverlayMock.h"
 #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h"
 #include "platform/testing/URLTestHelpers.h"
@@ -10346,6 +10351,104 @@ TEST_F(WebFrameTest,
HidingScrollbarsOnScrollableAreaDisablesScrollbars) {
   EXPECT_TRUE(scrollerArea->verticalScrollbar()->enabled());
 }
 
+// Makes sure that mouse hover over an overlay scrollbar doesn't activate
+// elements below unless the scrollbar is faded out.
+TEST_F(WebFrameTest, MouseOverLinkAndOverlayScrollbar) {
+  FrameTestHelpers::WebViewHelper webViewHelper;
+  webViewHelper.initialize(
+      true, nullptr, nullptr, nullptr,
+      [](WebSettings* settings) { settings->setDeviceSupportsMouse(true); });
+  webViewHelper.resize(WebSize(20, 20));
+  WebViewImpl* webView = webViewHelper.webView();
+
+  initializeWithHTML(*webView->mainFrameImpl()->frame(),
+                     "<!DOCTYPE html>"
+                     "<a id='a' href='javascript:void(0);'>"
+                    
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+                    
"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
+  HitTestResult hitTestResult =
+      webView->coreHitTestResultAt(WebPoint(18, aTag->offsetTop()));
+
+  EXPECT_TRUE(hitTestResult.URLElement());
+  EXPECT_TRUE(hitTestResult.innerElement());
+  EXPECT_TRUE(hitTestResult.scrollbar());
+
+  // Mouse over link. Mouse cursor should be hand.
+  PlatformMouseEvent mouseMoveOverLinkEvent(
+      IntPoint(aTag->offsetLeft(), aTag->offsetTop()),
+      IntPoint(aTag->offsetLeft(), aTag->offsetTop()),
+      WebPointerProperties::Button::NoButton, PlatformEvent::MouseMoved, 0,
+      PlatformEvent::NoModifiers, WTF::monotonicallyIncreasingTime());
+  document->frame()->eventHandler().handleMouseMoveEvent(
+      mouseMoveOverLinkEvent);
+
+  EXPECT_EQ(
+      Cursor::Type::Hand,
+      document->frame()->chromeClient().lastSetCursorForTesting().getType());
+
+  // Mouse over enabled overlay scrollbar. Mouse cursor should be pointer and
no
+  // active hover element.
+  PlatformMouseEvent mouseMoveEvent(
+      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
+      WebPointerProperties::Button::NoButton, PlatformEvent::MouseMoved, 0,
+      PlatformEvent::NoModifiers, WTF::monotonicallyIncreasingTime());
+  document->frame()->eventHandler().handleMouseMoveEvent(mouseMoveEvent);
+
+  EXPECT_EQ(
+      Cursor::Type::Pointer,
+      document->frame()->chromeClient().lastSetCursorForTesting().getType());
+
+  PlatformMouseEvent mousePressEvent(
+      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
+      WebPointerProperties::Button::Left, PlatformEvent::MousePressed, 0,
+      PlatformEvent::Modifiers::LeftButtonDown,
+      WTF::monotonicallyIncreasingTime());
+  document->frame()->eventHandler().handleMousePressEvent(mousePressEvent);
+
+  EXPECT_FALSE(document->activeHoverElement());
+  EXPECT_FALSE(document->hoverNode());
+
+  PlatformMouseEvent MouseReleaseEvent(
+      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
+      WebPointerProperties::Button::Left, PlatformEvent::MouseReleased, 0,
+      PlatformEvent::Modifiers::LeftButtonDown,
+      WTF::monotonicallyIncreasingTime());
+  document->frame()->eventHandler().handleMouseReleaseEvent(MouseReleaseEvent);
+
+  // Mouse over disabled overlay scrollbar. Mouse cursor should be hand and has
+  // active hover element.
+  webView->mainFrameImpl()->frameView()->setScrollbarsHidden(true);
+
+  // Ensure hittest only has link
+  hitTestResult = webView->coreHitTestResultAt(WebPoint(18,
aTag->offsetTop()));
+
+  EXPECT_TRUE(hitTestResult.URLElement());
+  EXPECT_TRUE(hitTestResult.innerElement());
+  EXPECT_FALSE(hitTestResult.scrollbar());
+
+  document->frame()->eventHandler().handleMouseMoveEvent(mouseMoveEvent);
+
+  EXPECT_EQ(
+      Cursor::Type::Hand,
+      document->frame()->chromeClient().lastSetCursorForTesting().getType());
+
+  document->frame()->eventHandler().handleMousePressEvent(mousePressEvent);
+
+  EXPECT_TRUE(document->activeHoverElement());
+  EXPECT_TRUE(document->hoverNode());
+
+  document->frame()->eventHandler().handleMouseReleaseEvent(MouseReleaseEvent);
+}
 static void disableCompositing(WebSettings* settings) {
   settings->setAcceleratedCompositingEnabled(false);
   settings->setPreferCompositingToLCDTextEnabled(false);

[bokan, 2016-11-04 13:28:06]

On 2016/11/04 02:01:09, commit-bot: I haz the power wrote:
> Failed to apply patch for
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:
> While running git apply --index -3 -p1;
>   error: patch failed: third_party/WebKit/Source/web/tests/WebFrameTest.cpp:79
>   Falling back to three-way merge...
>   Applied patch to 'third_party/WebKit/Source/web/tests/WebFrameTest.cpp' with
> conflicts.
>   U third_party/WebKit/Source/web/tests/WebFrameTest.cpp
> 
> Patch:       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
>
676f6d6ac30b0f28c2dd8a3d4d7caf43757960a9..978cdebe62784199e06dde16365ff7c34492f3e9
> 100644
> --- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
> +++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
> @@ -79,13 +79,18 @@
>  #include "core/testing/NullExecutionContext.h"
>  #include "modules/mediastream/MediaStream.h"
>  #include "modules/mediastream/MediaStreamRegistry.h"
> +#include "platform/Cursor.h"
>  #include "platform/DragImage.h"
> +#include "platform/PlatformMouseEvent.h"
>  #include "platform/PlatformResourceLoader.h"
>  #include "platform/RuntimeEnabledFeatures.h"
>  #include "platform/UserGestureIndicator.h"
>  #include "platform/geometry/FloatRect.h"
>  #include "platform/network/ResourceError.h"
> +#include "platform/scroll/Scrollbar.h"
> +#include "platform/scroll/ScrollbarTestSuite.h"
>  #include "platform/scroll/ScrollbarTheme.h"
> +#include "platform/scroll/ScrollbarThemeMock.h"
>  #include "platform/scroll/ScrollbarThemeOverlayMock.h"
>  #include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h"
>  #include "platform/testing/URLTestHelpers.h"
> @@ -10346,6 +10351,104 @@ TEST_F(WebFrameTest,
> HidingScrollbarsOnScrollableAreaDisablesScrollbars) {
>    EXPECT_TRUE(scrollerArea->verticalScrollbar()->enabled());
>  }
>  
> +// Makes sure that mouse hover over an overlay scrollbar doesn't activate
> +// elements below unless the scrollbar is faded out.
> +TEST_F(WebFrameTest, MouseOverLinkAndOverlayScrollbar) {
> +  FrameTestHelpers::WebViewHelper webViewHelper;
> +  webViewHelper.initialize(
> +      true, nullptr, nullptr, nullptr,
> +      [](WebSettings* settings) { settings->setDeviceSupportsMouse(true); });
> +  webViewHelper.resize(WebSize(20, 20));
> +  WebViewImpl* webView = webViewHelper.webView();
> +
> +  initializeWithHTML(*webView->mainFrameImpl()->frame(),
> +                     "<!DOCTYPE html>"
> +                     "<a id='a' href='javascript:void(0);'>"
> +                    
> "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
> +                    
> "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
> +  HitTestResult hitTestResult =
> +      webView->coreHitTestResultAt(WebPoint(18, aTag->offsetTop()));
> +
> +  EXPECT_TRUE(hitTestResult.URLElement());
> +  EXPECT_TRUE(hitTestResult.innerElement());
> +  EXPECT_TRUE(hitTestResult.scrollbar());
> +
> +  // Mouse over link. Mouse cursor should be hand.
> +  PlatformMouseEvent mouseMoveOverLinkEvent(
> +      IntPoint(aTag->offsetLeft(), aTag->offsetTop()),
> +      IntPoint(aTag->offsetLeft(), aTag->offsetTop()),
> +      WebPointerProperties::Button::NoButton, PlatformEvent::MouseMoved, 0,
> +      PlatformEvent::NoModifiers, WTF::monotonicallyIncreasingTime());
> +  document->frame()->eventHandler().handleMouseMoveEvent(
> +      mouseMoveOverLinkEvent);
> +
> +  EXPECT_EQ(
> +      Cursor::Type::Hand,
> +      document->frame()->chromeClient().lastSetCursorForTesting().getType());
> +
> +  // Mouse over enabled overlay scrollbar. Mouse cursor should be pointer and
> no
> +  // active hover element.
> +  PlatformMouseEvent mouseMoveEvent(
> +      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
> +      WebPointerProperties::Button::NoButton, PlatformEvent::MouseMoved, 0,
> +      PlatformEvent::NoModifiers, WTF::monotonicallyIncreasingTime());
> +  document->frame()->eventHandler().handleMouseMoveEvent(mouseMoveEvent);
> +
> +  EXPECT_EQ(
> +      Cursor::Type::Pointer,
> +      document->frame()->chromeClient().lastSetCursorForTesting().getType());
> +
> +  PlatformMouseEvent mousePressEvent(
> +      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
> +      WebPointerProperties::Button::Left, PlatformEvent::MousePressed, 0,
> +      PlatformEvent::Modifiers::LeftButtonDown,
> +      WTF::monotonicallyIncreasingTime());
> +  document->frame()->eventHandler().handleMousePressEvent(mousePressEvent);
> +
> +  EXPECT_FALSE(document->activeHoverElement());
> +  EXPECT_FALSE(document->hoverNode());
> +
> +  PlatformMouseEvent MouseReleaseEvent(
> +      IntPoint(18, aTag->offsetTop()), IntPoint(18, aTag->offsetTop()),
> +      WebPointerProperties::Button::Left, PlatformEvent::MouseReleased, 0,
> +      PlatformEvent::Modifiers::LeftButtonDown,
> +      WTF::monotonicallyIncreasingTime());
> + 
document->frame()->eventHandler().handleMouseReleaseEvent(MouseReleaseEvent);
> +
> +  // Mouse over disabled overlay scrollbar. Mouse cursor should be hand and
has
> +  // active hover element.
> +  webView->mainFrameImpl()->frameView()->setScrollbarsHidden(true);
> +
> +  // Ensure hittest only has link
> +  hitTestResult = webView->coreHitTestResultAt(WebPoint(18,
> aTag->offsetTop()));
> +
> +  EXPECT_TRUE(hitTestResult.URLElement());
> +  EXPECT_TRUE(hitTestResult.innerElement());
> +  EXPECT_FALSE(hitTestResult.scrollbar());
> +
> +  document->frame()->eventHandler().handleMouseMoveEvent(mouseMoveEvent);
> +
> +  EXPECT_EQ(
> +      Cursor::Type::Hand,
> +      document->frame()->chromeClient().lastSetCursorForTesting().getType());
> +
> +  document->frame()->eventHandler().handleMousePressEvent(mousePressEvent);
> +
> +  EXPECT_TRUE(document->activeHoverElement());
> +  EXPECT_TRUE(document->hoverNode());
> +
> + 
document->frame()->eventHandler().handleMouseReleaseEvent(MouseReleaseEvent);
> +}
>  static void disableCompositing(WebSettings* settings) {
>    settings->setAcceleratedCompositingEnabled(false);
>    settings->setPreferCompositingToLCDTextEnabled(false);

Sorry, my patch got reverted, please wait until I reland it.

[chaopeng, 2016-11-05 01:42:47]

The CQ bit was checked by chaopeng@chromium.org

[commit-bot: I haz the power, 2016-11-05 01:43:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-05 03:27:35]

Description was changed from

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

to

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

[commit-bot: I haz the power, 2016-11-05 03:27:37]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-11-05 03:30:38]

Description was changed from

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement, 
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436
==========

to

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436

Committed: https://crrev.com/524f6ac4628d3d5249d291314c6100bd0a337053
Cr-Commit-Position: refs/heads/master@{#430141}
==========

[commit-bot: I haz the power, 2016-11-05 03:30:39]

Patchset 10 (id:??) landed as
https://crrev.com/524f6ac4628d3d5249d291314c6100bd0a337053
Cr-Commit-Position: refs/heads/master@{#430141}

[bokan, 2016-11-08 17:11:23]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/2485203002/ by bokan@chromium.org.

The reason for reverting is: Due to crbug.com/662402, need to revert
crrev.com/2467693002 which this depends on. I'll reland when the dust settles..

[chaopeng, 2016-11-08 18:40:39]

Description was changed from

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436

Committed: https://crrev.com/524f6ac4628d3d5249d291314c6100bd0a337053
Cr-Commit-Position: refs/heads/master@{#430141}
==========

to

==========
Fix link's hover state if the link under scrollbar

In this patch, we check the hitTest includes scrollbar in
ChromeClientImpl::showMouseOverURL, ChromeClient::mouseDidMoveOverElement,
Document::updateHoverActiveState, EventHandler::selectAutoCursor to prevent
showing url, tooltip and hand cursor when mouse move on the link under scrollbar
or showing active state when mouse click on the link under scrollbar.

BUG=636436

Committed: https://crrev.com/524f6ac4628d3d5249d291314c6100bd0a337053
Cr-Commit-Position: refs/heads/master@{#430141}
==========