diff options
author | tkent <tkent@chromium.org> | 2015-12-04 01:25:52 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-04 09:26:36 +0000 |
commit | 91db68e8cb2080d1e9e07858efea3f7e198112a9 (patch) | |
tree | 3bb4e20908ccc8290572fc08c49ca40ca1eb3a5f | |
parent | 9836863217881492da2ff68a74dd66323d11768a (diff) | |
download | chromium_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}
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 |