Skip to content

Commit

Permalink
Use event source-id to filter out remote CRD input
Browse files Browse the repository at this point in the history
During a CRD session input can come from 2 sources:
   - The remote user (through CRD).
   - The local user.

All remote input is blocked for 2 seconds when local input is detected,
because we do not want the remote user to steal control of the local user.

Previously this was detected by remembering the location of all remote input,
and then monitoring all local input and filtering out the echos of the
remote input.
However this is not 100% accurate, so in this CL I introduce a system
to use the event's source-id to filter out the echos of the remote
commands.

Bug: b/208370410
Test: manually deployed and tested
Change-Id: I1dfd2afe0ebcd611219509a64b9f7f7bc3faac2a
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3440148
Reviewed-by: Ben Franz <bfranz@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Auto-Submit: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987305}
  • Loading branch information
Jeroen Dhollander authored and Chromium LUCI CQ committed Mar 31, 2022
1 parent 5b26d25 commit 89e6a4d
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 24 deletions.
8 changes: 4 additions & 4 deletions remoting/host/client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ ClientSession::ClientSession(
extension_manager_ =
std::make_unique<HostExtensionSessionManager>(extensions, this);

#if BUILDFLAG(IS_WIN)
// LocalInputMonitorWin filters out an echo of the injected input before it
// reaches |remote_input_filter_|.
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
// LocalMouseInputMonitorWin and LocalPointerInputMonitorChromeos filter out
// an echo of the injected input before it reaches |remote_input_filter_|.
remote_input_filter_.SetExpectLocalEcho(false);
#endif // BUILDFLAG(IS_WIN)
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_CHROMEOS)
}

ClientSession::~ClientSession() {
Expand Down
4 changes: 2 additions & 2 deletions remoting/host/client_session_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ TEST_F(ClientSessionTest, LocalInputTest) {

connection_->input_stub()->InjectMouseEvent(MakeMouseMoveEvent(100, 101));

#if !BUILDFLAG(IS_WIN)
#if !BUILDFLAG(IS_WIN) && !BUILDFLAG(IS_CHROMEOS)
// The OS echoes the injected event back.
client_session_->OnLocalPointerMoved(webrtc::DesktopVector(100, 101),
ui::ET_MOUSE_MOVED);
Expand All @@ -617,7 +617,7 @@ TEST_F(ClientSessionTest, LocalInputTest) {
connection_->input_stub()->InjectMouseEvent(MakeMouseMoveEvent(300, 301));

// Verify that we've received correct set of mouse events.
EXPECT_EQ(2U, mouse_events.size());
ASSERT_EQ(2U, mouse_events.size());
EXPECT_THAT(mouse_events[0], EqualsMouseMoveEvent(100, 101));
EXPECT_THAT(mouse_events[1], EqualsMouseMoveEvent(200, 201));

Expand Down
3 changes: 3 additions & 0 deletions remoting/host/input_injector_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class SystemInputInjectorStub : public ui::SystemInputInjector {
~SystemInputInjectorStub() override = default;

// SystemInputInjector implementation:
void SetDeviceId(int device_id) override {}
void MoveCursorTo(const gfx::PointF& location) override {}
void InjectMouseButton(ui::EventFlags button, bool down) override {}
void InjectMouseWheel(int delta_x, int delta_y) override {}
Expand Down Expand Up @@ -232,6 +233,8 @@ void InputInjectorChromeos::Core::Start(
}
DCHECK(delegate_);

delegate_->SetDeviceId(ui::ED_REMOTE_INPUT_DEVICE);

// Implemented by remoting::ClipboardAura.
clipboard_ = Clipboard::Create();
clipboard_->Start(std::move(client_clipboard));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ namespace remoting {

namespace {

bool IsInjectedByCrd(const ui::PlatformEvent& event) {
return event->source_device_id() == ui::ED_REMOTE_INPUT_DEVICE;
}

class LocalKeyboardInputMonitorChromeos : public LocalKeyboardInputMonitor {
public:
LocalKeyboardInputMonitorChromeos(
Expand Down Expand Up @@ -105,6 +109,11 @@ void LocalKeyboardInputMonitorChromeos::Core::WillProcessEvent(

void LocalKeyboardInputMonitorChromeos::Core::DidProcessEvent(
const ui::PlatformEvent& event) {
// Do not pass on events remotely injected by CRD, as we're supposed to
// monitor for local input only.
if (IsInjectedByCrd(event))
return;

ui::EventType type = ui::EventTypeFromNative(event);
if (type == ui::ET_KEY_PRESSED) {
ui::DomCode dom_code = ui::CodeFromNative(event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "remoting/host/chromeos/point_transformer.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
#include "ui/events/event.h"
#include "ui/events/event_constants.h"
#include "ui/events/event_utils.h"
#include "ui/events/platform/platform_event_observer.h"
#include "ui/events/platform/platform_event_source.h"
Expand All @@ -22,6 +23,10 @@ namespace remoting {

namespace {

bool IsInjectedByCrd(const ui::PlatformEvent& event) {
return event->source_device_id() == ui::ED_REMOTE_INPUT_DEVICE;
}

class LocalPointerInputMonitorChromeos : public LocalPointerInputMonitor {
public:
LocalPointerInputMonitorChromeos(
Expand Down Expand Up @@ -113,6 +118,11 @@ void LocalPointerInputMonitorChromeos::Core::WillProcessEvent(

void LocalPointerInputMonitorChromeos::Core::DidProcessEvent(
const ui::PlatformEvent& event) {
// Do not pass on events remotely injected by CRD, as we're supposed to
// monitor for local input only.
if (IsInjectedByCrd(event))
return;

ui::EventType type = ui::EventTypeFromNative(event);
if (type == ui::ET_MOUSE_MOVED || type == ui::ET_TOUCH_MOVED) {
HandlePointerMove(event, type);
Expand Down
4 changes: 3 additions & 1 deletion ui/chromeos/events/event_rewriter_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1061,8 +1061,10 @@ bool EventRewriterChromeOS::IsLastKeyboardOfType(DeviceType device_type) const {

EventRewriterChromeOS::DeviceType EventRewriterChromeOS::GetLastKeyboardType()
const {
if (last_keyboard_device_id_ == ED_UNKNOWN_DEVICE)
if ((last_keyboard_device_id_ == ED_UNKNOWN_DEVICE) ||
(last_keyboard_device_id_ == ED_REMOTE_INPUT_DEVICE)) {
return kDeviceUnknown;
}

const auto iter = device_id_to_info_.find(last_keyboard_device_id_);
if (iter == device_id_to_info_.end()) {
Expand Down
8 changes: 6 additions & 2 deletions ui/events/event_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,12 @@ enum class EventMomentumPhase {
BLOCKED,
};

// Device ID for Touch and Key Events.
enum EventDeviceId { ED_UNKNOWN_DEVICE = -1 };
enum EventDeviceId {
// Device ID for Touch, Mouse and Key Events.
ED_UNKNOWN_DEVICE = -1,
// Device ID for events injected through a remote connection (like CRD).
ED_REMOTE_INPUT_DEVICE = -2,
};

// Pointing device type.
enum class EventPointerType : int {
Expand Down
24 changes: 11 additions & 13 deletions ui/events/ozone/evdev/input_injector_evdev.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@

namespace ui {

namespace {

const int kDeviceIdForInjection = -1;

} // namespace

InputInjectorEvdev::InputInjectorEvdev(
std::unique_ptr<DeviceEventDispatcherEvdev> dispatcher,
CursorDelegateEvdev* cursor)
: cursor_(cursor), dispatcher_(std::move(dispatcher)) {}

InputInjectorEvdev::~InputInjectorEvdev() = default;

void InputInjectorEvdev::SetDeviceId(int device_id) {
device_id_ = device_id;
}

void InputInjectorEvdev::InjectMouseButton(EventFlags button, bool down) {
unsigned int code;
switch (button) {
Expand All @@ -50,15 +48,15 @@ void InputInjectorEvdev::InjectMouseButton(EventFlags button, bool down) {
}

dispatcher_->DispatchMouseButtonEvent(MouseButtonEventParams(
kDeviceIdForInjection, EF_NONE, cursor_->GetLocation(), code, down,
device_id_, EF_NONE, cursor_->GetLocation(), code, down,
MouseButtonMapType::kNone, PointerDetails(EventPointerType::kMouse),
EventTimeForNow()));
}

void InputInjectorEvdev::InjectMouseWheel(int delta_x, int delta_y) {
dispatcher_->DispatchMouseWheelEvent(MouseWheelEventParams(
kDeviceIdForInjection, cursor_->GetLocation(),
gfx::Vector2d(delta_x, delta_y), EventTimeForNow()));
device_id_, cursor_->GetLocation(), gfx::Vector2d(delta_x, delta_y),
EventTimeForNow()));
}

void InputInjectorEvdev::MoveCursorTo(const gfx::PointF& location) {
Expand All @@ -76,7 +74,7 @@ void InputInjectorEvdev::MoveCursorTo(const gfx::PointF& location) {
const int event_flags = EF_NOT_SUITABLE_FOR_MOUSE_WARPING;

dispatcher_->DispatchMouseMoveEvent(MouseMoveEventParams(
kDeviceIdForInjection, event_flags, cursor_->GetLocation(),
device_id_, event_flags, cursor_->GetLocation(),
nullptr /* ordinal_delta */, PointerDetails(EventPointerType::kMouse),
EventTimeForNow()));
}
Expand All @@ -88,9 +86,9 @@ void InputInjectorEvdev::InjectKeyEvent(DomCode physical_key,
return;

int evdev_code = KeycodeConverter::DomCodeToEvdevCode(physical_key);
dispatcher_->DispatchKeyEvent(KeyEventParams(
kDeviceIdForInjection, ui::EF_NONE, evdev_code, 0 /*scan_code*/, down,
suppress_auto_repeat, EventTimeForNow()));
dispatcher_->DispatchKeyEvent(
KeyEventParams(device_id_, EF_NONE, evdev_code, 0 /*scan_code*/, down,
suppress_auto_repeat, EventTimeForNow()));
}

} // namespace ui
4 changes: 4 additions & 0 deletions ui/events/ozone/evdev/input_injector_evdev.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define UI_EVENTS_OZONE_EVDEV_INPUT_INJECTOR_EVDEV_H_

#include "base/component_export.h"
#include "ui/events/event_constants.h"
#include "ui/events/ozone/evdev/event_dispatch_callback.h"
#include "ui/ozone/public/system_input_injector.h"

Expand All @@ -25,6 +26,7 @@ class COMPONENT_EXPORT(EVDEV) InputInjectorEvdev : public SystemInputInjector {
~InputInjectorEvdev() override;

// SystemInputInjector implementation.
void SetDeviceId(int device_id) override;
void InjectMouseButton(EventFlags button, bool down) override;
void InjectMouseWheel(int delta_x, int delta_y) override;
void MoveCursorTo(const gfx::PointF& location) override;
Expand All @@ -36,6 +38,8 @@ class COMPONENT_EXPORT(EVDEV) InputInjectorEvdev : public SystemInputInjector {
// Shared cursor state.
CursorDelegateEvdev* const cursor_;

int device_id_ = ED_UNKNOWN_DEVICE;

// Interface for dispatching events.
const std::unique_ptr<DeviceEventDispatcherEvdev> dispatcher_;
};
Expand Down
8 changes: 6 additions & 2 deletions ui/ozone/public/system_input_injector.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@ namespace ui {
// native events.
class COMPONENT_EXPORT(OZONE) SystemInputInjector {
public:
SystemInputInjector() {}
SystemInputInjector() = default;

SystemInputInjector(const SystemInputInjector&) = delete;
SystemInputInjector& operator=(const SystemInputInjector&) = delete;

virtual ~SystemInputInjector() {}
virtual ~SystemInputInjector() = default;

// Set the device id that will be used for all the generated events.
// The device id is set to |ui::ED_UNKNOWN_DEVICE| by default.
virtual void SetDeviceId(int device_id) = 0;

// Moves the cursor on the screen and generates the corresponding MouseMove or
// MouseDragged event. |location| is in physical screen coordinates,
Expand Down

0 comments on commit 89e6a4d

Please sign in to comment.