Commit 52d30818 authored by Mustafa Emre Acer's avatar Mustafa Emre Acer Committed by Commit Bot

Add protection against repeated click events

crrev.com/c/1140307 introduced protection against double clicks: It disallowed clicks when they happen shortly after a view is shown.

This prevents unintentional double clicks but doesn't protect users when they are tricked into repeatedly clicking at the same location.
In this scenario, double click protection is only good for the double click interval on that platform (e.g. 500 ms on Windows). After that, subsequential clicks are still accepted.

This CL adds an additional check to prevent repetitive clicks happening with short intervals.
As an example, if a dialog is shown at t0 (assuming a 500 ms double click interval):
- A click at t0 + 200 ms is ignored by the double click protection
- A subsequent click at t0 + 600 ms is ignored by this protection
- Another click at t0 + 1150 ms is accepted

Change-Id: I501c00aeda46bcbd434323f21c4b8433d04d2038
Bug: 63773
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1686858
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675798}
parent 5f4c8c5a
...@@ -191,13 +191,13 @@ jumbo_component("views") { ...@@ -191,13 +191,13 @@ jumbo_component("views") {
"drag_utils.h", "drag_utils.h",
"event_monitor.h", "event_monitor.h",
"event_monitor_mac.h", "event_monitor_mac.h",
"event_utils.h",
"focus/external_focus_tracker.h", "focus/external_focus_tracker.h",
"focus/focus_manager.h", "focus/focus_manager.h",
"focus/focus_manager_delegate.h", "focus/focus_manager_delegate.h",
"focus/focus_manager_factory.h", "focus/focus_manager_factory.h",
"focus/focus_search.h", "focus/focus_search.h",
"focus/widget_focus_manager.h", "focus/widget_focus_manager.h",
"input_event_activation_protector.h",
"layout/box_layout.h", "layout/box_layout.h",
"layout/fill_layout.h", "layout/fill_layout.h",
"layout/flex_layout.h", "layout/flex_layout.h",
...@@ -400,12 +400,12 @@ jumbo_component("views") { ...@@ -400,12 +400,12 @@ jumbo_component("views") {
"drag_utils.cc", "drag_utils.cc",
"drag_utils_mac.mm", "drag_utils_mac.mm",
"event_monitor_mac.mm", "event_monitor_mac.mm",
"event_utils.cc",
"focus/external_focus_tracker.cc", "focus/external_focus_tracker.cc",
"focus/focus_manager.cc", "focus/focus_manager.cc",
"focus/focus_manager_factory.cc", "focus/focus_manager_factory.cc",
"focus/focus_search.cc", "focus/focus_search.cc",
"focus/widget_focus_manager.cc", "focus/widget_focus_manager.cc",
"input_event_activation_protector.cc",
"layout/box_layout.cc", "layout/box_layout.cc",
"layout/fill_layout.cc", "layout/fill_layout.cc",
"layout/flex_layout.cc", "layout/flex_layout.cc",
......
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include "ui/views/controls/button/image_button.h" #include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/button/image_button_factory.h" #include "ui/views/controls/button/image_button_factory.h"
#include "ui/views/controls/image_view.h" #include "ui/views/controls/image_view.h"
#include "ui/views/event_utils.h"
#include "ui/views/layout/box_layout.h" #include "ui/views/layout/box_layout.h"
#include "ui/views/layout/layout_provider.h" #include "ui/views/layout/layout_provider.h"
#include "ui/views/paint_info.h" #include "ui/views/paint_info.h"
...@@ -404,9 +403,7 @@ void BubbleFrameView::ViewHierarchyChanged( ...@@ -404,9 +403,7 @@ void BubbleFrameView::ViewHierarchyChanged(
void BubbleFrameView::VisibilityChanged(View* starting_from, bool is_visible) { void BubbleFrameView::VisibilityChanged(View* starting_from, bool is_visible) {
NonClientFrameView::VisibilityChanged(starting_from, is_visible); NonClientFrameView::VisibilityChanged(starting_from, is_visible);
input_protector_.VisibilityChanged(is_visible);
if (is_visible)
view_shown_time_stamp_ = base::TimeTicks::Now();
} }
void BubbleFrameView::OnPaint(gfx::Canvas* canvas) { void BubbleFrameView::OnPaint(gfx::Canvas* canvas) {
...@@ -426,7 +423,7 @@ void BubbleFrameView::PaintChildren(const PaintInfo& paint_info) { ...@@ -426,7 +423,7 @@ void BubbleFrameView::PaintChildren(const PaintInfo& paint_info) {
} }
void BubbleFrameView::ButtonPressed(Button* sender, const ui::Event& event) { void BubbleFrameView::ButtonPressed(Button* sender, const ui::Event& event) {
if (IsPossiblyUnintendedInteraction(view_shown_time_stamp_, event)) if (input_protector_.IsPossiblyUnintendedInteraction(event))
return; return;
if (sender == close_) { if (sender == close_) {
...@@ -514,7 +511,7 @@ gfx::Rect BubbleFrameView::GetUpdatedWindowBounds( ...@@ -514,7 +511,7 @@ gfx::Rect BubbleFrameView::GetUpdatedWindowBounds(
} }
void BubbleFrameView::ResetViewShownTimeStampForTesting() { void BubbleFrameView::ResetViewShownTimeStampForTesting() {
view_shown_time_stamp_ = base::TimeTicks(); input_protector_.ResetForTesting();
} }
gfx::Rect BubbleFrameView::GetAvailableScreenBounds( gfx::Rect BubbleFrameView::GetAvailableScreenBounds(
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "ui/views/bubble/bubble_border.h" #include "ui/views/bubble/bubble_border.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
#include "ui/views/controls/label.h" #include "ui/views/controls/label.h"
#include "ui/views/input_event_activation_protector.h"
#include "ui/views/window/non_client_view.h" #include "ui/views/window/non_client_view.h"
namespace views { namespace views {
...@@ -216,9 +217,6 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView, ...@@ -216,9 +217,6 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
// A view to contain the footnote view, if it exists. // A view to contain the footnote view, if it exists.
FootnoteContainerView* footnote_container_; FootnoteContainerView* footnote_container_;
// Time when view has been shown.
base::TimeTicks view_shown_time_stamp_;
// Set preference for how the arrow will be adjusted if the window is outside // Set preference for how the arrow will be adjusted if the window is outside
// the available bounds. // the available bounds.
PreferredArrowAdjustment preferred_arrow_adjustment_ = PreferredArrowAdjustment preferred_arrow_adjustment_ =
...@@ -228,6 +226,8 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView, ...@@ -228,6 +226,8 @@ class VIEWS_EXPORT BubbleFrameView : public NonClientFrameView,
// hover). // hover).
bool hit_test_transparent_ = false; bool hit_test_transparent_ = false;
InputEventActivationProtector input_protector_;
DISALLOW_COPY_AND_ASSIGN(BubbleFrameView); DISALLOW_COPY_AND_ASSIGN(BubbleFrameView);
}; };
......
// Copyright (c) 2018 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 "ui/views/event_utils.h"
#include "base/time/time.h"
#include "ui/events/event.h"
#include "ui/views/metrics.h"
namespace views {
bool IsPossiblyUnintendedInteraction(const base::TimeTicks& initial_timestamp,
const ui::Event& event) {
return (event.IsMouseEvent() || event.IsTouchEvent()) &&
event.time_stamp() <
initial_timestamp +
base::TimeDelta::FromMilliseconds(GetDoubleClickInterval());
}
} // namespace views
// Copyright (c) 2018 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.
#ifndef UI_VIEWS_EVENT_UTILS_H_
#define UI_VIEWS_EVENT_UTILS_H_
#include "ui/views/views_export.h"
namespace base {
class TimeTicks;
}
namespace ui {
class Event;
}
namespace views {
// Returns true if the event is a mouse, touch, or pointer event that took place
// within the double-click time interval after the |initial_timestamp|.
VIEWS_EXPORT bool IsPossiblyUnintendedInteraction(
const base::TimeTicks& initial_timestamp,
const ui::Event& event);
} // namespace views
#endif // UI_VIEWS_EVENT_UTILS_H_
// Copyright (c) 2018 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 "ui/views/input_event_activation_protector.h"
#include "ui/events/event.h"
#include "ui/views/metrics.h"
namespace views {
void InputEventActivationProtector::VisibilityChanged(bool is_visible) {
if (is_visible)
view_shown_time_stamp_ = base::TimeTicks::Now();
}
bool InputEventActivationProtector::IsPossiblyUnintendedInteraction(
const ui::Event& event) {
if (view_shown_time_stamp_ == base::TimeTicks()) {
// The UI was never shown, ignore. This can happen in tests.
return false;
}
if (!event.IsMouseEvent() && !event.IsTouchEvent())
return false;
const base::TimeDelta kShortInterval =
base::TimeDelta::FromMilliseconds(GetDoubleClickInterval());
const bool short_event_after_last_event =
event.time_stamp() < last_event_timestamp_ + kShortInterval;
last_event_timestamp_ = event.time_stamp();
// Unintended if the user has been clicking with short intervals.
if (short_event_after_last_event) {
repeated_event_count_++;
return true;
}
repeated_event_count_ = 0;
// Unintended if the user clicked right after the UI showed.
return event.time_stamp() < view_shown_time_stamp_ + kShortInterval;
}
void InputEventActivationProtector::ResetForTesting() {
view_shown_time_stamp_ = base::TimeTicks();
last_event_timestamp_ = base::TimeTicks();
repeated_event_count_ = 0;
}
} // namespace views
// Copyright (c) 2018 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.
#ifndef UI_VIEWS_INPUT_EVENT_ACTIVATION_PROTECTOR_H_
#define UI_VIEWS_INPUT_EVENT_ACTIVATION_PROTECTOR_H_
#include "base/macros.h"
#include "base/time/time.h"
#include "ui/views/views_export.h"
namespace ui {
class Event;
}
namespace views {
// The goal of this class is to prevent potentially unintentional user
// interaction with a UI element.
class VIEWS_EXPORT InputEventActivationProtector {
public:
InputEventActivationProtector() = default;
~InputEventActivationProtector() = default;
// Updates the state of the protector based off of visibility changes. This
// method must be called when the visibility of the view is changed.
void VisibilityChanged(bool is_visible);
// Returns true if the event is a mouse, touch, or pointer event that took
// place within the double-click time interval after |view_shown_time_stamp_|.
bool IsPossiblyUnintendedInteraction(const ui::Event& event);
// Resets the state for click tracking.
void ResetForTesting();
private:
// Timestamp of when the view being tracked is first shown.
base::TimeTicks view_shown_time_stamp_;
// Timestamp of the last event.
base::TimeTicks last_event_timestamp_;
// Number of repeated UI events with short intervals.
size_t repeated_event_count_ = 0;
DISALLOW_COPY_AND_ASSIGN(InputEventActivationProtector);
};
} // namespace views
#endif // UI_VIEWS_INPUT_EVENT_ACTIVATION_PROTECTOR_H_
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "ui/views/controls/button/image_button.h" #include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/button/label_button.h" #include "ui/views/controls/button/label_button.h"
#include "ui/views/controls/button/md_text_button.h" #include "ui/views/controls/button/md_text_button.h"
#include "ui/views/event_utils.h"
#include "ui/views/layout/grid_layout.h" #include "ui/views/layout/grid_layout.h"
#include "ui/views/layout/layout_provider.h" #include "ui/views/layout/layout_provider.h"
#include "ui/views/style/platform_style.h" #include "ui/views/style/platform_style.h"
...@@ -167,9 +166,7 @@ gfx::Size DialogClientView::GetMaximumSize() const { ...@@ -167,9 +166,7 @@ gfx::Size DialogClientView::GetMaximumSize() const {
void DialogClientView::VisibilityChanged(View* starting_from, bool is_visible) { void DialogClientView::VisibilityChanged(View* starting_from, bool is_visible) {
ClientView::VisibilityChanged(starting_from, is_visible); ClientView::VisibilityChanged(starting_from, is_visible);
input_protector_.VisibilityChanged(is_visible);
if (is_visible)
view_shown_time_stamp_ = base::TimeTicks::Now();
} }
void DialogClientView::Layout() { void DialogClientView::Layout() {
...@@ -242,7 +239,7 @@ void DialogClientView::ButtonPressed(Button* sender, const ui::Event& event) { ...@@ -242,7 +239,7 @@ void DialogClientView::ButtonPressed(Button* sender, const ui::Event& event) {
if (!GetDialogDelegate()) if (!GetDialogDelegate())
return; return;
if (IsPossiblyUnintendedInteraction(view_shown_time_stamp_, event)) if (input_protector_.IsPossiblyUnintendedInteraction(event))
return; return;
if (sender == ok_button_) if (sender == ok_button_)
...@@ -254,7 +251,7 @@ void DialogClientView::ButtonPressed(Button* sender, const ui::Event& event) { ...@@ -254,7 +251,7 @@ void DialogClientView::ButtonPressed(Button* sender, const ui::Event& event) {
} }
void DialogClientView::ResetViewShownTimeStampForTesting() { void DialogClientView::ResetViewShownTimeStampForTesting() {
view_shown_time_stamp_ = base::TimeTicks(); input_protector_.ResetForTesting();
} }
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
#include "ui/views/input_event_activation_protector.h"
#include "ui/views/window/client_view.h" #include "ui/views/window/client_view.h"
#include "ui/views/window/dialog_observer.h" #include "ui/views/window/dialog_observer.h"
...@@ -142,8 +143,7 @@ class VIEWS_EXPORT DialogClientView : public ClientView, ...@@ -142,8 +143,7 @@ class VIEWS_EXPORT DialogClientView : public ClientView,
// SetupLayout(). Everything will be manually updated afterwards. // SetupLayout(). Everything will be manually updated afterwards.
bool adding_or_removing_views_ = false; bool adding_or_removing_views_ = false;
// Time when view has been shown. InputEventActivationProtector input_protector_;
base::TimeTicks view_shown_time_stamp_;
DISALLOW_COPY_AND_ASSIGN(DialogClientView); DISALLOW_COPY_AND_ASSIGN(DialogClientView);
}; };
......
...@@ -514,10 +514,11 @@ TEST_F(DialogClientViewTest, FocusChangingButtons) { ...@@ -514,10 +514,11 @@ TEST_F(DialogClientViewTest, FocusChangingButtons) {
} }
// Ensures that clicks are ignored for short time after view has been shown. // Ensures that clicks are ignored for short time after view has been shown.
TEST_F(DialogClientViewTest, IgnorePossiblyUnintendedClicks) { TEST_F(DialogClientViewTest, IgnorePossiblyUnintendedClicks_ClickAfterShown) {
widget()->Show(); widget()->Show();
SetDialogButtons(ui::DIALOG_BUTTON_CANCEL | ui::DIALOG_BUTTON_OK); SetDialogButtons(ui::DIALOG_BUTTON_CANCEL | ui::DIALOG_BUTTON_OK);
// Should ignore clicks right after the dialog is shown.
ui::MouseEvent mouse_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), ui::MouseEvent mouse_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE); ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE);
client_view()->ButtonPressed(client_view()->ok_button(), mouse_event); client_view()->ButtonPressed(client_view()->ok_button(), mouse_event);
...@@ -533,4 +534,45 @@ TEST_F(DialogClientViewTest, IgnorePossiblyUnintendedClicks) { ...@@ -533,4 +534,45 @@ TEST_F(DialogClientViewTest, IgnorePossiblyUnintendedClicks) {
EXPECT_TRUE(widget()->IsClosed()); EXPECT_TRUE(widget()->IsClosed());
} }
// Ensures that repeated clicks with short intervals after view has been shown
// are also ignored.
TEST_F(DialogClientViewTest, IgnorePossiblyUnintendedClicks_RepeatedClicks) {
widget()->Show();
SetDialogButtons(ui::DIALOG_BUTTON_CANCEL | ui::DIALOG_BUTTON_OK);
const base::TimeTicks kNow = ui::EventTimeForNow();
const base::TimeDelta kShortClickInterval =
base::TimeDelta::FromMilliseconds(GetDoubleClickInterval());
// Should ignore clicks right after the dialog is shown.
ui::MouseEvent mouse_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
kNow, ui::EF_NONE, ui::EF_NONE);
client_view()->ButtonPressed(client_view()->ok_button(), mouse_event);
client_view()->ButtonPressed(client_view()->cancel_button(), mouse_event);
EXPECT_FALSE(widget()->IsClosed());
// Should ignore repeated clicks with short intervals, even though enough time
// has passed since the dialog was shown.
const base::TimeDelta kRepeatedClickInterval = kShortClickInterval / 2;
const size_t kNumClicks = 4;
ASSERT_TRUE(kNumClicks * kRepeatedClickInterval > kShortClickInterval);
base::TimeTicks event_time = kNow;
for (size_t i = 0; i < kNumClicks; i++) {
client_view()->ButtonPressed(
client_view()->cancel_button(),
ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
event_time, ui::EF_NONE, ui::EF_NONE));
EXPECT_FALSE(widget()->IsClosed());
event_time += kRepeatedClickInterval;
}
// Sufficient time passed, events are now allowed.
event_time += kShortClickInterval;
client_view()->ButtonPressed(
client_view()->cancel_button(),
ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
event_time, ui::EF_NONE, ui::EF_NONE));
EXPECT_TRUE(widget()->IsClosed());
}
} // namespace views } // namespace views
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment