From 43512ed0d791dae4cf5c482dfd0852d3b5701c7f Mon Sep 17 00:00:00 2001
From: Tim Dresser <tdresser@chromium.org>
Date: Thu, 10 Mar 2016 14:38:44 -0500
Subject: [PATCH] Hide tooltips while scrolling.

BUG=586852
TEST=FrameViewTest.HideTooltipWhenScrollPositionChanges

Review URL: https://codereview.chromium.org/1735363003

Cr-Commit-Position: refs/heads/master@{#378446}
(cherry picked from commit 933d8eb82718fe845ece37c26cd0d593e8d74590)

Review URL: https://codereview.chromium.org/1782163002 .

Cr-Commit-Position: refs/branch-heads/2661@{#182}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}
---
 .../WebKit/Source/core/frame/FrameView.cpp    |  3 +++
 .../Source/core/frame/FrameViewTest.cpp       | 21 +++++++++++++++----
 ui/views/corewm/tooltip_controller.cc         |  4 ++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index 98993b73fa7e0..69dcfa20d7352 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -1590,6 +1590,9 @@ void FrameView::scrollPositionChanged()
     document->enqueueScrollEventForNode(document);
 
     m_frame->eventHandler().dispatchFakeMouseMoveEventSoon();
+    Page* page = frame().page();
+    if (page)
+        page->chromeClient().clearToolTip();
 
     if (LayoutView* layoutView = document->layoutView()) {
         if (layoutView->usesCompositing())
diff --git a/third_party/WebKit/Source/core/frame/FrameViewTest.cpp b/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
index ab9fd994d66c2..e620a9b8444f2 100644
--- a/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameViewTest.cpp
@@ -20,10 +20,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "wtf/OwnPtr.h"
 
-// This test ensures that FrameView informs the ChromeClient of changes to the
-// paint artifact so that they can be shown to the user (e.g. via the
-// compositor).
-
 using testing::_;
 using testing::AnyNumber;
 
@@ -41,6 +37,7 @@ class MockChromeClient : public EmptyChromeClient {
     }
     MOCK_METHOD1(didPaint, void(const PaintArtifact&));
     MOCK_METHOD2(attachRootGraphicsLayer, void(GraphicsLayer*, LocalFrame* localRoot));
+    MOCK_METHOD2(setToolTip, void(const String&, TextDirection));
 
     void scheduleAnimation(Widget*) override { m_hasScheduledAnimation = true; }
     bool m_hasScheduledAnimation;
@@ -52,6 +49,11 @@ class FrameViewTestBase : public testing::Test {
         : m_chromeClient(adoptPtrWillBeNoop(new MockChromeClient))
     { }
 
+    ~FrameViewTestBase()
+    {
+        testing::Mock::VerifyAndClearExpectations(&chromeClient());
+    }
+
     void SetUp() override
     {
         Page::PageClients clients;
@@ -103,6 +105,9 @@ class FrameViewSlimmingPaintV2Test : public FrameViewTestBase {
     RuntimeEnabledFeatures::Backup m_featuresBackup;
 };
 
+// These tests ensure that FrameView informs the ChromeClient of changes to the
+// paint artifact so that they can be shown to the user (e.g. via the
+// compositor).
 TEST_F(FrameViewSlimmingPaintV2Test, PaintOnce)
 {
     EXPECT_CALL(chromeClient(), didPaint(_));
@@ -144,5 +149,13 @@ TEST_F(FrameViewTest, SetPaintInvalidationOutOfUpdateAllLifecyclePhases)
     EXPECT_FALSE(chromeClient().m_hasScheduledAnimation);
 }
 
+// If we don't hide the tooltip on scroll, it can negatively impact scrolling
+// performance. See crbug.com/586852 for details.
+TEST_F(FrameViewTest, HideTooltipWhenScrollPositionChanges)
+{
+    EXPECT_CALL(chromeClient(), setToolTip(String(), _));
+    document().view()->scrollTo(DoublePoint(1, 1));
+}
+
 } // namespace
 } // namespace blink
diff --git a/ui/views/corewm/tooltip_controller.cc b/ui/views/corewm/tooltip_controller.cc
index c7f466b5e7edd..c5dad8c4598c2 100644
--- a/ui/views/corewm/tooltip_controller.cc
+++ b/ui/views/corewm/tooltip_controller.cc
@@ -232,6 +232,10 @@ void TooltipController::OnMouseEvent(ui::MouseEvent* event) {
       // Hide the tooltip for click, release, drag, wheel events.
       if (tooltip_->IsVisible())
         tooltip_->Hide();
+
+      // Don't reshow the tooltip during scroll.
+      if (tooltip_timer_.IsRunning())
+        tooltip_timer_.Reset();
       break;
     default:
       break;