summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortkent <tkent@chromium.org>2015-12-04 01:25:52 -0800
committerCommit bot <commit-bot@chromium.org>2015-12-04 09:26:36 +0000
commit91db68e8cb2080d1e9e07858efea3f7e198112a9 (patch)
tree3bb4e20908ccc8290572fc08c49ca40ca1eb3a5f
parent9836863217881492da2ff68a74dd66323d11768a (diff)
downloadchromium_src-91db68e8cb2080d1e9e07858efea3f7e198112a9.zip
chromium_src-91db68e8cb2080d1e9e07858efea3f7e198112a9.tar.gz
chromium_src-91db68e8cb2080d1e9e07858efea3f7e198112a9.tar.bz2
Do not show a tooltip same as the previous one.
If a tooltip is closed by a keyboard event and the mouse cursor was not moved, the same tooltip was shown again after some delay only on Windows. Blink receives native mousemove events repeatedly on Windows even if the mouse cursor is not moved. ChromeClient saves the last tooltip string and the last mouse position, and do not call virtual setToolTip() if they are not changed. This CL not only fixes the bug, but also reduces unnecessary IPC messages for tooltip. BUG=557660 TEST=automated Review URL: https://codereview.chromium.org/1500813004 Cr-Commit-Position: refs/heads/master@{#363174}
-rw-r--r--third_party/WebKit/Source/core/core.gypi1
-rw-r--r--third_party/WebKit/Source/core/input/EventHandler.cpp2
-rw-r--r--third_party/WebKit/Source/core/page/ChromeClient.cpp11
-rw-r--r--third_party/WebKit/Source/core/page/ChromeClient.h7
-rw-r--r--third_party/WebKit/Source/core/page/ChromeClientTest.cpp70
5 files changed, 90 insertions, 1 deletions
diff --git a/third_party/WebKit/Source/core/core.gypi b/third_party/WebKit/Source/core/core.gypi
index bc3dfa1..57d5cc9 100644
--- a/third_party/WebKit/Source/core/core.gypi
+++ b/third_party/WebKit/Source/core/core.gypi
@@ -3919,6 +3919,7 @@
'loader/LinkHeaderTest.cpp',
'loader/LinkLoaderTest.cpp',
'loader/MixedContentCheckerTest.cpp',
+ 'page/ChromeClientTest.cpp',
'page/ContextMenuControllerTest.cpp',
'page/NetworkStateNotifierTest.cpp',
'page/PagePopupClientTest.cpp',
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp
index 444132c..1363f3f 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -3200,7 +3200,7 @@ bool EventHandler::handleAccessKey(const PlatformKeyboardEvent& evt)
WebInputEventResult EventHandler::keyEvent(const PlatformKeyboardEvent& initialKeyEvent)
{
RefPtrWillBeRawPtr<FrameView> protector(m_frame->view());
- m_frame->chromeClient().setToolTip(String(), LTR);
+ m_frame->chromeClient().clearToolTip();
if (initialKeyEvent.windowsVirtualKeyCode() == VK_CAPITAL)
capsLockStateMayHaveChanged();
diff --git a/third_party/WebKit/Source/core/page/ChromeClient.cpp b/third_party/WebKit/Source/core/page/ChromeClient.cpp
index ac60531..89c71fe 100644
--- a/third_party/WebKit/Source/core/page/ChromeClient.cpp
+++ b/third_party/WebKit/Source/core/page/ChromeClient.cpp
@@ -167,9 +167,20 @@ void ChromeClient::setToolTip(const HitTestResult& result)
}
}
+ if (m_lastToolTipPoint == result.hitTestLocation().point() && m_lastToolTipText == toolTip)
+ return;
+ m_lastToolTipPoint = result.hitTestLocation().point();
+ m_lastToolTipText = toolTip;
setToolTip(toolTip, toolTipDirection);
}
+void ChromeClient::clearToolTip()
+{
+ // Do not check m_lastToolTip* and do not update them intentionally.
+ // We don't want to show tooltips with same content after clearToolTip().
+ setToolTip(String(), LTR);
+}
+
void ChromeClient::print(LocalFrame* frame)
{
// Defer loads in case the client method runs a new event loop that would
diff --git a/third_party/WebKit/Source/core/page/ChromeClient.h b/third_party/WebKit/Source/core/page/ChromeClient.h
index 431a1f0b..771528b 100644
--- a/third_party/WebKit/Source/core/page/ChromeClient.h
+++ b/third_party/WebKit/Source/core/page/ChromeClient.h
@@ -22,6 +22,7 @@
#ifndef ChromeClient_h
#define ChromeClient_h
+#include "base/gtest_prod_util.h"
#include "core/CoreExport.h"
#include "core/dom/AXObjectCache.h"
#include "core/frame/ConsoleTypes.h"
@@ -155,6 +156,7 @@ public:
void mouseDidMoveOverElement(const HitTestResult&);
virtual void setToolTip(const String&, TextDirection) = 0;
+ void clearToolTip();
void print(LocalFrame*);
@@ -270,6 +272,11 @@ protected:
private:
bool canOpenModalIfDuringPageDismissal(Frame* mainFrame, DialogType, const String& message);
void setToolTip(const HitTestResult&);
+
+ LayoutPoint m_lastToolTipPoint;
+ String m_lastToolTipText;
+
+ FRIEND_TEST_ALL_PREFIXES(ChromeClientTest, SetToolTipFlood);
};
} // namespace blink
diff --git a/third_party/WebKit/Source/core/page/ChromeClientTest.cpp b/third_party/WebKit/Source/core/page/ChromeClientTest.cpp
new file mode 100644
index 0000000..42099ad
--- /dev/null
+++ b/third_party/WebKit/Source/core/page/ChromeClientTest.cpp
@@ -0,0 +1,70 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "core/page/ChromeClient.h"
+
+#include "core/dom/Document.h"
+#include "core/layout/HitTestResult.h"
+#include "core/loader/EmptyClients.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+namespace {
+
+class ChromeClientToolTipLogger : public EmptyChromeClient {
+public:
+ void setToolTip(const String& text, TextDirection) override
+ {
+ m_toolTipForLastSetToolTip = text;
+ }
+
+ String toolTipForLastSetToolTip() const { return m_toolTipForLastSetToolTip; }
+ void clearToolTipForLastSetToolTip() { m_toolTipForLastSetToolTip = String(); }
+
+private:
+ String m_toolTipForLastSetToolTip;
+};
+
+} // anonymous namespace
+
+class ChromeClientTest : public testing::Test {
+};
+
+TEST_F(ChromeClientTest, SetToolTipFlood)
+{
+ ChromeClientToolTipLogger logger;
+ ChromeClient* client = &logger;
+ HitTestResult result(HitTestRequest(HitTestRequest::Move), LayoutPoint(10, 20));
+ RefPtrWillBeRawPtr<Document> doc = Document::create();
+ RefPtrWillBeRawPtr<Element> element = HTMLElement::create(HTMLNames::divTag, *doc);
+ element->setAttribute(HTMLNames::titleAttr, "tooltip");
+ result.setInnerNode(element.get());
+
+ client->setToolTip(result);
+ EXPECT_EQ("tooltip", logger.toolTipForLastSetToolTip());
+
+ // seToolTip(HitTestResult) again in the same condition.
+ logger.clearToolTipForLastSetToolTip();
+ client->setToolTip(result);
+ // setToolTip(String,TextDirection) should not be called.
+ EXPECT_EQ(String(), logger.toolTipForLastSetToolTip());
+
+ // Cancel the tooltip, and setToolTip(HitTestResult) again.
+ client->clearToolTip();
+ logger.clearToolTipForLastSetToolTip();
+ client->setToolTip(result);
+ // setToolTip(String,TextDirection) should not be called.
+ EXPECT_EQ(String(), logger.toolTipForLastSetToolTip());
+
+ logger.clearToolTipForLastSetToolTip();
+ element->setAttribute(HTMLNames::titleAttr, "updated");
+ client->setToolTip(result);
+ // setToolTip(String,TextDirection) should be called because tooltip string
+ // is different from the last one.
+ EXPECT_EQ("updated", logger.toolTipForLastSetToolTip());
+}
+
+} // namespace blink