Skip to content

Commit

Permalink
Rename CrdHostDelegate to CrdAdminSessionController
Browse files Browse the repository at this point in the history
This reflects the fact that this class does not only serve as the
delegate to the CRD host but also:
  * controls the life of the active session,
  * will in a follow-up CL gain the responsibility of allowing the
    CRD session to survive a Chrome reboot.

Fixed: b/284284610
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Change-Id: I3ee01ad4d56ed53bac1a288b56180a3abe712b8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4565640
Reviewed-by: Miriam Polzer <mpolzer@google.com>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Ben Franz <bfranz@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/main@{#1150448}
  • Loading branch information
Jeroen Dhollander authored and Chromium LUCI CQ committed May 30, 2023
1 parent 7f77f41 commit 9f2c182
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 145 deletions.
6 changes: 3 additions & 3 deletions chrome/browser/ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2442,8 +2442,8 @@ source_set("ash") {
"policy/off_hours/off_hours_proto_parser.h",
"policy/remote_commands/affiliated_remote_commands_invalidator.cc",
"policy/remote_commands/affiliated_remote_commands_invalidator.h",
"policy/remote_commands/crd_host_delegate.cc",
"policy/remote_commands/crd_host_delegate.h",
"policy/remote_commands/crd_admin_session_controller.cc",
"policy/remote_commands/crd_admin_session_controller.h",
"policy/remote_commands/crd_logging.h",
"policy/remote_commands/crd_remote_command_utils.cc",
"policy/remote_commands/crd_remote_command_utils.h",
Expand Down Expand Up @@ -5480,7 +5480,7 @@ source_set("unit_tests") {
"policy/off_hours/device_off_hours_controller_unittest.cc",
"policy/off_hours/off_hours_policy_applier_unittest.cc",
"policy/off_hours/off_hours_proto_parser_unittest.cc",
"policy/remote_commands/crd_host_delegate_unittest.cc",
"policy/remote_commands/crd_admin_session_controller_unittest.cc",
"policy/remote_commands/device_command_fetch_crd_availability_info_job_unittest.cc",
"policy/remote_commands/device_command_fetch_support_packet_job_unittest.cc",
"policy/remote_commands/device_command_get_available_routines_job_unittest.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ash/policy/remote_commands/crd_host_delegate.h"
#include "chrome/browser/ash/policy/remote_commands/crd_admin_session_controller.h"

#include <iomanip>
#include <ostream>
Expand Down Expand Up @@ -31,14 +31,15 @@ namespace {

// Default implementation of the `RemotingService`, which will contact the real
// remoting service.
class DefaultRemotingService : public CrdHostDelegate::RemotingServiceProxy {
class DefaultRemotingService
: public CrdAdminSessionController::RemotingServiceProxy {
public:
DefaultRemotingService() = default;
DefaultRemotingService(const DefaultRemotingService&) = delete;
DefaultRemotingService& operator=(const DefaultRemotingService&) = delete;
~DefaultRemotingService() override = default;

// `CrdHostDelegate::RemotingService` implementation:
// `CrdAdminSessionController::RemotingService` implementation:
void StartSession(remoting::mojom::SupportSessionParamsPtr params,
const remoting::ChromeOsEnterpriseParams& enterprise_params,
StartSessionCallback callback) override {
Expand Down Expand Up @@ -68,7 +69,7 @@ std::ostream& operator<<(

} // namespace

class CrdHostDelegate::CrdHostSession
class CrdAdminSessionController::CrdHostSession
: public remoting::mojom::SupportHostObserver {
public:
CrdHostSession(const SessionParameters& parameters,
Expand All @@ -83,7 +84,8 @@ class CrdHostDelegate::CrdHostSession
CrdHostSession& operator=(const CrdHostSession&) = delete;
~CrdHostSession() override = default;

void Start(CrdHostDelegate::RemotingServiceProxy& remoting_service) {
void Start(
CrdAdminSessionController::RemotingServiceProxy& remoting_service) {
CRD_DVLOG(3) << "Starting CRD session with parameters " << parameters_;

remoting_service.StartSession(
Expand Down Expand Up @@ -215,26 +217,26 @@ class CrdHostDelegate::CrdHostSession
base::WeakPtrFactory<CrdHostSession> weak_factory_{this};
};

CrdHostDelegate::CrdHostDelegate()
: CrdHostDelegate(std::make_unique<DefaultRemotingService>()) {}
CrdAdminSessionController::CrdAdminSessionController()
: CrdAdminSessionController(std::make_unique<DefaultRemotingService>()) {}

CrdHostDelegate::CrdHostDelegate(
CrdAdminSessionController::CrdAdminSessionController(
std::unique_ptr<RemotingServiceProxy> remoting_service)
: remoting_service_(std::move(remoting_service)) {}

CrdHostDelegate::~CrdHostDelegate() = default;
CrdAdminSessionController::~CrdAdminSessionController() = default;

bool CrdHostDelegate::HasActiveSession() const {
bool CrdAdminSessionController::HasActiveSession() const {
return active_session_ != nullptr;
}

void CrdHostDelegate::TerminateSession(base::OnceClosure callback) {
void CrdAdminSessionController::TerminateSession(base::OnceClosure callback) {
CRD_DVLOG(3) << "Terminating CRD session";
active_session_ = nullptr;
std::move(callback).Run();
}

void CrdHostDelegate::StartCrdHostAndGetCode(
void CrdAdminSessionController::StartCrdHostAndGetCode(
const SessionParameters& parameters,
AccessCodeCallback success_callback,
ErrorCallback error_callback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_HOST_DELEGATE_H_
#define CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_HOST_DELEGATE_H_
#ifndef CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_ADMIN_SESSION_CONTROLLER_H_
#define CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_ADMIN_SESSION_CONTROLLER_H_

#include <memory>

Expand All @@ -14,10 +14,12 @@

namespace policy {

// Delegate that will start a session with the CRD native host.
// Controller that owns the admin initiated CRD session (if any).
//
// Will keep the session alive and active as long as this class lives.
// Deleting this class object will forcefully interrupt the active CRD session.
class CrdHostDelegate : public DeviceCommandStartCrdSessionJob::Delegate {
class CrdAdminSessionController
: public DeviceCommandStartCrdSessionJob::Delegate {
public:
// Proxy class to establish a connection with the Remoting service.
// Overwritten in unittests to inject a test service.
Expand All @@ -36,12 +38,13 @@ class CrdHostDelegate : public DeviceCommandStartCrdSessionJob::Delegate {
StartSessionCallback callback) = 0;
};

CrdHostDelegate();
explicit CrdHostDelegate(
CrdAdminSessionController();
explicit CrdAdminSessionController(
std::unique_ptr<RemotingServiceProxy> remoting_service);
CrdHostDelegate(const CrdHostDelegate&) = delete;
CrdHostDelegate& operator=(const CrdHostDelegate&) = delete;
~CrdHostDelegate() override;
CrdAdminSessionController(const CrdAdminSessionController&) = delete;
CrdAdminSessionController& operator=(const CrdAdminSessionController&) =
delete;
~CrdAdminSessionController() override;

// DeviceCommandStartCrdSessionJob::Delegate implementation:
bool HasActiveSession() const override;
Expand All @@ -61,4 +64,4 @@ class CrdHostDelegate : public DeviceCommandStartCrdSessionJob::Delegate {
};
} // namespace policy

#endif // CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_HOST_DELEGATE_H_
#endif // CHROME_BROWSER_ASH_POLICY_REMOTE_COMMANDS_CRD_ADMIN_SESSION_CONTROLLER_H_

0 comments on commit 9f2c182

Please sign in to comment.