Skip to content

Commit

Permalink
Remove dead code for IPC channels
Browse files Browse the repository at this point in the history
We no longer use the code path to add IO-thread associated interfaces
directly to an IPC::Channel, except for some BrowserMessageFilter code
which also turns out to be dead code.

Delete this stuff.

Bug: None
Change-Id: Iad964273134fb953562c3829359117eee11179e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5530315
Reviewed-by: Nasko Oskov <[email protected]>
Reviewed-by: Tom Sepez <[email protected]>
Commit-Queue: Ken Rockot <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1299048}
  • Loading branch information
krockot authored and Chromium LUCI CQ committed May 10, 2024
1 parent 6be81bf commit 2cf8572
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ void AgentSchedulingGroupHost::AddFilter(BrowserMessageFilter* filter) {
return;
}

filter->RegisterAssociatedInterfaces(channel_.get());
channel_->AddFilter(filter->GetFilter());
}
#endif
Expand Down
1 change: 0 additions & 1 deletion content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4301,7 +4301,6 @@ IPC::ChannelProxy* RenderProcessHostImpl::GetChannel() {

#if BUILDFLAG(CONTENT_ENABLE_LEGACY_IPC)
void RenderProcessHostImpl::AddFilter(BrowserMessageFilter* filter) {
filter->RegisterAssociatedInterfaces(channel_.get());
channel_->AddFilter(filter->GetFilter());
}
#endif
Expand Down
17 changes: 0 additions & 17 deletions content/public/browser/browser_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@ class BrowserMessageFilter::Internal : public IPC::MessageFilter {
scoped_refptr<BrowserMessageFilter> filter_;
};

BrowserMessageFilter::BrowserMessageFilter() = default;

BrowserMessageFilter::BrowserMessageFilter(uint32_t message_class_to_filter)
: message_classes_to_filter_(1, message_class_to_filter) {}

Expand All @@ -122,14 +120,6 @@ BrowserMessageFilter::BrowserMessageFilter(
DCHECK(num_message_classes_to_filter);
}

void BrowserMessageFilter::AddAssociatedInterface(
const std::string& name,
const IPC::ChannelProxy::GenericAssociatedInterfaceFactory& factory,
base::OnceClosure filter_removed_callback) {
associated_interfaces_.emplace_back(name, factory);
filter_removed_callbacks_.emplace_back(std::move(filter_removed_callback));
}

base::ProcessHandle BrowserMessageFilter::PeerHandle() {
return peer_process_.Handle();
}
Expand Down Expand Up @@ -196,11 +186,4 @@ IPC::MessageFilter* BrowserMessageFilter::GetFilter() {
return internal_;
}

void BrowserMessageFilter::RegisterAssociatedInterfaces(
IPC::ChannelProxy* proxy) {
for (const auto& entry : associated_interfaces_)
proxy->AddGenericAssociatedInterfaceForIOThread(entry.first, entry.second);
associated_interfaces_.clear();
}

} // namespace content
21 changes: 1 addition & 20 deletions content/public/browser/browser_message_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class MessageFilter;
}

namespace content {
class BrowserAssociatedInterfaceTest;
struct BrowserMessageFilterTraits;

// Base class for message filters in the browser process. You can receive and
Expand All @@ -42,7 +41,6 @@ class CONTENT_EXPORT BrowserMessageFilter
BrowserMessageFilter, BrowserMessageFilterTraits>,
public IPC::Sender {
public:
BrowserMessageFilter(); // For mojo-only BrowserAssociatedInterface.
explicit BrowserMessageFilter(uint32_t message_class_to_filter);
BrowserMessageFilter(const uint32_t* message_classes_to_filter,
size_t num_message_classes_to_filter);
Expand Down Expand Up @@ -88,16 +86,6 @@ class CONTENT_EXPORT BrowserMessageFilter
// your function will be called on the requested thread.
virtual bool OnMessageReceived(const IPC::Message& message) = 0;

// Adds an associated interface factory to this filter. Must be called before
// RegisterAssociatedInterfaces().
//
// |filter_removed_callback| is called on the IO thread when this filter is
// removed.
void AddAssociatedInterface(
const std::string& name,
const IPC::ChannelProxy::GenericAssociatedInterfaceFactory& factory,
const base::OnceClosure filter_removed_callback);

// Can be called on any thread, after OnChannelConnected is called.
base::ProcessHandle PeerHandle();

Expand Down Expand Up @@ -126,17 +114,14 @@ class CONTENT_EXPORT BrowserMessageFilter

class Internal;
friend class AgentSchedulingGroupHost;
friend class BrowserAssociatedInterfaceTest;
friend class BrowserChildProcessHostImpl;
friend class BrowserPpapiHost;
friend class RenderProcessHostImpl;

// These are private because the only classes that need access to them are
// made friends above. These are only guaranteed to be valid to call on
// creation. After that this class could outlive the filter and new interface
// registrations could race with incoming requests.
// creation. After that this class could outlive the filter.
IPC::MessageFilter* GetFilter();
void RegisterAssociatedInterfaces(IPC::ChannelProxy* proxy);

// This implements IPC::MessageFilter so that we can hide that from child
// classes. Internal keeps a reference to this class, which is why there's a
Expand All @@ -149,10 +134,6 @@ class CONTENT_EXPORT BrowserMessageFilter

std::vector<uint32_t> message_classes_to_filter_;

std::vector<std::pair<std::string,
IPC::ChannelProxy::GenericAssociatedInterfaceFactory>>
associated_interfaces_;

// Callbacks to be called in OnFilterRemoved().
std::vector<base::OnceClosure> filter_removed_callbacks_;
};
Expand Down
22 changes: 0 additions & 22 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,28 +105,6 @@ class COMPONENT_EXPORT(IPC) Channel : public Sender {
// Requests an associated interface from the remote endpoint.
virtual void GetRemoteAssociatedInterface(
mojo::GenericPendingAssociatedReceiver receiver) = 0;

// Template helper to add an interface factory to this channel.
template <typename Interface>
using AssociatedReceiverFactory = base::RepeatingCallback<void(
mojo::PendingAssociatedReceiver<Interface>)>;
template <typename Interface>
void AddAssociatedInterface(
const AssociatedReceiverFactory<Interface>& factory) {
AddGenericAssociatedInterface(
Interface::Name_,
base::BindRepeating(&BindPendingAssociatedReceiver<Interface>,
factory));
}

private:
template <typename Interface>
static void BindPendingAssociatedReceiver(
const AssociatedReceiverFactory<Interface>& factory,
mojo::ScopedInterfaceEndpointHandle handle) {
factory.Run(
mojo::PendingAssociatedReceiver<Interface>(std::move(handle)));
}
};

// The maximum message size in bytes. Attempting to receive a message of this
Expand Down
123 changes: 0 additions & 123 deletions ipc/ipc_channel_mojo_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -673,129 +673,6 @@ DEFINE_IPC_CHANNEL_MOJO_TEST_CLIENT(IPCChannelMojoTestSendOkClient) {
Close();
}

class ListenerWithSimpleAssociatedInterface
: public IPC::Listener,
public IPC::mojom::SimpleTestDriver {
public:
static const int kNumMessages;

explicit ListenerWithSimpleAssociatedInterface(base::OnceClosure quit_closure)
: quit_closure_(std::move(quit_closure)) {}

~ListenerWithSimpleAssociatedInterface() override = default;

bool OnMessageReceived(const IPC::Message& message) override {
base::PickleIterator iter(message);
int32_t should_be_expected;
EXPECT_TRUE(iter.ReadInt(&should_be_expected));
EXPECT_EQ(should_be_expected, next_expected_value_);
num_messages_received_++;
return true;
}

void OnChannelError() override { CHECK(!quit_closure_); }

void RegisterInterfaceFactory(IPC::Channel* channel) {
channel->GetAssociatedInterfaceSupport()->AddAssociatedInterface(
base::BindRepeating(
&ListenerWithSimpleAssociatedInterface::BindReceiver,
base::Unretained(this)));
}

private:
// IPC::mojom::SimpleTestDriver:
void ExpectValue(int32_t value) override {
next_expected_value_ = value;
}

void GetExpectedValue(GetExpectedValueCallback callback) override {
NOTREACHED();
}

void RequestValue(RequestValueCallback callback) override { NOTREACHED(); }

void RequestQuit(RequestQuitCallback callback) override {
EXPECT_EQ(kNumMessages, num_messages_received_);
std::move(callback).Run();
std::move(quit_closure_).Run();
}

void BindReceiver(
mojo::PendingAssociatedReceiver<IPC::mojom::SimpleTestDriver> receiver) {
DCHECK(!receiver_.is_bound());
receiver_.Bind(std::move(receiver));
}

int32_t next_expected_value_ = 0;
int num_messages_received_ = 0;
base::OnceClosure quit_closure_;

mojo::AssociatedReceiver<IPC::mojom::SimpleTestDriver> receiver_{this};
};

const int ListenerWithSimpleAssociatedInterface::kNumMessages = 1000;

class ListenerSendingAssociatedMessages : public IPC::Listener {
public:
explicit ListenerSendingAssociatedMessages(base::OnceClosure quit_closure)
: quit_closure_(std::move(quit_closure)) {}

bool OnMessageReceived(const IPC::Message& message) override { return true; }

void OnChannelConnected(int32_t peer_pid) override {
DCHECK(channel_);
channel_->GetAssociatedInterfaceSupport()->GetRemoteAssociatedInterface(
driver_.BindNewEndpointAndPassReceiver());

// Send a bunch of interleaved messages, alternating between the associated
// interface and a legacy IPC::Message.
for (int i = 0; i < ListenerWithSimpleAssociatedInterface::kNumMessages;
++i) {
driver_->ExpectValue(i);
SendValue(channel_, i);
}
driver_->RequestQuit(base::BindOnce(
&ListenerSendingAssociatedMessages::OnQuitAck, base::Unretained(this)));
}

void set_channel(IPC::Channel* channel) { channel_ = channel; }

private:
void OnQuitAck() { std::move(quit_closure_).Run(); }

raw_ptr<IPC::Channel> channel_ = nullptr;
mojo::AssociatedRemote<IPC::mojom::SimpleTestDriver> driver_;
base::OnceClosure quit_closure_;
};

TEST_F(IPCChannelMojoTest, SimpleAssociatedInterface) {
Init("SimpleAssociatedInterfaceClient");

base::RunLoop run_loop;
ListenerWithSimpleAssociatedInterface listener(run_loop.QuitClosure());
CreateChannel(&listener);
ASSERT_TRUE(ConnectChannel());

listener.RegisterInterfaceFactory(channel());

run_loop.Run();
channel()->Close();

EXPECT_TRUE(WaitForClientShutdown());
DestroyChannel();
}

DEFINE_IPC_CHANNEL_MOJO_TEST_CLIENT(SimpleAssociatedInterfaceClient) {
base::RunLoop run_loop;
ListenerSendingAssociatedMessages listener(run_loop.QuitClosure());
Connect(&listener);
listener.set_channel(channel());

run_loop.Run();

Close();
}

class ChannelProxyRunner {
public:
ChannelProxyRunner(mojo::ScopedMessagePipeHandle handle,
Expand Down

0 comments on commit 2cf8572

Please sign in to comment.