Skip to content

Commit

Permalink
[remoting][linux host][open url] Update host for the new UX
Browse files Browse the repository at this point in the history
This CL changes some host code to make the new open URL UX work.
Basically:

1. Introduce a new url-forwarder-config data channel for querying and
   configuring the URL forwarder, and the corresponding
   UrlForwarderConfigMessageHandler for handling it.
2. Replace ScopedUrlForwarder with a UrlForwarderConfigurator singleton.
3. Remove default browser restoring logic from the daemon script and the
   RemoteOpenUrl, since we don't do auto-restoring any more.
4. Rename setup_url_forwarder.py to configure_url_forwarder.py so that
   we don't need to strangely call "setup-url-forwarder --setup".

Bug: b:183135000
Change-Id: I26c77bbdf67d95608532e250656e176796df77eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2946788
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#892728}
  • Loading branch information
ywh233 authored and Chromium LUCI CQ committed Jun 15, 2021
1 parent a595dfc commit 9e5d621
Show file tree
Hide file tree
Showing 21 changed files with 382 additions and 190 deletions.
10 changes: 6 additions & 4 deletions remoting/host/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,6 @@ static_library("common") {
"resizing_host_observer.h",
"resources.h",
"sas_injector.h",
"scoped_url_forwarder.cc",
"scoped_url_forwarder.h",
"screen_controls.h",
"screen_resolution.cc",
"screen_resolution.h",
Expand All @@ -343,6 +341,10 @@ static_library("common") {
"token_validator_base.h",
"token_validator_factory_impl.cc",
"token_validator_factory_impl.h",
"url_forwarder_configurator.cc",
"url_forwarder_configurator.h",
"url_forwarder_control_message_handler.cc",
"url_forwarder_control_message_handler.h",
"usage_stats_consent.h",
"xmpp_register_support_host_request.cc",
"xmpp_register_support_host_request.h",
Expand Down Expand Up @@ -476,8 +478,8 @@ static_library("common") {
"curtain_mode_linux.cc",
"disconnect_window_linux.cc",
"keyboard_layout_monitor_linux.cc",
"scoped_url_forwarder_linux.cc",
"scoped_url_forwarder_linux.h",
"url_forwarder_configurator_linux.cc",
"url_forwarder_configurator_linux.h",
"xsession_chooser_linux.cc",
]
}
Expand Down
15 changes: 15 additions & 0 deletions remoting/host/client_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "remoting/host/remote_open_url_message_handler.h"
#include "remoting/host/screen_controls.h"
#include "remoting/host/screen_resolution.h"
#include "remoting/host/url_forwarder_control_message_handler.h"
#include "remoting/proto/control.pb.h"
#include "remoting/proto/event.pb.h"
#include "remoting/protocol/audio_stream.h"
Expand Down Expand Up @@ -206,6 +207,11 @@ void ClientSession::SetCapabilities(
kRemoteOpenUrlDataChannelName,
base::BindRepeating(&ClientSession::CreateRemoteOpenUrlMessageHandler,
base::Unretained(this)));
data_channel_manager_.RegisterCreateHandlerCallback(
UrlForwarderControlMessageHandler::kDataChannelName,
base::BindRepeating(
&ClientSession::CreateUrlForwarderControlMessageHandler,
base::Unretained(this)));
}

std::vector<ActionRequest::Action> supported_actions;
Expand Down Expand Up @@ -897,4 +903,13 @@ void ClientSession::CreateRemoteOpenUrlMessageHandler(
new RemoteOpenUrlMessageHandler(channel_name, std::move(pipe));
}

void ClientSession::CreateUrlForwarderControlMessageHandler(
const std::string& channel_name,
std::unique_ptr<protocol::MessagePipe> pipe) {
// UrlForwarderControlMessageHandler manages its own lifetime and is tied to
// the lifetime of |pipe|. Once |pipe| is closed, this instance will be
// cleaned up.
new UrlForwarderControlMessageHandler(channel_name, std::move(pipe));
}

} // namespace remoting
4 changes: 4 additions & 0 deletions remoting/host/client_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ class ClientSession : public protocol::HostStub,
const std::string& channel_name,
std::unique_ptr<protocol::MessagePipe> pipe);

void CreateUrlForwarderControlMessageHandler(
const std::string& channel_name,
std::unique_ptr<protocol::MessagePipe> pipe);

EventHandler* event_handler_;

// Used to create a DesktopEnvironment instance for this session.
Expand Down
4 changes: 2 additions & 2 deletions remoting/host/installer/linux/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ install:
"$(INSTALL_DIR)"
install "$(SRC_DIR)/remoting/host/installer/linux/Xsession" \
"$(INSTALL_DIR)"
install "$(SRC_DIR)/remoting/host/linux/setup_url_forwarder.py" \
"$(INSTALL_DIR)/setup-url-forwarder"
install "$(SRC_DIR)/remoting/host/linux/configure_url_forwarder.py" \
"$(INSTALL_DIR)/configure-url-forwarder"

install -m 0644 \
"$(BUILD_DIR)/remoting/com.google.chrome.remote_desktop.json" \
Expand Down
2 changes: 1 addition & 1 deletion remoting/host/linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ if (enable_me2me_host) {
}

copy("remoting_me2me_host_copy_setup_url_forwarder_script") {
sources = [ "setup_url_forwarder.py" ]
sources = [ "configure_url_forwarder.py" ]
outputs = [ "$root_build_dir/remoting/setup-url-forwarder" ]
}

Expand Down
File renamed without changes.
23 changes: 0 additions & 23 deletions remoting/host/linux/linux_me2me_host.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@

USER_SESSION_PATH = os.path.join(SCRIPT_DIR, "user-session")

SETUP_URL_FORWARDER_PATH = os.path.join(SCRIPT_DIR, "setup-url-forwarder")

CHROME_REMOTING_GROUP_NAME = "chrome-remote-desktop"

HOME_DIR = os.environ["HOME"]
Expand Down Expand Up @@ -827,21 +825,6 @@ def sigusr1_handler(signum, frame):
finally:
self.host_proc.stdin.close()

def restore_default_browser(self):
# Restores the previous default browser settings in case the host crashes
# during a remote session. It's noop if the current default browser is not
# the CRD URL forwarder.

if not os.path.exists(SETUP_URL_FORWARDER_PATH):
print('Cannot find the URL forwarder setup script', file=sys.stderr)
return
print('Attempting to restore previous default browser...')
retcode = subprocess.call([SETUP_URL_FORWARDER_PATH, "--restore"],
env=self.child_env)
if retcode != 0:
print('URL forwarder setup script returned a non-zero code:', retcode,
file=sys.stderr)

def shutdown_all_procs(self):
"""Send SIGTERM to all procs and wait for them to exit. Will fallback to
SIGKILL if a process doesn't exit within 10 seconds.
Expand Down Expand Up @@ -1301,7 +1284,6 @@ def cleanup():

global g_desktop
if g_desktop is not None:
g_desktop.restore_default_browser()
g_desktop.shutdown_all_procs()
if g_desktop.xorg_conf is not None:
os.remove(g_desktop.xorg_conf)
Expand Down Expand Up @@ -1852,11 +1834,6 @@ def main():
if desktop.host_proc is None:
logging.info("Launching host process")

# Restore the previous default browser in case the daemon script has
# crashed or the system has been rebooted in the middle of a remote
# session.
desktop.restore_default_browser()

extra_start_host_args = []
if HOST_EXTRA_PARAMS_ENV_VAR in os.environ:
extra_start_host_args = \
Expand Down
3 changes: 0 additions & 3 deletions remoting/host/remote_open_url_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/callback_helpers.h"
#include "base/logging.h"
#include "remoting/base/compound_buffer.h"
#include "remoting/host/scoped_url_forwarder.h"
#include "remoting/protocol/message_serialization.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -51,7 +50,6 @@ RemoteOpenUrlMessageHandler::~RemoteOpenUrlMessageHandler() {
void RemoteOpenUrlMessageHandler::OnConnected() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

scoped_url_forwarder_ = ScopedUrlForwarder::Create();
ipc_server_.StartServer();
}

Expand Down Expand Up @@ -80,7 +78,6 @@ void RemoteOpenUrlMessageHandler::OnIncomingMessage(
void RemoteOpenUrlMessageHandler::OnDisconnecting() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

scoped_url_forwarder_.reset();
ipc_server_.StopServer();

// The remote connection is going away, so inform all IPC clients to open the
Expand Down
4 changes: 0 additions & 4 deletions remoting/host/remote_open_url_message_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ class GURL;

namespace remoting {

class ScopedUrlForwarder;

class RemoteOpenUrlMessageHandler final
: public mojom::RemoteUrlOpener,
public protocol::NamedMessagePipeHandler {
Expand Down Expand Up @@ -49,8 +47,6 @@ class RemoteOpenUrlMessageHandler final
MojoIpcServer<mojom::RemoteUrlOpener> ipc_server_{
GetRemoteOpenUrlIpcChannelName(), 0, this};

std::unique_ptr<ScopedUrlForwarder> scoped_url_forwarder_;

static_assert(
std::is_same<
mojo::ReceiverId,
Expand Down
27 changes: 0 additions & 27 deletions remoting/host/scoped_url_forwarder.cc

This file was deleted.

29 changes: 0 additions & 29 deletions remoting/host/scoped_url_forwarder.h

This file was deleted.

67 changes: 0 additions & 67 deletions remoting/host/scoped_url_forwarder_linux.cc

This file was deleted.

30 changes: 0 additions & 30 deletions remoting/host/scoped_url_forwarder_linux.h

This file was deleted.

27 changes: 27 additions & 0 deletions remoting/host/url_forwarder_configurator.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2021 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/url_forwarder_configurator.h"

#include "base/macros.h"
#include "build/build_config.h"

namespace remoting {

UrlForwarderConfigurator::UrlForwarderConfigurator() = default;

UrlForwarderConfigurator::~UrlForwarderConfigurator() = default;

#if !defined(OS_LINUX)

// static
UrlForwarderConfigurator* UrlForwarderConfigurator::GetInstance() {
// Unsupported platforms.
NOTREACHED();
return nullptr;
}

#endif // !defined(OS_LINUX)

} // namespace remoting

0 comments on commit 9e5d621

Please sign in to comment.