Skip to content

Commit

Permalink
Remoting: Remove references to POSIX shared memory.
Browse files Browse the repository at this point in the history
https://codereview.chromium.org/1509723003/ removed all dependencies on the
implementation of shared memory. This CL removes the remaining references.

BUG=547247

Review URL: https://codereview.chromium.org/1509063003

Cr-Commit-Position: refs/heads/master@{#367117}
  • Loading branch information
erikchen authored and Commit bot committed Dec 29, 2015
1 parent 6ccca56 commit 9e1ffaa
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 33 deletions.
25 changes: 25 additions & 0 deletions ipc/attachment_broker_privileged.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
#endif

#if defined(OS_MACOSX) && !defined(OS_IOS)
#include <mach/mach.h>

#include "base/process/port_provider_mac.h"
#include "ipc/attachment_broker_privileged_mac.h"
#endif

Expand All @@ -24,6 +27,19 @@ namespace IPC {
namespace {

#if defined(OS_MACOSX) && !defined(OS_IOS)

// A fake port provider that does nothing. Intended for single process unit
// tests.
class FakePortProvider : public base::PortProvider {
mach_port_t TaskForPid(base::ProcessHandle process) const override {
DCHECK_EQ(process, getpid());
return mach_task_self();
}
};

base::LazyInstance<FakePortProvider>::Leaky
g_fake_port_provider = LAZY_INSTANCE_INITIALIZER;

// Passed as a constructor parameter to AttachmentBrokerPrivilegedMac.
base::PortProvider* g_port_provider = nullptr;
#endif // defined(OS_MACOSX) && !defined(OS_IOS)
Expand Down Expand Up @@ -86,6 +102,15 @@ void AttachmentBrokerPrivileged::CreateBrokerIfNeeded() {
}
#endif // defined(OS_MACOSX) && !defined(OS_IOS)

// static
void AttachmentBrokerPrivileged::CreateBrokerForSingleProcessTests() {
#if defined(OS_MACOSX) && !defined(OS_IOS)
CreateBrokerIfNeeded(&g_fake_port_provider.Get());
#else
CreateBrokerIfNeeded();
#endif // defined(OS_MACOSX) && !defined(OS_IOS)
}

void AttachmentBrokerPrivileged::RegisterCommunicationChannel(
Endpoint* endpoint) {
base::AutoLock auto_lock(*get_lock());
Expand Down
5 changes: 5 additions & 0 deletions ipc/attachment_broker_privileged.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ class IPC_EXPORT AttachmentBrokerPrivileged : public IPC::AttachmentBroker {
static void CreateBrokerIfNeeded();
#endif // defined(OS_MACOSX) && !defined(OS_IOS)

// Similar to CreateBrokerIfNeeded(), but useful for single process unit tests
// that don't need real attachment brokering, and don't want to deal with
// setting up a fake PortProvider.
static void CreateBrokerForSingleProcessTests();

// AttachmentBroker overrides.
void RegisterCommunicationChannel(Endpoint* endpoint) override;
void DeregisterCommunicationChannel(Endpoint* endpoint) override;
Expand Down
3 changes: 2 additions & 1 deletion remoting/host/chromoting_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <stdint.h>

#include "base/memory/shared_memory_handle.h"
#include "ipc/ipc_platform_file.h"
#include "net/base/ip_endpoint.h"
#include "remoting/host/chromoting_param_traits.h"
Expand Down Expand Up @@ -139,7 +140,7 @@ IPC_MESSAGE_CONTROL0(ChromotingDesktopDaemonMsg_InjectSas)
// Notifies the network process that a shared buffer has been created.
IPC_MESSAGE_CONTROL3(ChromotingDesktopNetworkMsg_CreateSharedBuffer,
int /* id */,
IPC::PlatformFileForTransit /* handle */,
base::SharedMemoryHandle /* handle */,
uint32_t /* size */)

// Request the network process to stop using a shared buffer.
Expand Down
18 changes: 1 addition & 17 deletions remoting/host/desktop_session_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,8 @@ class DesktopSessionAgent::SharedBuffer : public webrtc::SharedMemory {
size_t size,
int id) {
scoped_ptr<base::SharedMemory> memory(new base::SharedMemory());
#if defined(OS_MACOSX) && !defined(OS_IOS)
// Remoting does not yet support Mach primitive backed SharedMemory, so
// force the underlying primitive to be a POSIX fd.
// https://crbug.com/547247.
if (!memory->CreateAndMapAnonymousPosix(size))
return nullptr;
#else
if (!memory->CreateAndMapAnonymous(size))
return nullptr;
#endif // defined(OS_MACOSX) && !defined(OS_IOS)
return make_scoped_ptr(
new SharedBuffer(agent, std::move(memory), size, id));
}
Expand Down Expand Up @@ -204,16 +196,8 @@ webrtc::SharedMemory* DesktopSessionAgent::CreateSharedMemory(size_t size) {
// speaking it never happens.
next_shared_buffer_id_ += 2;

IPC::PlatformFileForTransit handle;
#if defined(OS_WIN)
handle = buffer->shared_memory()->handle().GetHandle(),
#else
handle =
base::FileDescriptor(base::SharedMemory::GetFdFromSharedMemoryHandle(
buffer->shared_memory()->handle()), false);
#endif
SendToNetwork(new ChromotingDesktopNetworkMsg_CreateSharedBuffer(
buffer->id(), handle, buffer->size()));
buffer->id(), buffer->shared_memory()->handle(), buffer->size()));
}

return buffer.release();
Expand Down
16 changes: 2 additions & 14 deletions remoting/host/desktop_session_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ class DesktopSessionProxy::IpcSharedBufferCore
size_(size) {
if (!shared_memory_.Map(size)) {
LOG(ERROR) << "Failed to map a shared buffer: id=" << id
#if defined(OS_WIN)
<< ", handle=" << handle.GetHandle()
#else
<< ", handle.fd="
<< base::SharedMemory::GetFdFromSharedMemoryHandle(handle)
#endif
<< ", size=" << size;
}
}
Expand Down Expand Up @@ -476,18 +470,12 @@ void DesktopSessionProxy::OnAudioPacket(const std::string& serialized_packet) {

void DesktopSessionProxy::OnCreateSharedBuffer(
int id,
IPC::PlatformFileForTransit handle,
base::SharedMemoryHandle handle,
uint32_t size) {
DCHECK(caller_task_runner_->BelongsToCurrentThread());

#if defined(OS_WIN)
base::SharedMemoryHandle shm_handle =
base::SharedMemoryHandle(handle, base::GetCurrentProcId());
#else
base::SharedMemoryHandle shm_handle = base::SharedMemoryHandle(handle);
#endif
scoped_refptr<IpcSharedBufferCore> shared_buffer =
new IpcSharedBufferCore(id, shm_handle, desktop_process_.Handle(), size);
new IpcSharedBufferCore(id, handle, desktop_process_.Handle(), size);

if (shared_buffer->memory() != nullptr &&
!shared_buffers_.insert(std::make_pair(id, shared_buffer)).second) {
Expand Down
3 changes: 2 additions & 1 deletion remoting/host/desktop_session_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/shared_memory_handle.h"
#include "base/memory/weak_ptr.h"
#include "base/process/process.h"
#include "base/sequenced_task_runner_helpers.h"
Expand Down Expand Up @@ -153,7 +154,7 @@ class DesktopSessionProxy

// Registers a new shared buffer created by the desktop process.
void OnCreateSharedBuffer(int id,
IPC::PlatformFileForTransit handle,
base::SharedMemoryHandle handle,
uint32_t size);

// Drops a cached reference to the shared buffer.
Expand Down
2 changes: 2 additions & 0 deletions remoting/host/ipc_desktop_environment_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "base/process/process_handle.h"
#include "base/run_loop.h"
#include "build/build_config.h"
#include "ipc/attachment_broker_privileged.h"
#include "ipc/ipc_channel.h"
#include "ipc/ipc_channel_proxy.h"
#include "ipc/ipc_listener.h"
Expand Down Expand Up @@ -240,6 +241,7 @@ IpcDesktopEnvironmentTest::IpcDesktopEnvironmentTest()
remote_input_injector_(nullptr),
terminal_id_(-1),
client_session_control_factory_(&client_session_control_) {
IPC::AttachmentBrokerPrivileged::CreateBrokerForSingleProcessTests();
}

IpcDesktopEnvironmentTest::~IpcDesktopEnvironmentTest() {
Expand Down
13 changes: 13 additions & 0 deletions remoting/host/remoting_me2me_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "base/strings/stringize_macros.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "ipc/attachment_broker_unprivileged.h"
#include "ipc/ipc_channel.h"
#include "ipc/ipc_channel_proxy.h"
#include "ipc/ipc_listener.h"
Expand Down Expand Up @@ -381,6 +382,8 @@ class HostProcess : public ConfigWatcher::Delegate,

scoped_ptr<ChromotingHostContext> context_;

scoped_ptr<IPC::AttachmentBrokerUnprivileged> attachment_broker_;

// Accessed on the UI thread.
scoped_ptr<IPC::ChannelProxy> daemon_channel_;

Expand Down Expand Up @@ -471,6 +474,7 @@ HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
int* exit_code_out,
ShutdownWatchdog* shutdown_watchdog)
: context_(std::move(context)),
attachment_broker_(IPC::AttachmentBrokerUnprivileged::CreateBroker()),
state_(HOST_STARTING),
use_service_account_(false),
enable_vp9_(false),
Expand Down Expand Up @@ -537,6 +541,11 @@ bool HostProcess::InitWithCommandLine(const base::CommandLine* cmd_line) {
IPC::Channel::MODE_CLIENT,
this,
context_->network_task_runner());
if (attachment_broker_) {
attachment_broker_->DesignateBrokerCommunicationChannel(
daemon_channel_.get());
}

#else // !defined(REMOTING_MULTI_PROCESS)
// Connect to the daemon process.
std::string channel_name =
Expand All @@ -545,6 +554,10 @@ bool HostProcess::InitWithCommandLine(const base::CommandLine* cmd_line) {
daemon_channel_ =
IPC::ChannelProxy::Create(channel_name, IPC::Channel::MODE_CLIENT, this,
context_->network_task_runner().get());
if (attachment_broker_) {
attachment_broker_->DesignateBrokerCommunicationChannel(
daemon_channel_.get());
}
}

if (cmd_line->HasSwitch(kHostConfigSwitchName)) {
Expand Down

0 comments on commit 9e1ffaa

Please sign in to comment.