Skip to content

Commit

Permalink
Remove unused parts of IPC::ChannelHandle.
Browse files Browse the repository at this point in the history
In SFI NaCl, ChannelHandle is just a file descriptor. On other platforms
it's just a mojo message pipe handle. This CL removes the other unused
fields. It also removes IPC::Channel::GenerateMojoChannelHandlePair()
and updates its remaining use.

BUG=659448

Review-Url: https://codereview.chromium.org/2484943004
Cr-Commit-Position: refs/heads/master@{#431821}
  • Loading branch information
sammc authored and Commit bot committed Nov 14, 2016
1 parent 340ad8a commit 9bf370c
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 134 deletions.
1 change: 1 addition & 0 deletions android_webview/browser/aw_printing_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "android_webview/browser/aw_printing_message_filter.h"

#include "android_webview/browser/aw_print_manager.h"
#include "base/file_descriptor_posix.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_view_host.h"
Expand Down
4 changes: 4 additions & 0 deletions android_webview/browser/aw_printing_message_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
#include "base/macros.h"
#include "content/public/browser/browser_message_filter.h"

namespace base {
struct FileDescriptor;
}

namespace android_webview {

// This class filters out incoming printing related IPC messages for the
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/printing/printing_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#endif

#if defined(OS_ANDROID)
#include "base/file_descriptor_posix.h"
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/printing/print_view_manager_basic.h"
#endif
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/printing/printing_message_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class Profile;

namespace base {
class DictionaryValue;
#if defined(OS_ANDROID)
struct FileDescriptor;
#endif
}

namespace printing {
Expand Down
10 changes: 0 additions & 10 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,6 @@ class IPC_EXPORT Channel : public Sender {
static std::string GenerateUniqueRandomChannelID();
#endif

// Deprecated: Create a mojo::MessagePipe directly and release() its handles
// instead.
//
// Generates a pair of channel handles that can be used as the client and
// server ends of a ChannelMojo. |name_postfix| is ignored.
static void GenerateMojoChannelHandlePair(
const std::string& name_postfix,
IPC::ChannelHandle* handle0,
IPC::ChannelHandle* handle1);

#if defined(OS_LINUX)
// Sandboxed processes live in a PID namespace, so when sending the IPC hello
// message from client to server we need to send the PID from the global
Expand Down
11 changes: 0 additions & 11 deletions ipc/ipc_channel_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,6 @@ std::unique_ptr<Channel> Channel::CreateServer(
#endif
}

// static
void Channel::GenerateMojoChannelHandlePair(
const std::string& name_postfix,
IPC::ChannelHandle* handle0,
IPC::ChannelHandle* handle1) {
DCHECK_NE(handle0, handle1);
mojo::MessagePipe message_pipe;
*handle0 = ChannelHandle(message_pipe.handle0.release());
*handle1 = ChannelHandle(message_pipe.handle1.release());
}

Channel::~Channel() {
}

Expand Down
68 changes: 14 additions & 54 deletions ipc/ipc_channel_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,71 +10,31 @@
#include "build/build_config.h"
#include "mojo/public/cpp/system/message_pipe.h"

#if defined(OS_POSIX)
#if defined(OS_NACL_SFI)
#include "base/file_descriptor_posix.h"
#elif defined(OS_WIN)
#include <windows.h>
#endif // defined (OS_WIN)

// On Windows, any process can create an IPC channel and others can fetch
// it by name. We pass around the channel names over IPC.
// On Windows the initialization of ChannelHandle with an existing pipe
// handle is provided for convenience.
// NOTE: A ChannelHandle with a pipe handle Will NOT be marshalled over IPC.

// On POSIX, we instead pass around handles to channel endpoints via IPC.
// When it's time to IPC a new channel endpoint around, we send both the
// channel name as well as a base::FileDescriptor, which is itself a special
// type that knows how to copy a socket endpoint over IPC.
//
// In sum, this data structure can be used to pass channel information by name
// in both Windows and Posix. When passing a handle to a channel over IPC,
// use this data structure only for POSIX.
#endif // defined (OS_NACL_SFI)

namespace IPC {

// Note that serialization for this object is defined in the ParamTraits
// template specialization in ipc_message_utils.h.
#if defined(OS_NACL_SFI)
struct ChannelHandle {
ChannelHandle() {}
explicit ChannelHandle(const base::FileDescriptor& s) : socket(s) {}

base::FileDescriptor socket;
};
#else
struct ChannelHandle {
// Note that serialization for this object is defined in the ParamTraits
// template specialization in ipc_message_utils.h.
ChannelHandle() {}
// The name that is passed in should be an absolute path for Posix.
// Otherwise there may be a problem in IPC communication between
// processes with different working directories.
ChannelHandle(const std::string& n) : name(n) {}
ChannelHandle(const char* n) : name(n) {}
#if defined(OS_WIN)
explicit ChannelHandle(HANDLE h) : pipe(h) {}
#elif defined(OS_POSIX)
ChannelHandle(const std::string& n, const base::FileDescriptor& s)
: name(n), socket(s) {}
#endif // defined(OS_POSIX)
ChannelHandle(mojo::MessagePipeHandle h) : mojo_handle(h) {}

bool is_mojo_channel_handle() const {
#if defined(OS_WIN)
if (pipe.handle)
return false;
#elif defined(OS_POSIX)
if (socket.fd != -1)
return false;
#endif // defined(OS_POSIX)
return mojo_handle.is_valid() && name.empty();
}
bool is_mojo_channel_handle() const { return mojo_handle.is_valid(); }

std::string name;
#if defined(OS_POSIX)
base::FileDescriptor socket;
#elif defined(OS_WIN)
// A simple container to automatically initialize pipe handle
struct PipeHandle {
PipeHandle() : handle(NULL) {}
PipeHandle(HANDLE h) : handle(h) {}
HANDLE handle;
};
PipeHandle pipe;
#endif // defined (OS_WIN)
mojo::MessagePipeHandle mojo_handle;
};
#endif // defined(OS_NACL_SFI)

} // namespace IPC

Expand Down
6 changes: 2 additions & 4 deletions ipc/ipc_channel_nacl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,11 @@ ChannelNacl::ChannelNacl(const IPC::ChannelHandle& channel_handle,
mode_(mode),
waiting_connect_(true),
pipe_(-1),
pipe_name_(channel_handle.name),
weak_ptr_factory_(this) {
if (!CreatePipe(channel_handle)) {
// The pipe may have been closed already.
const char *modestr = (mode_ & MODE_SERVER_FLAG) ? "server" : "client";
LOG(WARNING) << "Unable to create pipe named \"" << channel_handle.name
<< "\" in " << modestr << " mode";
LOG(WARNING) << "Unable to create pipe in " << modestr << " mode";
}
}

Expand All @@ -148,7 +146,7 @@ bool ChannelNacl::Connect() {
WillConnect();

if (pipe_ == -1) {
DLOG(WARNING) << "Channel creation failed: " << pipe_name_;
DLOG(WARNING) << "Channel creation failed";
return false;
}

Expand Down
6 changes: 0 additions & 6 deletions ipc/ipc_channel_nacl.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ class ChannelNacl : public Channel,
// The pipe used for communication.
int pipe_;

// The "name" of our pipe. On Windows this is the global identifier for
// the pipe. On POSIX it's used as a key in a local map of file descriptors.
// For NaCl, we don't actually support looking up file descriptors by name,
// and it's only used for debug information.
std::string pipe_name_;

// We use a thread for reading, so that we can simply block on reading and
// post the received data back to the main thread to be properly interleaved
// with other tasks in the MessagePump.
Expand Down
34 changes: 15 additions & 19 deletions ipc/ipc_message_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "ipc/ipc_mojo_param_traits.h"

#if defined(OS_POSIX)
#include "base/file_descriptor_posix.h"
#include "ipc/ipc_platform_file_attachment_posix.h"
#endif

Expand Down Expand Up @@ -1018,45 +1019,40 @@ void ParamTraits<base::UnguessableToken>::Log(const param_type& p,

void ParamTraits<IPC::ChannelHandle>::GetSize(base::PickleSizer* sizer,
const param_type& p) {
GetParamSize(sizer, p.name);
#if defined(OS_POSIX)
#if defined(OS_NACL_SFI)
GetParamSize(sizer, p.socket);
#endif
#else
GetParamSize(sizer, p.mojo_handle);
#endif
}

void ParamTraits<IPC::ChannelHandle>::Write(base::Pickle* m,
const param_type& p) {
#if defined(OS_WIN)
// On Windows marshalling pipe handle is not supported.
DCHECK(p.pipe.handle == NULL);
#endif // defined (OS_WIN)
WriteParam(m, p.name);
#if defined(OS_POSIX)
#if defined(OS_NACL_SFI)
WriteParam(m, p.socket);
#endif
#else
WriteParam(m, p.mojo_handle);
#endif
}

bool ParamTraits<IPC::ChannelHandle>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* r) {
return ReadParam(m, iter, &r->name)
#if defined(OS_POSIX)
&& ReadParam(m, iter, &r->socket)
#if defined(OS_NACL_SFI)
return ReadParam(m, iter, &r->socket);
#else
return ReadParam(m, iter, &r->mojo_handle);
#endif
&& ReadParam(m, iter, &r->mojo_handle);
}

void ParamTraits<IPC::ChannelHandle>::Log(const param_type& p,
std::string* l) {
l->append(base::StringPrintf("ChannelHandle(%s", p.name.c_str()));
#if defined(OS_POSIX)
l->append(", ");
l->append("ChannelHandle(");
#if defined(OS_NACL_SFI)
ParamTraits<base::FileDescriptor>::Log(p.socket, l);
#endif
l->append(", ");
#else
LogParam(p.mojo_handle, l);
#endif
l->append(")");
}

Expand Down
6 changes: 0 additions & 6 deletions ipc/ipc_message_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,6 @@ TEST(IPCMessageUtilsTest, MojoChannelHandle) {
base::PickleIterator iter(message);
IPC::ChannelHandle result_handle;
EXPECT_TRUE(IPC::ReadParam(&message, &iter, &result_handle));
EXPECT_TRUE(result_handle.name.empty());
#if defined(OS_POSIX)
EXPECT_EQ(-1, result_handle.socket.fd);
#elif defined(OS_WIN)
EXPECT_EQ(nullptr, result_handle.pipe.handle);
#endif
EXPECT_EQ(channel_handle.mojo_handle, result_handle.mojo_handle);
}

Expand Down
12 changes: 5 additions & 7 deletions ppapi/nacl_irt/irt_start.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
#include "ppapi/nacl_irt/plugin_startup.h"

namespace {
IPC::ChannelHandle MakeIPCHandle(const char* name, int fd) {
return IPC::ChannelHandle(name,
base::FileDescriptor(fd, false /* auto_close */));
IPC::ChannelHandle MakeIPCHandle(int fd) {
return IPC::ChannelHandle(base::FileDescriptor(fd, false /* auto_close */));
}
} // namespace

Expand All @@ -32,10 +31,9 @@ void nacl_irt_start(uint32_t* info) {

// In SFI mode, the FDs of IPC channels are NACL_CHROME_DESC_BASE and its
// successor, which is set in nacl_listener.cc.
ppapi::SetIPCChannelHandles(
MakeIPCHandle("NaCl Browser", NACL_CHROME_DESC_BASE),
MakeIPCHandle("NaCl Renderer", NACL_CHROME_DESC_BASE + 1),
MakeIPCHandle("NaCl Manifest", NACL_CHROME_DESC_BASE + 2));
ppapi::SetIPCChannelHandles(MakeIPCHandle(NACL_CHROME_DESC_BASE),
MakeIPCHandle(NACL_CHROME_DESC_BASE + 1),
MakeIPCHandle(NACL_CHROME_DESC_BASE + 2));
// The Mojo EDK must be initialized before using IPC.
mojo::edk::Init();
ppapi::StartUpPlugin();
Expand Down
13 changes: 6 additions & 7 deletions ppapi/proxy/ppapi_proxy_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -547,19 +547,18 @@ void TwoWayTest::SetUp() {
io_thread_.StartWithOptions(options);
plugin_thread_.Start();

IPC::ChannelHandle local_handle, remote_handle;
IPC::Channel::GenerateMojoChannelHandlePair("TwoWayTestChannel",
&local_handle, &remote_handle);
mojo::MessagePipe pipe;
base::WaitableEvent remote_harness_set_up(
base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
plugin_thread_.task_runner()->PostTask(
FROM_HERE, base::Bind(&SetUpRemoteHarness, remote_harness_, remote_handle,
base::RetainedRef(io_thread_.task_runner()),
&shutdown_event_, &remote_harness_set_up));
FROM_HERE,
base::Bind(&SetUpRemoteHarness, remote_harness_, pipe.handle0.release(),
base::RetainedRef(io_thread_.task_runner()), &shutdown_event_,
&remote_harness_set_up));
remote_harness_set_up.Wait();
local_harness_->SetUpHarnessWithChannel(
local_handle, io_thread_.task_runner().get(), &shutdown_event_,
pipe.handle1.release(), io_thread_.task_runner().get(), &shutdown_event_,
true); // is_client
}

Expand Down
11 changes: 1 addition & 10 deletions tools/ipc_fuzzer/fuzzer/fuzzer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1324,16 +1324,7 @@ struct FuzzTraits<IPC::ChannelHandle> {
if (!fuzzer->ShouldGenerate())
return true;

// TODO(inferno): Add way to generate real channel handles.
#if defined(OS_WIN)
HANDLE fake_handle = (HANDLE)(RandU64());
p->pipe = IPC::ChannelHandle::PipeHandle(fake_handle);
return true;
#elif defined(OS_POSIX)
return
FuzzParam(&p->name, fuzzer) &&
FuzzParam(&p->socket, fuzzer);
#endif
return FuzzParam(&p->mojo_handle, fuzzer);
}
};

Expand Down

0 comments on commit 9bf370c

Please sign in to comment.