0

[crd host] Report more disconnect reasons to the client

* Remove the Session::Close overload that only takes `error`, and change
  existing code to provide the extra details.
* Update ConnectionToClient::Disconnect and
  ClientSession::DisconnectSession to take the extra details, and update
  existing code to provide them.

Bug: 382334458, b/379187218
Change-Id: I173849a83c08a92db171e022fb8283668a444e8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6349923
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Auto-Submit: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1432406}
This commit is contained in:
Yuwei Huang
2025-03-13 15:15:08 -07:00
committed by Chromium LUCI CQ
parent 16a923e562
commit 8e6e6ad1a3
39 changed files with 248 additions and 142 deletions

@ -14,6 +14,7 @@
#include "base/command_line.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/location.h"
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
#include "base/task/single_thread_task_runner.h"
@ -106,7 +107,8 @@ ChromotingHost::~ChromotingHost() {
// Disconnect all of the clients.
while (!clients_.empty()) {
clients_.front()->DisconnectSession(ErrorCode::OK);
clients_.front()->DisconnectSession(ErrorCode::OK, /* error_details= */ {},
FROM_HERE);
}
// Destroy the session manager to make sure that |signal_strategy_| does not
@ -172,10 +174,12 @@ void ChromotingHost::OnSessionAuthenticating(ClientSession* client) {
// authenticates. This allows the backoff to protect from parallel
// connection attempts as well as sequential ones.
if (login_backoff_.ShouldRejectRequest()) {
LOG(WARNING) << "Disconnecting client " << client->client_jid()
<< " due to"
" an overload of failed login attempts.";
client->DisconnectSession(ErrorCode::HOST_OVERLOAD);
client->DisconnectSession(
ErrorCode::HOST_OVERLOAD,
base::StringPrintf("Disconnecting client %s due to an overload of "
"failed login attempts.",
client->client_jid().c_str()),
FROM_HERE);
return;
}
login_backoff_.InformOfRequest(false);
@ -190,7 +194,9 @@ void ChromotingHost::OnSessionAuthenticated(ClientSession* client) {
base::WeakPtr<ChromotingHost> self = weak_factory_.GetWeakPtr();
while (clients_.size() > 1) {
clients_[(clients_.front().get() == client) ? 1 : 0]->DisconnectSession(
ErrorCode::OK);
ErrorCode::OK,
"Disconnecting session because a new session has been authenticated.",
FROM_HERE);
// Quit if the host was destroyed.
if (!self) {
@ -296,14 +302,17 @@ void ChromotingHost::BindSessionServices(
void ChromotingHost::OnIncomingSession(
protocol::Session* session,
protocol::SessionManager::IncomingSessionResponse* response) {
protocol::SessionManager::IncomingSessionResponse* response,
std::string* rejection_reason,
base::Location* rejection_location) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(started_);
if (login_backoff_.ShouldRejectRequest()) {
LOG(WARNING) << "Rejecting connection due to"
" an overload of failed login attempts.";
*response = protocol::SessionManager::OVERLOAD;
*rejection_reason =
"Rejecting connection due to an overload of failed login attempts.";
*rejection_location = FROM_HERE;
return;
}

@ -162,7 +162,9 @@ class ChromotingHost : public ClientSession::EventHandler,
// Callback for SessionManager to accept incoming sessions.
void OnIncomingSession(
protocol::Session* session,
protocol::SessionManager::IncomingSessionResponse* response);
protocol::SessionManager::IncomingSessionResponse* response,
std::string* rejection_reason,
base::Location* rejection_location);
// The host uses a pairing registry to generate and store pairing information
// for clients for PIN-less authentication.

@ -18,6 +18,7 @@
#include <vector>
#include "base/functional/callback_helpers.h"
#include "base/location.h"
#include "base/memory/ptr_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/scoped_refptr.h"
@ -432,8 +433,13 @@ TEST_F(ChromotingHostTest, IncomingSessionAccepted) {
MockSession* session = session_unowned1_.get();
protocol::SessionManager::IncomingSessionResponse response =
protocol::SessionManager::DECLINE;
host_->OnIncomingSession(session_unowned1_.release(), &response);
std::string rejection_reason;
base::Location rejection_location;
host_->OnIncomingSession(session_unowned1_.release(), &response,
&rejection_reason, &rejection_location);
EXPECT_EQ(protocol::SessionManager::ACCEPT, response);
EXPECT_TRUE(rejection_reason.empty());
EXPECT_EQ(nullptr, rejection_location.program_counter());
EXPECT_CALL(*session, Close(_, _, _))
.WillOnce(InvokeWithoutArgs(
@ -446,6 +452,8 @@ TEST_F(ChromotingHostTest, LoginBackOffTriggersIfClientsDoNotAuthenticate) {
protocol::SessionManager::IncomingSessionResponse response =
protocol::SessionManager::DECLINE;
std::string rejection_reason;
base::Location rejection_location;
std::array<protocol::Session::EventHandler*, kNumFailuresIgnored + 1>
session_event_handlers;
for (auto*& session_event_handler : session_event_handlers) {
@ -462,8 +470,11 @@ TEST_F(ChromotingHostTest, LoginBackOffTriggersIfClientsDoNotAuthenticate) {
session_event_handler->OnSessionStateChange(Session::CLOSED);
}));
// Simulate the incoming connection.
host_->OnIncomingSession(session.release(), &response);
host_->OnIncomingSession(session.release(), &response, &rejection_reason,
&rejection_location);
EXPECT_EQ(protocol::SessionManager::ACCEPT, response);
EXPECT_TRUE(rejection_reason.empty());
EXPECT_EQ(nullptr, rejection_location.program_counter());
// Begin authentication; this will increase the backoff count, and since
// OnSessionAuthenticated is never called, the host should only allow
// kNumFailuresIgnored + 1 connections before beginning the backoff.
@ -472,8 +483,11 @@ TEST_F(ChromotingHostTest, LoginBackOffTriggersIfClientsDoNotAuthenticate) {
}
// As this is connection kNumFailuresIgnored + 2, it should be rejected.
host_->OnIncomingSession(session_unowned2_.get(), &response);
host_->OnIncomingSession(session_unowned2_.get(), &response,
&rejection_reason, &rejection_location);
EXPECT_EQ(protocol::SessionManager::OVERLOAD, response);
EXPECT_FALSE(rejection_reason.empty());
EXPECT_NE(nullptr, rejection_location.program_counter());
EXPECT_EQ(host_->client_sessions_for_tests().size(), kNumFailuresIgnored + 1);
// Shut down host while objects owned by this test are still in scope.
@ -485,6 +499,8 @@ TEST_F(ChromotingHostTest, LoginBackOffResetsIfClientsAuthenticate) {
protocol::SessionManager::IncomingSessionResponse response =
protocol::SessionManager::DECLINE;
std::string rejection_reason;
base::Location rejection_location;
std::array<protocol::Session::EventHandler*, kNumFailuresIgnored + 1>
session_event_handlers;
for (auto*& session_event_handler : session_event_handlers) {
@ -501,8 +517,11 @@ TEST_F(ChromotingHostTest, LoginBackOffResetsIfClientsAuthenticate) {
session_event_handler->OnSessionStateChange(Session::CLOSED);
}));
// Simulate the incoming connection.
host_->OnIncomingSession(session.release(), &response);
host_->OnIncomingSession(session.release(), &response, &rejection_reason,
&rejection_location);
EXPECT_EQ(protocol::SessionManager::ACCEPT, response);
EXPECT_TRUE(rejection_reason.empty());
EXPECT_EQ(nullptr, rejection_location.program_counter());
// Begin authentication; this will increase the backoff count
host_->OnSessionAuthenticating(
host_->client_sessions_for_tests().front().get());
@ -527,8 +546,11 @@ TEST_F(ChromotingHostTest, LoginBackOffResetsIfClientsAuthenticate) {
.WillOnce(InvokeWithoutArgs([&session_event_handler]() {
session_event_handler->OnSessionStateChange(Session::CLOSED);
}));
host_->OnIncomingSession(session.release(), &response);
host_->OnIncomingSession(session.release(), &response, &rejection_reason,
&rejection_location);
EXPECT_EQ(protocol::SessionManager::ACCEPT, response);
EXPECT_TRUE(rejection_reason.empty());
EXPECT_EQ(nullptr, rejection_location.program_counter());
// Shut down host while objects owned by this test are still in scope.
ShutdownHost();

@ -25,6 +25,7 @@
#include "base/memory/scoped_refptr.h"
#include "base/sequence_checker.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
#include "base/task/single_thread_task_runner.h"
#include "base/time/time.h"
#include "build/build_config.h"
@ -563,9 +564,12 @@ void ClientSession::OnConnectionAuthenticated(
event_handler_->OnSessionPoliciesReceived(effective_policies_);
if (validation_result.has_value()) {
LOG(ERROR) << "Session policies disallowed by validator. Error code: "
<< static_cast<int>(*validation_result);
DisconnectSession(*validation_result);
// TODO: crbug.com/382334458 - Include error details and location in the
// validation result.
std::string error_details = base::StringPrintf(
"Session policies disallowed by validator. Error code: %d",
static_cast<int>(*validation_result));
DisconnectSession(*validation_result, error_details, FROM_HERE);
return;
}
@ -575,7 +579,9 @@ void ClientSession::OnConnectionAuthenticated(
max_duration_timer_.Start(
FROM_HERE, max_duration,
base::BindOnce(&ClientSession::DisconnectSession,
base::Unretained(this), ErrorCode::MAX_SESSION_LENGTH));
base::Unretained(this), ErrorCode::MAX_SESSION_LENGTH,
"Maximum session duration has been reached.",
FROM_HERE));
}
// Notify EventHandler.
@ -823,7 +829,9 @@ const std::string& ClientSession::client_jid() const {
return client_jid_;
}
void ClientSession::DisconnectSession(protocol::ErrorCode error) {
void ClientSession::DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(connection_.get());
@ -831,16 +839,17 @@ void ClientSession::DisconnectSession(protocol::ErrorCode error) {
// This triggers OnConnectionClosed(), and the session may be destroyed
// as the result, so this call must be the last in this method.
connection_->Disconnect(error);
connection_->Disconnect(error, error_details, error_location);
}
void ClientSession::OnLocalKeyPressed(std::uint32_t usb_keycode) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
bool is_local = remote_input_filter_.LocalKeyPressed(usb_keycode);
if (is_local && desktop_environment_options_.terminate_upon_input()) {
LOG(WARNING)
<< "Disconnecting CRD session because local input was detected.";
DisconnectSession(ErrorCode::OK);
DisconnectSession(
ErrorCode::OK,
"Disconnecting CRD session because local keyboard input was detected.",
FROM_HERE);
}
}
@ -850,9 +859,10 @@ void ClientSession::OnLocalPointerMoved(const webrtc::DesktopVector& position,
bool is_local = remote_input_filter_.LocalPointerMoved(position, type);
if (is_local) {
if (desktop_environment_options_.terminate_upon_input()) {
LOG(WARNING)
<< "Disconnecting CRD session because local input was detected.";
DisconnectSession(ErrorCode::OK);
DisconnectSession(
ErrorCode::OK,
"Disconnecting CRD session because local mouse input was detected.",
FROM_HERE);
} else {
desktop_and_cursor_composer_notifier_.OnLocalInput();
}
@ -1027,7 +1037,8 @@ void ClientSession::OnDesktopEnvironmentCreated(
// Drop the connection if it could not be created for any reason (for instance
// the curtain could not initialize).
if (!desktop_environment) {
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR,
"Failed to create desktop environment.", FROM_HERE);
return;
}
desktop_environment_ = std::move(desktop_environment);
@ -1095,8 +1106,9 @@ void ClientSession::OnDesktopEnvironmentCreated(
void ClientSession::OnLocalSessionPoliciesChanged(
const SessionPolicies& new_policies) {
DCHECK(local_session_policy_update_subscription_);
HOST_LOG << "Effective policies have changed. Terminating session.";
DisconnectSession(ErrorCode::SESSION_POLICIES_CHANGED);
DisconnectSession(ErrorCode::SESSION_POLICIES_CHANGED,
"Effective policies have changed. Terminating session.",
FROM_HERE);
}
void ClientSession::OnVideoSizeChanged(protocol::VideoStream* video_stream,

@ -179,7 +179,9 @@ class ClientSession : public protocol::HostStub,
// ClientSessionControl interface.
const std::string& client_jid() const override;
void DisconnectSession(protocol::ErrorCode error) override;
void DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
void OnLocalKeyPressed(std::uint32_t usb_keycode) override;
void OnLocalPointerMoved(const webrtc::DesktopVector& position,
ui::EventType type) override;

@ -30,7 +30,9 @@ class ClientSessionControl {
// Disconnects the client session, tears down transport resources and stops
// scheduler components.
virtual void DisconnectSession(protocol::ErrorCode error) = 0;
virtual void DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) = 0;
// Called when local mouse or touch movement is detected.
virtual void OnLocalPointerMoved(const webrtc::DesktopVector& position,

@ -274,7 +274,7 @@ void ClientSessionTest::SetUp() {
void ClientSessionTest::TearDown() {
if (client_session_) {
if (connection_->is_connected()) {
client_session_->DisconnectSession(ErrorCode::OK);
client_session_->DisconnectSession(ErrorCode::OK, {}, FROM_HERE);
}
client_session_.reset();
desktop_environment_factory_.reset();
@ -643,7 +643,7 @@ TEST_F(ClientSessionTest, MultiMonMouseMove) {
// Events should clamp to the entire desktop (800+1024, 35+768).
connection_->input_stub()->InjectMouseEvent(MakeMouseMoveEvent(2000, 1000));
client_session_->DisconnectSession(ErrorCode::OK);
client_session_->DisconnectSession(ErrorCode::OK, {}, FROM_HERE);
client_session_.reset();
EXPECT_EQ(7U, mouse_events_.size());
@ -699,7 +699,7 @@ TEST_F(ClientSessionTest, MultiMonMouseMove_SameSize) {
// Events should clamp to the entire desktop (800+800, 35+600).
connection_->input_stub()->InjectMouseEvent(MakeMouseMoveEvent(2000, 1000));
client_session_->DisconnectSession(ErrorCode::OK);
client_session_->DisconnectSession(ErrorCode::OK, {}, FROM_HERE);
client_session_.reset();
EXPECT_EQ(7U, mouse_events_.size());
@ -754,7 +754,7 @@ TEST_F(ClientSessionTest, DisableInputs) {
connection_->input_stub()->InjectKeyEvent(MakeKeyEvent(true, 3));
connection_->input_stub()->InjectMouseEvent(MakeMouseMoveEvent(300, 301));
client_session_->DisconnectSession(ErrorCode::OK);
client_session_->DisconnectSession(ErrorCode::OK, {}, FROM_HERE);
client_session_.reset();
EXPECT_EQ(2U, mouse_events_.size());
@ -844,7 +844,7 @@ TEST_F(ClientSessionTest, RestoreEventState) {
mousedown.set_button_down(true);
connection_->input_stub()->InjectMouseEvent(mousedown);
client_session_->DisconnectSession(ErrorCode::OK);
client_session_->DisconnectSession(ErrorCode::OK, {}, FROM_HERE);
client_session_.reset();
EXPECT_EQ(2U, mouse_events_.size());

@ -55,7 +55,9 @@ void ContinueWindow::DisconnectSession() {
disconnect_timer_.Stop();
if (client_session_control_) {
client_session_control_->DisconnectSession(ErrorCode::MAX_SESSION_LENGTH);
client_session_control_->DisconnectSession(
ErrorCode::MAX_SESSION_LENGTH,
"Maximum session duration has been reached.", FROM_HERE);
}
}

@ -21,6 +21,7 @@
#include "base/logging.h"
#include "base/mac/login_util.h"
#include "base/memory/ptr_util.h"
#include "base/strings/stringprintf.h"
#include "base/task/single_thread_task_runner.h"
#include "remoting/base/errors.h"
#include "remoting/host/client_session_control.h"
@ -93,7 +94,9 @@ class SessionWatcher : public base::RefCountedThreadSafe<SessionWatcher> {
void RemoveEventHandler();
// Disconnects the client session.
void DisconnectSession(ErrorCode error);
void DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location);
// Handlers for the switch-in event.
static OSStatus SessionActivateHandler(EventHandlerCallRef handler,
@ -156,16 +159,17 @@ void SessionWatcher::ActivateCurtain() {
// In case of fast user switch, there will be two host processes running,
// one as the logged on user and another one as root. AgentProcessBroker
// will terminate the root host process in that case.
LOG(ERROR)
<< "Connecting to the console login session is not yet supported.";
DisconnectSession(ErrorCode::LOGIN_SCREEN_NOT_SUPPORTED);
DisconnectSession(
ErrorCode::LOGIN_SCREEN_NOT_SUPPORTED,
"Connecting to the console login session is not yet supported.",
FROM_HERE);
return;
}
// Try to install the switch-in handler. Do this before switching out the
// current session so that the console session is not affected if it fails.
if (!InstallEventHandler()) {
LOG(ERROR) << "Failed to install the switch-in handler.";
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR,
"Failed to install the switch-in handler.", FROM_HERE);
return;
}
@ -198,14 +202,18 @@ void SessionWatcher::ActivateCurtain() {
std::optional<OSStatus> err = base::mac::SwitchToLoginWindow();
if (!err.has_value()) {
// Disconnect the session since we are unable to enter curtain mode.
LOG(ERROR) << "SACSwitchToLoginWindow unavailable - unable to enter "
"curtain mode.";
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
DisconnectSession(
ErrorCode::HOST_CONFIGURATION_ERROR,
"SACSwitchToLoginWindow unavailable - unable to enter curtain mode.",
FROM_HERE);
return;
}
if (err.value() != noErr) {
OSSTATUS_LOG(ERROR, err.value()) << "Failed to switch to login window";
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
DisconnectSession(
ErrorCode::HOST_CONFIGURATION_ERROR,
base::StringPrintf("Failed to switch to login window: %d",
err.value()),
FROM_HERE);
return;
}
if (is_headless) {
@ -213,10 +221,12 @@ void SessionWatcher::ActivateCurtain() {
// since the call to SACSwitchToLoginWindow very likely failed. If we
// allow them to unlock the machine, the local desktop would be visible if
// the local monitor were plugged in.
LOG(ERROR) << "Machine is running in headless mode (no monitors "
<< "attached), we attempted to curtain the session but "
<< "SACSwitchToLoginWindow is likely to fail in this mode.";
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
static const char error_details[] =
"Machine is running in headless mode (no monitors attached), we "
"attempted to curtain the session but SACSwitchToLoginWindow is "
"likely to fail in this mode.";
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR, error_details,
FROM_HERE);
return;
}
}
@ -234,7 +244,10 @@ bool SessionWatcher::InstallEventHandler() {
&event_handler_);
if (result != noErr) {
event_handler_ = nullptr;
DisconnectSession(ErrorCode::HOST_CONFIGURATION_ERROR);
DisconnectSession(
ErrorCode::HOST_CONFIGURATION_ERROR,
base::StringPrintf("Failed to install event handler: %d", result),
FROM_HERE);
return false;
}
@ -250,23 +263,27 @@ void SessionWatcher::RemoveEventHandler() {
}
}
void SessionWatcher::DisconnectSession(ErrorCode error) {
void SessionWatcher::DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
if (!caller_task_runner_->BelongsToCurrentThread()) {
caller_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&SessionWatcher::DisconnectSession, this, error));
FROM_HERE, base::BindOnce(&SessionWatcher::DisconnectSession, this,
error, error_details, error_location));
return;
}
if (client_session_control_) {
client_session_control_->DisconnectSession(error);
client_session_control_->DisconnectSession(error, error_details,
error_location);
}
}
OSStatus SessionWatcher::SessionActivateHandler(EventHandlerCallRef handler,
EventRef event,
void* user_data) {
static_cast<SessionWatcher*>(user_data)->DisconnectSession(ErrorCode::OK);
static_cast<SessionWatcher*>(user_data)->DisconnectSession(
ErrorCode::OK, "User session activated", FROM_HERE);
return noErr;
}

@ -26,6 +26,7 @@
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "remoting/base/auto_thread_task_runner.h"
#include "remoting/base/errors.h"
#include "remoting/host/action_executor.h"
#include "remoting/host/audio_capturer.h"
#include "remoting/host/base/desktop_environment_options.h"
@ -48,7 +49,6 @@
#include "remoting/proto/event.pb.h"
#include "remoting/proto/url_forwarder_control.pb.h"
#include "remoting/protocol/clipboard_stub.h"
#include "remoting/protocol/errors.h"
#include "remoting/protocol/input_event_tracker.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
#include "third_party/webrtc/modules/desktop_capture/mouse_cursor.h"
@ -165,8 +165,13 @@ const std::string& DesktopSessionAgent::client_jid() const {
return client_jid_;
}
void DesktopSessionAgent::DisconnectSession(protocol::ErrorCode error) {
void DesktopSessionAgent::DisconnectSession(
ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
if (desktop_session_state_handler_) {
// TODO: crbug.com/382334458 - serialize `error_details` and
// `error_location` over mojo.
desktop_session_state_handler_->DisconnectSession(error);
}
}

@ -19,6 +19,7 @@
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "remoting/base/errors.h"
#include "remoting/host/base/desktop_environment_options.h"
#include "remoting/host/client_session_control.h"
#include "remoting/host/desktop_display_info.h"
@ -31,7 +32,6 @@
#include "remoting/proto/event.pb.h"
#include "remoting/proto/url_forwarder_control.pb.h"
#include "remoting/protocol/clipboard_stub.h"
#include "remoting/protocol/errors.h"
#include "third_party/webrtc/modules/desktop_capture/mouse_cursor_monitor.h"
#include "ui/events/types/event_type.h"
@ -157,7 +157,9 @@ class DesktopSessionAgent
// ClientSessionControl interface.
const std::string& client_jid() const override;
void DisconnectSession(protocol::ErrorCode error) override;
void DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
void OnLocalKeyPressed(std::uint32_t usb_keycode) override;
void OnLocalPointerMoved(const webrtc::DesktopVector& position,
ui::EventType type) override;

@ -385,7 +385,10 @@ void DesktopSessionProxy::DisconnectSession(protocol::ErrorCode error) {
// Disconnect the client session if it hasn't been disconnected yet.
if (client_session_control_.get()) {
client_session_control_->DisconnectSession(error);
// TODO: crbug.com/382334458 - serialize `error_details` and
// `error_location` over mojo.
client_session_control_->DisconnectSession(
error, "Session disconnected by DesktopSessionAgent.", FROM_HERE);
}
}

@ -39,7 +39,9 @@ void DisconnectWindowAura::Start(
const base::WeakPtr<ClientSessionControl>& client_session_control) {
ash::Shell::Get()->system_tray_notifier()->NotifyRemotingScreenShareStart(
base::BindRepeating(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::OK));
client_session_control, ErrorCode::OK,
"Remote screen share stopped by the user.",
FROM_HERE));
}
} // namespace

@ -278,7 +278,8 @@ void DisconnectWindowGtk::OnClicked(GtkButton* button) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (client_session_control_.get()) {
client_session_control_->DisconnectSession(ErrorCode::OK);
client_session_control_->DisconnectSession(
ErrorCode::OK, "Disconnect button was clicked.", FROM_HERE);
}
}
@ -286,7 +287,8 @@ gboolean DisconnectWindowGtk::OnDelete(GtkWidget* window, GdkEvent* event) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (client_session_control_.get()) {
client_session_control_->DisconnectSession(ErrorCode::OK);
client_session_control_->DisconnectSession(
ErrorCode::OK, "Disconnect window deleted.", FROM_HERE);
}
return TRUE;
}

@ -75,9 +75,9 @@ void DisconnectWindowMac::Start(
DCHECK(window_controller_ == nil);
// Create the window.
base::OnceClosure disconnect_callback =
base::BindOnce(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::OK);
base::OnceClosure disconnect_callback = base::BindOnce(
&ClientSessionControl::DisconnectSession, client_session_control,
ErrorCode::OK, "Disconnect button was clicked.", FROM_HERE);
std::string client_jid = client_session_control->client_jid();
std::string username = client_jid.substr(0, client_jid.find('/'));

@ -349,7 +349,8 @@ void DisconnectWindowWin::EndDialog() {
auto_hide_timer_.Stop();
if (client_session_control_) {
client_session_control_->DisconnectSession(ErrorCode::OK);
client_session_control_->DisconnectSession(
ErrorCode::OK, "Disconnect window closed.", FROM_HERE);
}
}
@ -376,7 +377,8 @@ void DisconnectWindowWin::ShowDialog() {
// If the window still isn't visible, then disconnect the session.
if (!IsWindowVisible(hwnd_)) {
client_session_control_->DisconnectSession(ErrorCode::OK);
client_session_control_->DisconnectSession(
ErrorCode::OK, "Disconnect window is invisible.", FROM_HERE);
}
}
was_auto_hidden_ = false;

@ -43,7 +43,6 @@
#include "remoting/proto/event.pb.h"
#include "remoting/protocol/clipboard_stub.h"
#include "remoting/protocol/desktop_capturer.h"
#include "remoting/protocol/errors.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/webrtc/modules/desktop_capture/desktop_capture_types.h"
#include "third_party/webrtc/modules/desktop_capture/mouse_cursor_monitor.h"
@ -123,7 +122,12 @@ class MockClientSessionControl : public ClientSessionControl {
~MockClientSessionControl() override;
MOCK_METHOD(const std::string&, client_jid, (), (const, override));
MOCK_METHOD(void, DisconnectSession, (protocol::ErrorCode), (override));
MOCK_METHOD(void,
DisconnectSession,
(ErrorCode error,
std::string_view error_details,
const base::Location& error_location),
(override));
MOCK_METHOD(void,
OnLocalPointerMoved,
(const webrtc::DesktopVector&, ui::EventType),

@ -45,7 +45,9 @@ class HostWindowProxy::Core : public base::RefCountedThreadSafe<Core>,
// ClientSessionControl interface.
const std::string& client_jid() const override;
void DisconnectSession(protocol::ErrorCode error) override;
void DisconnectSession(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
void OnLocalKeyPressed(uint32_t usb_keycode) override;
void OnLocalPointerMoved(const webrtc::DesktopVector& position,
ui::EventType type) override;
@ -151,15 +153,20 @@ const std::string& HostWindowProxy::Core::client_jid() const {
return client_jid_;
}
void HostWindowProxy::Core::DisconnectSession(protocol::ErrorCode error) {
void HostWindowProxy::Core::DisconnectSession(
ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
if (!caller_task_runner_->BelongsToCurrentThread()) {
caller_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&Core::DisconnectSession, this, error));
FROM_HERE, base::BindOnce(&Core::DisconnectSession, this, error,
error_details, error_location));
return;
}
if (client_session_control_.get()) {
client_session_control_->DisconnectSession(error);
client_session_control_->DisconnectSession(error, error_details,
error_location);
}
}

@ -86,21 +86,24 @@ void LocalInputMonitorImpl::StartMonitoringForClientSession(
hotkey_input_monitor_ = LocalHotkeyInputMonitor::Create(
caller_task_runner_, input_task_runner_, ui_task_runner_,
base::BindOnce(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::OK));
client_session_control, ErrorCode::OK,
"Disconnection keyboard shortcut pressed", FROM_HERE));
pointer_input_monitor_ = LocalPointerInputMonitor::Create(
caller_task_runner_, input_task_runner_, ui_task_runner_,
base::BindRepeating(&ClientSessionControl::OnLocalPointerMoved,
client_session_control),
base::BindOnce(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::OK));
client_session_control, ErrorCode::OK,
"Local pointer input detected", FROM_HERE));
keyboard_input_monitor_ = LocalKeyboardInputMonitor::Create(
caller_task_runner_, input_task_runner_, ui_task_runner_,
base::BindRepeating(&ClientSessionControl::OnLocalKeyPressed,
client_session_control),
base::BindOnce(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::OK));
client_session_control, ErrorCode::OK,
"Local keyboard input detected.", FROM_HERE));
OnMonitoringStarted();
}

@ -70,7 +70,8 @@ TEST_F(LocalInputMonitorTest, BasicWithClientSession) {
EXPECT_CALL(client_session_control_, client_jid())
.Times(AnyNumber())
.WillRepeatedly(ReturnRef(client_jid_));
EXPECT_CALL(client_session_control_, DisconnectSession(_)).Times(AnyNumber());
EXPECT_CALL(client_session_control_, DisconnectSession(_, _, _))
.Times(AnyNumber());
EXPECT_CALL(client_session_control_, OnLocalPointerMoved(_, _))
.Times(AnyNumber());
EXPECT_CALL(client_session_control_, SetDisableInputs(_)).Times(0);

@ -359,7 +359,7 @@ void IpcDesktopEnvironmentTest::SetUp() {
EXPECT_CALL(client_session_control_, client_jid())
.Times(AnyNumber())
.WillRepeatedly(ReturnRef(client_jid_));
EXPECT_CALL(client_session_control_, DisconnectSession(_))
EXPECT_CALL(client_session_control_, DisconnectSession(_, _, _))
.Times(AnyNumber())
.WillRepeatedly(InvokeWithoutArgs(
this, &IpcDesktopEnvironmentTest::DeleteDesktopEnvironment));

@ -85,7 +85,8 @@ It2MeDesktopEnvironment::It2MeDesktopEnvironment(
ui_task_runner->PostDelayedTask(
FROM_HERE,
base::BindOnce(&ClientSessionControl::DisconnectSession,
client_session_control, ErrorCode::MAX_SESSION_LENGTH),
client_session_control, ErrorCode::MAX_SESSION_LENGTH,
"Maximum session duration has been reached.", FROM_HERE),
options.maximum_session_duration());
} else if (enable_user_interface) {
// Create the continue window. The implication of this window is that the

@ -179,7 +179,6 @@ static_library("protocol") {
"sdp_message.h",
"secure_channel_factory.cc",
"secure_channel_factory.h",
"session.cc",
"session.h",
"session_authz_authenticator.cc",
"session_authz_authenticator.h",

@ -88,7 +88,9 @@ class ConnectionToClient {
virtual Session* session() = 0;
// Disconnect the client connection.
virtual void Disconnect(ErrorCode error) = 0;
virtual void Disconnect(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) = 0;
// Start video stream that sends screen content from |desktop_capturer| to the
// client. |screen_id| should be webrtc::kFullDesktopScreenId for

@ -508,7 +508,7 @@ TEST_P(ConnectionTest, MAYBE_Disconnect) {
OnConnectionState(ConnectionToHost::CLOSED, ErrorCode::OK));
EXPECT_CALL(host_event_handler_, OnConnectionClosed(ErrorCode::OK));
client_session_->Close(ErrorCode::OK);
client_session_->Close(ErrorCode::OK, /* error_details= */ {}, FROM_HERE);
base::RunLoop().RunUntilIdle();
}

@ -93,7 +93,9 @@ ClientStub* FakeConnectionToClient::client_stub() {
return client_stub_;
}
void FakeConnectionToClient::Disconnect(ErrorCode disconnect_error) {
void FakeConnectionToClient::Disconnect(ErrorCode disconnect_error,
std::string_view error_details,
const base::Location& error_location) {
CHECK(is_connected_);
is_connected_ = false;

@ -75,7 +75,9 @@ class FakeConnectionToClient : public ConnectionToClient {
std::unique_ptr<AudioSource> audio_source) override;
ClientStub* client_stub() override;
void Disconnect(ErrorCode disconnect_error) override;
void Disconnect(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
Session* session() override;

@ -84,12 +84,14 @@ protocol::Session* IceConnectionToClient::session() {
return session_.get();
}
void IceConnectionToClient::Disconnect(ErrorCode error) {
void IceConnectionToClient::Disconnect(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
// This should trigger OnConnectionClosed() event and this object
// may be destroyed as the result.
session_->Close(error);
session_->Close(error, error_details, error_location);
}
std::unique_ptr<VideoStream> IceConnectionToClient::StartVideoStream(
@ -209,7 +211,7 @@ void IceConnectionToClient::OnIceTransportRouteChange(
void IceConnectionToClient::OnIceTransportError(ErrorCode error) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
Disconnect(error);
Disconnect(error, /* error_details= */ {}, FROM_HERE);
}
void IceConnectionToClient::OnChannelInitialized(
@ -222,7 +224,7 @@ void IceConnectionToClient::OnChannelInitialized(
void IceConnectionToClient::OnChannelClosed(
ChannelDispatcherBase* channel_dispatcher) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
Disconnect(ErrorCode::OK);
Disconnect(ErrorCode::OK, /* error_details= */ {}, FROM_HERE);
}
void IceConnectionToClient::NotifyIfChannelsReady() {

@ -48,7 +48,9 @@ class IceConnectionToClient : public ConnectionToClient,
void SetEventHandler(
ConnectionToClient::EventHandler* event_handler) override;
Session* session() override;
void Disconnect(ErrorCode error) override;
void Disconnect(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
std::unique_ptr<VideoStream> StartVideoStream(
webrtc::ScreenId screen_id,
std::unique_ptr<DesktopCapturer> desktop_capturer) override;

@ -54,7 +54,7 @@ void IceConnectionToHost::Connect(
}
void IceConnectionToHost::Disconnect(ErrorCode error) {
session_->Close(error);
session_->Close(error, /* error_details= */ {}, FROM_HERE);
}
void IceConnectionToHost::ApplyNetworkSettings(
@ -179,7 +179,7 @@ void IceConnectionToHost::OnIceTransportRouteChange(
}
void IceConnectionToHost::OnIceTransportError(ErrorCode error) {
session_->Close(error);
session_->Close(error, /* error_details= */ {}, FROM_HERE);
}
void IceConnectionToHost::OnChannelInitialized(
@ -189,7 +189,7 @@ void IceConnectionToHost::OnChannelInitialized(
void IceConnectionToHost::OnChannelClosed(
ChannelDispatcherBase* channel_dispatcher) {
session_->Close(ErrorCode::OK);
session_->Close(ErrorCode::OK, /* error_details= */ {}, FROM_HERE);
}
void IceConnectionToHost::OnVideoChannelStatus(bool active) {

@ -33,9 +33,6 @@ class Transport;
// JingleSessionManager.
class JingleSession : public Session {
public:
// The Close() override hides all the Close() overloads in the base class, so
// they need to be unhidden with a using statement.
using Session::Close;
JingleSession(const JingleSession&) = delete;
JingleSession& operator=(const JingleSession&) = delete;

@ -4,9 +4,11 @@
#include "remoting/protocol/jingle_session_manager.h"
#include <string>
#include <utility>
#include "base/functional/bind.h"
#include "base/location.h"
#include "remoting/protocol/authenticator.h"
#include "remoting/protocol/content_description.h"
#include "remoting/protocol/jingle_messages.h"
@ -112,8 +114,11 @@ bool JingleSessionManager::OnSignalStrategyIncomingStanza(
}
IncomingSessionResponse response = SessionManager::DECLINE;
std::string rejection_reason;
base::Location rejection_location;
if (!incoming_session_callback_.is_null()) {
incoming_session_callback_.Run(session, &response);
incoming_session_callback_.Run(session, &response, &rejection_reason,
&rejection_location);
}
if (response == SessionManager::ACCEPT) {
@ -133,7 +138,7 @@ bool JingleSessionManager::OnSignalStrategyIncomingStanza(
NOTREACHED();
}
session->Close(error);
session->Close(error, rejection_reason, rejection_location);
delete session;
DCHECK(sessions_.find(message->sid) == sessions_.end());
}

@ -10,6 +10,7 @@
#include "base/callback_list.h"
#include "base/functional/bind.h"
#include "base/location.h"
#include "base/memory/ptr_util.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
@ -64,8 +65,12 @@ const char kNormalizedHostJid[] = "host@gmail.com/123";
class MockSessionManagerListener {
public:
MOCK_METHOD2(OnIncomingSession,
void(Session*, SessionManager::IncomingSessionResponse*));
MOCK_METHOD(void,
OnIncomingSession,
(Session*,
SessionManager::IncomingSessionResponse*,
std::string*,
base::Location*));
};
class MockSessionEventHandler : public Session::EventHandler {
@ -229,7 +234,7 @@ class JingleSessionTest : public testing::Test {
}
void SetHostExpectation(bool expect_fail) {
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _))
.WillOnce(
DoAll(WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
SetArgPointee<1>(protocol::SessionManager::ACCEPT)));
@ -365,7 +370,7 @@ TEST_F(JingleSessionTest, RejectConnection) {
CreateSessionManagers(FakeAuthenticator::Config(FakeAuthenticator::ACCEPT));
// Reject incoming session.
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _))
.WillOnce(SetArgPointee<1>(protocol::SessionManager::DECLINE));
{
@ -463,7 +468,7 @@ TEST_F(JingleSessionTest, ConnectWithBadMultistepAuth) {
TEST_F(JingleSessionTest, TestIncompatibleProtocol) {
CreateSessionManagers(FakeAuthenticator::Config(FakeAuthenticator::ACCEPT));
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _)).Times(0);
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _)).Times(0);
EXPECT_CALL(client_session_event_handler_,
OnSessionStateChange(Session::FAILED))
@ -484,7 +489,7 @@ TEST_F(JingleSessionTest, TestIncompatibleProtocol) {
TEST_F(JingleSessionTest, TestLegacyIceConnection) {
CreateSessionManagers(FakeAuthenticator::Config(FakeAuthenticator::ACCEPT));
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _)).Times(0);
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _)).Times(0);
EXPECT_CALL(client_session_event_handler_,
OnSessionStateChange(Session::FAILED))
@ -506,7 +511,7 @@ TEST_F(JingleSessionTest, DeleteSessionOnIncomingConnection) {
FakeAuthenticator::ACCEPT, true);
CreateSessionManagers(auth_config);
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _))
.WillOnce(
DoAll(WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
SetArgPointee<1>(protocol::SessionManager::ACCEPT)));
@ -533,7 +538,7 @@ TEST_F(JingleSessionTest, DeleteSessionOnAuth) {
FakeAuthenticator::ACCEPT, true);
CreateSessionManagers(auth_config, kMessagesTillStarted);
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _))
EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _, _, _))
.WillOnce(
DoAll(WithArg<0>(Invoke(this, &JingleSessionTest::SetHostSession)),
SetArgPointee<1>(protocol::SessionManager::ACCEPT)));
@ -637,7 +642,8 @@ TEST_F(JingleSessionTest, ImmediatelyCloseSessionAfterConnect) {
FakeAuthenticator::CLIENT, auth_config,
client_signal_strategy_->GetLocalAddress().id(), kNormalizedHostJid));
client_session_->Close(ErrorCode::HOST_OVERLOAD);
client_session_->Close(ErrorCode::HOST_OVERLOAD, /* error_details= */ {},
FROM_HERE);
base::RunLoop().RunUntilIdle();
// We should only send a SESSION_TERMINATE message if the session has been
// closed before SESSION_INITIATE message.

@ -1,15 +0,0 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "remoting/protocol/session.h"
#include "base/location.h"
namespace remoting::protocol {
void Session::Close(ErrorCode error) {
Close(error, /* error_details= */ {}, /* error_location= */ {});
}
} // namespace remoting::protocol

@ -89,12 +89,6 @@ class Session {
// session becomes AUTHENTICATED. The transport must outlive the session.
virtual void SetTransport(Transport* transport) = 0;
// Closes connection. EventHandler is guaranteed not to be called after this
// method returns.
// |error| specifies the error code in case when the session is being closed
// due to an error.
void Close(ErrorCode error);
// Closes connection. EventHandler is guaranteed not to be called after this
// method returns.
// |error| specifies the error code in case when the session is being closed

@ -56,6 +56,7 @@
#include <string>
#include "base/functional/callback.h"
#include "base/location.h"
#include "remoting/protocol/session.h"
#include "remoting/protocol/session_observer.h"
@ -93,7 +94,9 @@ class SessionManager {
// Session::set_config(). The callback must take ownership of the |session| if
// it ACCEPTs it.
typedef base::RepeatingCallback<void(Session* session,
IncomingSessionResponse* response)>
IncomingSessionResponse* response,
std::string* rejection_reason,
base::Location* rejection_location)>
IncomingSessionCallback;
SessionManager() {}

@ -9,6 +9,7 @@
#include "base/functional/bind.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "base/task/single_thread_task_runner.h"
#include "components/webrtc/thread_wrapper.h"
#include "net/base/io_buffer.h"
@ -79,13 +80,15 @@ protocol::Session* WebrtcConnectionToClient::session() {
return session_.get();
}
void WebrtcConnectionToClient::Disconnect(ErrorCode error) {
void WebrtcConnectionToClient::Disconnect(
ErrorCode error,
std::string_view error_details,
const base::Location& error_location) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
// This should trigger OnConnectionClosed() event and this object
// may be destroyed as the result.
// TODO: crbug.com/382334458 - Pass WebRTC error details to Close().
session_->Close(error);
session_->Close(error, error_details, error_location);
}
std::unique_ptr<VideoStream> WebrtcConnectionToClient::StartVideoStream(
@ -226,7 +229,8 @@ void WebrtcConnectionToClient::OnWebrtcTransportConnected() {
void WebrtcConnectionToClient::OnWebrtcTransportError(ErrorCode error) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
Disconnect(error);
// TODO: crbug.com/382334458 - Pass WebRTC error details to Close().
Disconnect(error, /* error_details= */ {}, FROM_HERE);
}
void WebrtcConnectionToClient::OnWebrtcTransportProtocolChanged() {
@ -301,9 +305,10 @@ void WebrtcConnectionToClient::OnChannelClosed(
// or the client page is closed normally. If the client goes offline then the
// channel will remain open. Hence it should be safe to report ErrorCode::OK
// here.
HOST_LOG << "Channel " << channel_dispatcher->channel_name()
<< " was closed.";
Disconnect(ErrorCode::OK);
std::string details = base::StringPrintf(
"Channel %s was closed.", channel_dispatcher->channel_name().c_str());
HOST_LOG << details;
Disconnect(ErrorCode::OK, details, FROM_HERE);
}
bool WebrtcConnectionToClient::allChannelsConnected() {

@ -45,7 +45,9 @@ class WebrtcConnectionToClient : public ConnectionToClient,
void SetEventHandler(
ConnectionToClient::EventHandler* event_handler) override;
Session* session() override;
void Disconnect(ErrorCode error) override;
void Disconnect(ErrorCode error,
std::string_view error_details,
const base::Location& error_location) override;
std::unique_ptr<VideoStream> StartVideoStream(
webrtc::ScreenId screen_id,
std::unique_ptr<DesktopCapturer> desktop_capturer) override;

@ -58,7 +58,7 @@ void WebrtcConnectionToHost::Connect(
}
void WebrtcConnectionToHost::Disconnect(ErrorCode error) {
session_->Close(error);
session_->Close(error, /* error_details= */ {}, FROM_HERE);
}
void WebrtcConnectionToHost::ApplyNetworkSettings(