Adding a DataChannelHandler for Action messages

This change adds a DataChannelHandler for Action messages and ties it
into ClientSession.  When a new handler is created, it will receive an
ActionExecutor (added in a previous CL) which is used to invoke the
requested action.

BUG: 892434
Change-Id: I56b60fdc56b89900297943eafe78f50982b7681f
Reviewed-on: https://chromium-review.googlesource.com/c/1294715Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608941}

remoting/host/BUILD.gn

@@ -56,6 +56,8 @@ static_library("host") {
   sources = [
     "action_executor.cc",
     "action_executor.h",
+    "action_message_handler.cc",
+    "action_message_handler.h",
     "audio_capturer.cc",
     "audio_capturer.h",
     "audio_capturer_chromeos.cc",

remoting/host/action_message_handler.cc

+// Copyright 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 "remoting/host/action_message_handler.h"
+
+#include <utility>
+
+#include "base/bind_helpers.h"
+#include "remoting/base/compound_buffer.h"
+#include "remoting/host/action_executor.h"
+#include "remoting/proto/action.pb.h"
+#include "remoting/protocol/message_serialization.h"
+
+namespace remoting {
+
+using protocol::ActionResponse;
+
+ActionMessageHandler::ActionMessageHandler(
+    const std::string& name,
+    const std::vector<protocol::ActionRequest::Action>& actions,
+    std::unique_ptr<protocol::MessagePipe> pipe,
+    std::unique_ptr<ActionExecutor> action_executor)
+    : protocol::NamedMessagePipeHandler(name, std::move(pipe)),
+      action_executor_(std::move(action_executor)),
+      supported_actions_(actions) {
+  DCHECK(action_executor_);
+}
+
+ActionMessageHandler::~ActionMessageHandler() = default;
+
+void ActionMessageHandler::OnIncomingMessage(
+    std::unique_ptr<CompoundBuffer> message) {
+  DCHECK(message);
+  std::unique_ptr<protocol::ActionRequest> request =
+      protocol::ParseMessage<protocol::ActionRequest>(message.get());
+
+  ActionResponse response;
+  response.set_request_id(request ? request->request_id() : 0);
+
+  if (!request) {
+    response.set_code(ActionResponse::PROTOCOL_ERROR);
+    response.set_protocol_error_type(ActionResponse::INVALID_MESSAGE_ERROR);
+  } else if (!request->has_action()) {
+    // |has_action()| will return false if either the field is not set or the
+    // value is out of range.  Unfortunately we can't distinguish between these
+    // two conditions so we return the same error for both.
+    response.set_code(ActionResponse::PROTOCOL_ERROR);
+    response.set_protocol_error_type(ActionResponse::INVALID_ACTION_ERROR);
+  } else if (supported_actions_.count(request->action()) == 0) {
+    // We received an action which is valid, but not supported by this platform
+    // or connection mode.
+    response.set_code(ActionResponse::PROTOCOL_ERROR);
+    response.set_protocol_error_type(ActionResponse::UNSUPPORTED_ACTION_ERROR);
+  } else {
+    // Valid action request received.  None of the supported actions at this
+    // time support return codes, if we add actions in the future which could
+    // fail in an observable way, we should consider returning that info to the
+    // client.
+    action_executor_->ExecuteAction(*request);
+    response.set_code(ActionResponse::ACTION_SUCCESS);
+  }
+
+  Send(response, base::DoNothing());
+}
+
+}  // namespace remoting

remoting/host/action_message_handler.h

+// Copyright 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 REMOTING_HOST_ACTION_MESSAGE_HANDLER_H_
+#define REMOTING_HOST_ACTION_MESSAGE_HANDLER_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/containers/flat_set.h"
+#include "base/macros.h"
+#include "remoting/proto/action.pb.h"
+#include "remoting/protocol/named_message_pipe_handler.h"
+
+namespace remoting {
+
+class ActionExecutor;
+
+constexpr char kActionDataChannelPrefix[] = "actions";
+
+class ActionMessageHandler : public protocol::NamedMessagePipeHandler {
+ public:
+  ActionMessageHandler(
+      const std::string& name,
+      const std::vector<protocol::ActionRequest::Action>& actions,
+      std::unique_ptr<protocol::MessagePipe> pipe,
+      std::unique_ptr<ActionExecutor> action_executor);
+  ~ActionMessageHandler() override;
+
+  // protocol::NamedMessagePipeHandler implementation.
+  void OnIncomingMessage(std::unique_ptr<CompoundBuffer> message) override;
+
+ private:
+  std::unique_ptr<ActionExecutor> action_executor_;
+
+  // Populated via the negotiated capabilities between host and client.
+  base::flat_set<protocol::ActionRequest::Action> supported_actions_;
+
+  DISALLOW_COPY_AND_ASSIGN(ActionMessageHandler);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_ACTION_MESSAGE_HANDLER_H_

remoting/host/client_session.cc

@@ -14,8 +14,10 @@
 #include "build/build_config.h"
 #include "remoting/base/capabilities.h"
 #include "remoting/base/constants.h"
-#include "remoting/base/session_options.h"
 #include "remoting/base/logging.h"
+#include "remoting/base/session_options.h"
+#include "remoting/host/action_executor.h"
+#include "remoting/host/action_message_handler.h"
 #include "remoting/host/audio_capturer.h"
 #include "remoting/host/desktop_environment.h"
 #include "remoting/host/file_transfer_message_handler.h"
@@ -38,6 +40,8 @@
 
 namespace remoting {
 
+using protocol::ActionRequest;
+
 namespace {
 
 // Name of command-line flag to disable use of I444 by default.
@@ -190,6 +194,21 @@ void ClientSession::SetCapabilities(
                    base::Unretained(this)));
   }
 
+  std::vector<ActionRequest::Action> supported_actions;
+  if (HasCapability(capabilities_, protocol::kSendAttentionSequenceAction))
+    supported_actions.push_back(ActionRequest::SEND_ATTENTION_SEQUENCE);
+  if (HasCapability(capabilities_, protocol::kLockWorkstationAction))
+    supported_actions.push_back(ActionRequest::LOCK_WORKSTATION);
+
+  if (supported_actions.size() > 0) {
+    // Register the action message handler.
+    data_channel_manager_.RegisterCreateHandlerCallback(
+        kActionDataChannelPrefix,
+        base::BindRepeating(&ClientSession::CreateActionMessageHandler,
+                            base::Unretained(this),
+                            std::move(supported_actions)));
+  }
+
   VLOG(1) << "Client capabilities: " << *client_capabilities_;
 
   desktop_environment_->SetCapabilities(capabilities_);
@@ -499,4 +518,19 @@ void ClientSession::CreateFileTransferMessageHandler(
       desktop_environment_->CreateFileProxyWrapper());
 }
 
+void ClientSession::CreateActionMessageHandler(
+    std::vector<ActionRequest::Action> capabilities,
+    const std::string& channel_name,
+    std::unique_ptr<protocol::MessagePipe> pipe) {
+  std::unique_ptr<ActionExecutor> action_executor =
+      desktop_environment_->CreateActionExecutor();
+  if (!action_executor)
+    return;
+
+  // ActionMessageHandler manages its own lifetime and is tied to the lifetime
+  // of |pipe|. Once |pipe| is closed, this instance will be cleaned up.
+  new ActionMessageHandler(channel_name, capabilities, std::move(pipe),
+                           std::move(action_executor));
+}
+
 }  // namespace remoting

remoting/host/client_session.h

@@ -22,6 +22,7 @@
 #include "remoting/host/host_experiment_session_plugin.h"
 #include "remoting/host/host_extension_session_manager.h"
 #include "remoting/host/remote_input_filter.h"
+#include "remoting/proto/action.pb.h"
 #include "remoting/protocol/clipboard_echo_filter.h"
 #include "remoting/protocol/clipboard_filter.h"
 #include "remoting/protocol/clipboard_stub.h"
@@ -161,6 +162,11 @@ class ClientSession : public protocol::HostStub,
                           const webrtc::DesktopSize& size,
                           const webrtc::DesktopVector& dpi) override;
 
+  void CreateActionMessageHandler(
+      std::vector<protocol::ActionRequest::Action> capabilities,
+      const std::string& channel_name,
+      std::unique_ptr<protocol::MessagePipe> pipe);
+
   void CreateFileTransferMessageHandler(
       const std::string& channel_name,
       std::unique_ptr<protocol::MessagePipe> pipe);

remoting/host/me2me_desktop_environment.cc

@@ -30,6 +30,10 @@
 #include <unistd.h>
 #endif  // defined(OS_POSIX)
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif  // defined(OS_WIN)
+
 namespace remoting {
 
 Me2MeDesktopEnvironment::~Me2MeDesktopEnvironment() {
@@ -69,6 +73,17 @@ std::string Me2MeDesktopEnvironment::GetCapabilities() const {
     capabilities += protocol::kFileTransferCapability;
   }
 
+#if defined(OS_WIN)
+  capabilities += " ";
+  capabilities += protocol::kSendAttentionSequenceAction;
+
+  if (base::win::OSInfo::GetInstance()->version_type() !=
+      base::win::VersionType::SUITE_HOME) {
+    capabilities += " ";
+    capabilities += protocol::kLockWorkstationAction;
+  }
+#endif  // defined(OS_WIN)
+
   return capabilities;
 }
 

remoting/host/win/session_action_executor.cc

@@ -29,11 +29,11 @@ void SessionActionExecutor::ExecuteAction(const ActionRequest& request) {
   DCHECK(request.has_action());
 
   switch (request.action()) {
-    case protocol::ActionRequest::SEND_ATTENTION_SEQUENCE:
+    case ActionRequest::SEND_ATTENTION_SEQUENCE:
       execute_action_task_runner_->PostTask(FROM_HERE, inject_sas_);
       break;
 
-    case protocol::ActionRequest::LOCK_WORKSTATION:
+    case ActionRequest::LOCK_WORKSTATION:
       execute_action_task_runner_->PostTask(FROM_HERE, lock_workstation_);
       break;
 

remoting/proto/action.proto

@@ -7,9 +7,11 @@ package remoting.protocol;
 // Next Id: 3
 message ActionRequest {
   enum Action {
+    ACTION_UNDEFINED = 0;
     SEND_ATTENTION_SEQUENCE = 1;
     LOCK_WORKSTATION = 2;
   }
+  // Represents the Action to be executed remotely.
   optional Action action = 1;
 
   // Identifies an individual request so a response can be sent at a later time
@@ -23,18 +25,24 @@ message ActionResponse {
   optional uint32 request_id = 1;
 
   enum ReturnCode {
-    ACTION_SUCCESS = 1;
-    ACTION_ERROR = 2;
+    RETURN_CODE_UNDEFINED = 0;
+    PROTOCOL_ERROR = 1;
+    ACTION_SUCCESS = 2;
   }
   optional ReturnCode code = 2;
 
-  // ErrorCode field is populated if |code()| indicates an error occurred.
-  enum ErrorCode {
-    // The action supplied is not known.
-    UNKNOWN_ACTION_ERROR = 1;
+  // ProtocolErrorType field may be populated if |code()| is a PROTOCOL_ERROR.
+  enum ProtocolErrorType {
+    ERROR_CODE_UNDEFINED = 0;
+
+    // The ActionRequest received could not be deserialized.
+    INVALID_MESSAGE_ERROR = 1;
+
+    // The action supplied was missing or undefined.
+    INVALID_ACTION_ERROR = 2;
 
     // The action supplied is not supported by the platform or connection mode.
-    UNSUPPORTED_ACTION_ERROR = 2;
+    UNSUPPORTED_ACTION_ERROR = 3;
   }
-  optional ErrorCode error = 3;
+  optional ProtocolErrorType protocol_error_type = 3;
 }

remoting/protocol/capability_names.h

@@ -9,12 +9,18 @@ namespace remoting {
 namespace protocol {
 
 // Used for negotiating client-host capabilities for touch events.
-const char kTouchEventsCapability[] = "touchEvents";
+constexpr char kTouchEventsCapability[] = "touchEvents";
 
-const char kSendInitialResolution[] = "sendInitialResolution";
-const char kRateLimitResizeRequests[] = "rateLimitResizeRequests";
+constexpr char kSendInitialResolution[] = "sendInitialResolution";
+constexpr char kRateLimitResizeRequests[] = "rateLimitResizeRequests";
 
-const char kFileTransferCapability[] = "fileTransfer";
+constexpr char kFileTransferCapability[] = "fileTransfer";
+
+// TODO(joedow): Ideally these would be dynamically created via the
+// DataChannelManager, we should consider moving them there if we begin using
+// WebRTC data channels for individual features more frequently.
+constexpr char kLockWorkstationAction[] = "lockWorkstationAction";
+constexpr char kSendAttentionSequenceAction[] = "sendAttentionSequenceAction";
 
 }  // namespace protocol
 }  // namespace remoting