Skip to content

Commit

Permalink
Introduce IPC::Channel::Create*() to ensure it being heap-allocated.
Browse files Browse the repository at this point in the history
This change introduces IPC::Channel::Create*() API to turn
IPC::Channel into a heap allocated object. This will allow us to
make Channel a polymorphic class.

This change also tries to hide Channel::Mode from public API
so that we can simplify channel creation code paths cleaner in
following changes. ChannelProxy has to follow same pattern to
finish this cleanup. Such changes will follow.

TEST=none
BUG=377980
R=darin@chromium.org,cpu@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273575

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273713 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
morrita@chromium.org committed May 30, 2014
1 parent 1df3479 commit e482111
Show file tree
Hide file tree
Showing 23 changed files with 235 additions and 142 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/pepper_flash_settings_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ void PepperFlashSettingsManager::Core::ConnectToChannel(
return;
}

channel_.reset(new IPC::Channel(handle, IPC::Channel::MODE_CLIENT, this));
channel_ = IPC::Channel::CreateClient(handle, this);
if (!channel_->Connect()) {
DLOG(ERROR) << "Couldn't connect to plugin";
NotifyErrorFromIOThread();
Expand Down
11 changes: 5 additions & 6 deletions chrome/utility/importer/firefox_importer_unittest_utils_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,7 @@ bool FFUnitTestDecryptorProxy::Setup(const base::FilePath& nss_path) {
message_loop_.reset(new base::MessageLoopForIO());

listener_.reset(new FFDecryptorServerChannelListener());
channel_.reset(new IPC::Channel(kTestChannelID,
IPC::Channel::MODE_SERVER,
listener_.get()));
channel_ = IPC::Channel::CreateServer(kTestChannelID, listener_.get());
CHECK(channel_->Connect());
listener_->SetSender(channel_.get());

Expand Down Expand Up @@ -264,9 +262,10 @@ MULTIPROCESS_IPC_TEST_MAIN(NSSDecrypterChildProcess) {
base::MessageLoopForIO main_message_loop;
FFDecryptorClientChannelListener listener;

IPC::Channel channel(kTestChannelID, IPC::Channel::MODE_CLIENT, &listener);
CHECK(channel.Connect());
listener.SetSender(&channel);
scoped_ptr<IPC::Channel> channel = IPC::Channel::CreateClient(
kTestChannelID, &listener);
CHECK(channel->Connect());
listener.SetSender(channel.get());

// run message loop
base::MessageLoop::current()->Run();
Expand Down
4 changes: 2 additions & 2 deletions cloud_print/service/win/service_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ void ServiceListener::Connect() {
SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION |
FILE_FLAG_OVERLAPPED, NULL));
if (handle.IsValid()) {
channel_.reset(new IPC::Channel(IPC::ChannelHandle(handle),
IPC::Channel::MODE_CLIENT, this));
channel_ = IPC::Channel::CreateClient(IPC::ChannelHandle(handle),
this);
channel_->Connect();
} else {
ipc_thread_->message_loop()->PostDelayedTask(
Expand Down
4 changes: 2 additions & 2 deletions cloud_print/service/win/setup_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ void SetupListener::Connect(const base::string16& user) {
IPC::Channel::kReadBufferSize,
IPC::Channel::kReadBufferSize, 5000, &attribs));
if (pipe.IsValid()) {
channel_.reset(new IPC::Channel(IPC::ChannelHandle(pipe),
IPC::Channel::MODE_SERVER, this));
channel_ = IPC::Channel::CreateServer(IPC::ChannelHandle(pipe),
this);
channel_->Connect();
}
}
Expand Down
3 changes: 1 addition & 2 deletions components/nacl/broker/nacl_broker_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ void NaClBrokerListener::Listen() {
std::string channel_name =
CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kProcessChannelID);
channel_.reset(new IPC::Channel(
channel_name, IPC::Channel::MODE_CLIENT, this));
channel_ = IPC::Channel::CreateClient(channel_name, this);
CHECK(channel_->Connect());
base::MessageLoop::current()->Run();
}
Expand Down
3 changes: 1 addition & 2 deletions components/nacl/loader/nacl_ipc_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,7 @@ NaClIPCAdapter::NaClIPCAdapter(const IPC::ChannelHandle& handle,
cond_var_(&lock_),
task_runner_(runner),
locked_data_() {
io_thread_data_.channel_.reset(
new IPC::Channel(handle, IPC::Channel::MODE_SERVER, this));
io_thread_data_.channel_ = IPC::Channel::CreateServer(handle, this);
// Note, we can not PostTask for ConnectChannelOnIOThread here. If we did,
// and that task ran before this constructor completes, the reference count
// would go to 1 and then to 0 because of the Task, before we've been returned
Expand Down
2 changes: 1 addition & 1 deletion content/browser/plugin_data_remover_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class PluginDataRemoverImpl::Context
return;

DCHECK(!channel_.get());
channel_.reset(new IPC::Channel(handle, IPC::Channel::MODE_CLIENT, this));
channel_ = IPC::Channel::CreateClient(handle, this);
if (!channel_->Connect()) {
NOTREACHED() << "Couldn't connect to plugin";
SignalDone();
Expand Down
2 changes: 1 addition & 1 deletion content/browser/plugin_service_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class MockPluginProcessHostClient : public PluginProcessHost::Client,
ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
ASSERT_TRUE(set_plugin_info_called_);
ASSERT_TRUE(!channel_);
channel_ = new IPC::Channel(handle, IPC::Channel::MODE_CLIENT, this);
channel_ = IPC::Channel::CreateClient(handle, this).release();
ASSERT_TRUE(channel_->Connect());
}

Expand Down
3 changes: 1 addition & 2 deletions content/common/child_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@ void ChildProcessHostImpl::ForceShutdown() {

std::string ChildProcessHostImpl::CreateChannel() {
channel_id_ = IPC::Channel::GenerateVerifiedChannelID(std::string());
channel_.reset(new IPC::Channel(
channel_id_, IPC::Channel::MODE_SERVER, this));
channel_ = IPC::Channel::CreateServer(channel_id_, this);
if (!channel_->Connect())
return std::string();

Expand Down
5 changes: 3 additions & 2 deletions content/renderer/render_thread_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ TEST_F(RenderThreadImplBrowserTest,
std::string channel_id = IPC::Channel::GenerateVerifiedChannelID(
std::string());
DummyListener dummy_listener;
IPC::Channel channel(channel_id, IPC::Channel::MODE_SERVER, &dummy_listener);
ASSERT_TRUE(channel.Connect());
scoped_ptr<IPC::Channel> channel(
IPC::Channel::CreateServer(channel_id, &dummy_listener));
ASSERT_TRUE(channel->Connect());

scoped_ptr<MockRenderProcess> mock_process(new MockRenderProcess);
// Owned by mock_process.
Expand Down
1 change: 1 addition & 0 deletions ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ component("ipc") {
"file_descriptor_set_posix.h",
"ipc_channel.cc",
"ipc_channel.h",
"ipc_channel_common.cc",
"ipc_channel_factory.cc",
"ipc_channel_factory.h",
"ipc_channel_handle.h",
Expand Down
1 change: 1 addition & 0 deletions ipc/ipc.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'file_descriptor_set_posix.h',
'ipc_channel.cc',
'ipc_channel.h',
'ipc_channel_common.cc',
'ipc_channel_factory.cc',
'ipc_channel_factory.h',
'ipc_channel_handle.h',
Expand Down
57 changes: 43 additions & 14 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,15 @@ class IPC_EXPORT Channel : public Sender {
};

// Some Standard Modes
// TODO(morrita): These are under deprecation work. You should use Create*()
// functions instead.
enum Mode {
MODE_NONE = MODE_NO_FLAG,
MODE_SERVER = MODE_SERVER_FLAG,
MODE_CLIENT = MODE_CLIENT_FLAG,
// Channels on Windows are named by default and accessible from other
// processes. On POSIX channels are anonymous by default and not accessible
// from other processes. Named channels work via named unix domain sockets.
// On Windows MODE_NAMED_SERVER is equivalent to MODE_SERVER and
// MODE_NAMED_CLIENT is equivalent to MODE_CLIENT.
MODE_NAMED_SERVER = MODE_SERVER_FLAG | MODE_NAMED_FLAG,
MODE_NAMED_CLIENT = MODE_CLIENT_FLAG | MODE_NAMED_FLAG,
#if defined(OS_POSIX)
// An "open" named server accepts connections from ANY client.
// The caller must then implement their own access-control based on the
// client process' user Id.
MODE_OPEN_NAMED_SERVER = MODE_OPEN_ACCESS_FLAG | MODE_SERVER_FLAG |
MODE_NAMED_FLAG
#endif
Expand Down Expand Up @@ -106,15 +100,47 @@ class IPC_EXPORT Channel : public Sender {
// the file descriptor in the channel handle is != -1, the channel takes
// ownership of the file descriptor and will close it appropriately, otherwise
// it will create a new descriptor internally.
// |mode| specifies whether this Channel is to operate in server mode or
// client mode. In server mode, the Channel is responsible for setting up the
// IPC object, whereas in client mode, the Channel merely connects to the
// already established IPC object.
// |listener| receives a callback on the current thread for each newly
// received message.
//
Channel(const IPC::ChannelHandle &channel_handle, Mode mode,
Listener* listener);
// There are four type of modes how channels operate:
//
// - Server and named server: In these modes, the Channel is
// responsible for settingb up the IPC object
// - An "open" named server: It accepts connections from ANY client.
// The caller must then implement their own access-control based on the
// client process' user Id.
// - Client and named client: In these mode, the Channel merely
// connects to the already established IPC object.
//
// Each mode has its own Create*() API to create the Channel object.
//
// TODO(morrita): Replace CreateByModeForProxy() with one of above Create*().
//
static scoped_ptr<Channel> CreateByModeForProxy(
const IPC::ChannelHandle &channel_handle, Mode mode,Listener* listener);
static scoped_ptr<Channel> CreateClient(
const IPC::ChannelHandle &channel_handle, Listener* listener);

// Channels on Windows are named by default and accessible from other
// processes. On POSIX channels are anonymous by default and not accessible
// from other processes. Named channels work via named unix domain sockets.
// On Windows MODE_NAMED_SERVER is equivalent to MODE_SERVER and
// MODE_NAMED_CLIENT is equivalent to MODE_CLIENT.
static scoped_ptr<Channel> CreateNamedServer(
const IPC::ChannelHandle &channel_handle, Listener* listener);
static scoped_ptr<Channel> CreateNamedClient(
const IPC::ChannelHandle &channel_handle, Listener* listener);
#if defined(OS_POSIX)
// An "open" named server accepts connections from ANY client.
// The caller must then implement their own access-control based on the
// client process' user Id.
static scoped_ptr<Channel> CreateOpenNamedServer(
const IPC::ChannelHandle &channel_handle, Listener* listener);
#endif
static scoped_ptr<Channel> CreateServer(
const IPC::ChannelHandle &channel_handle, Listener* listener);


virtual ~Channel();

Expand Down Expand Up @@ -220,6 +246,9 @@ class IPC_EXPORT Channel : public Sender {
Channel() : channel_impl_(0) { }

private:
Channel(const IPC::ChannelHandle &channel_handle, Mode mode,
Listener* listener);

// PIMPL to which all channel calls are delegated.
class ChannelImpl;
ChannelImpl *channel_impl_;
Expand Down
55 changes: 55 additions & 0 deletions ipc/ipc_channel_common.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2014 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 "ipc/ipc_channel.h"

namespace IPC {

// static
scoped_ptr<Channel> Channel::CreateByModeForProxy(
const IPC::ChannelHandle &channel_handle, Mode mode, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, mode, listener));
}

// static
scoped_ptr<Channel> Channel::CreateClient(
const IPC::ChannelHandle &channel_handle, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, Channel::MODE_CLIENT, listener));
}

// static
scoped_ptr<Channel> Channel::CreateNamedServer(
const IPC::ChannelHandle &channel_handle, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, Channel::MODE_NAMED_SERVER, listener));
}

// static
scoped_ptr<Channel> Channel::CreateNamedClient(
const IPC::ChannelHandle &channel_handle, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, Channel::MODE_NAMED_CLIENT, listener));
}

#if defined(OS_POSIX)
// static
scoped_ptr<Channel> Channel::CreateOpenNamedServer(
const IPC::ChannelHandle &channel_handle, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, Channel::MODE_OPEN_NAMED_SERVER, listener));
}
#endif

// static
scoped_ptr<Channel> Channel::CreateServer(
const IPC::ChannelHandle &channel_handle, Listener* listener) {
return make_scoped_ptr(
new Channel(channel_handle, Channel::MODE_SERVER, listener));
}


} // namespace IPC

Loading

0 comments on commit e482111

Please sign in to comment.