Commit 71960eab authored by Tal Pressman's avatar Tal Pressman Committed by Commit Bot

[MBI] Always associate AgentSchedulingGroupHost with AgentSchedulingGroup.

Regardless of whether we are in "MBI mode" or "legacy mode", the
AgentSchedulingGroup and AgentSchedulingGroupHost interfaces should be
associated with one another. This is because we want to preserve the
current behavior, where all channel-associated interfaces are all bound to
the same pipe and not separated by the direction (browser->renderer or
renderer->browser).

This CL moves the binding of the ASGH interface to the method that was
used to bind the RouteProvider interfaces (and renames it
appropriately).

Bug: 1111231
Change-Id: I4304e00a65b44d11f5dad6fb51ec1bedde76ec26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2528813
Commit-Queue: Tal Pressman <talp@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarDominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827139}
parent ee9a8742
......@@ -52,48 +52,6 @@ class AgentGroupHostUserData : public base::SupportsUserData::Data {
} // namespace
// MaybeAssociatedReceiver:
AgentSchedulingGroupHost::MaybeAssociatedReceiver::MaybeAssociatedReceiver(
AgentSchedulingGroupHost& host,
bool should_associate) {
if (should_associate) {
receiver_or_monostate_
.emplace<AssociatedReceiver<mojom::AgentSchedulingGroupHost>>(&host);
receiver_ = &absl::get<AssociatedReceiver<mojom::AgentSchedulingGroupHost>>(
receiver_or_monostate_);
} else {
receiver_or_monostate_.emplace<Receiver<mojom::AgentSchedulingGroupHost>>(
&host);
receiver_ = &absl::get<Receiver<mojom::AgentSchedulingGroupHost>>(
receiver_or_monostate_);
}
}
AgentSchedulingGroupHost::MaybeAssociatedReceiver::~MaybeAssociatedReceiver() =
default;
PendingRemote<mojom::AgentSchedulingGroupHost>
AgentSchedulingGroupHost::MaybeAssociatedReceiver::BindNewPipeAndPassRemote() {
return absl::get<Receiver<mojom::AgentSchedulingGroupHost>*>(receiver_)
->BindNewPipeAndPassRemote();
}
PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
AgentSchedulingGroupHost::MaybeAssociatedReceiver::
BindNewEndpointAndPassRemote() {
return absl::get<AssociatedReceiver<mojom::AgentSchedulingGroupHost>*>(
receiver_)
->BindNewEndpointAndPassRemote();
}
void AgentSchedulingGroupHost::MaybeAssociatedReceiver::reset() {
absl::visit([](auto* r) { r->reset(); }, receiver_);
}
bool AgentSchedulingGroupHost::MaybeAssociatedReceiver::is_bound() {
return absl::visit([](auto* r) { return r->is_bound(); }, receiver_);
}
// MaybeAssociatedRemote:
AgentSchedulingGroupHost::MaybeAssociatedRemote::MaybeAssociatedRemote(
bool should_associate) {
......@@ -162,7 +120,7 @@ AgentSchedulingGroupHost::AgentSchedulingGroupHost(RenderProcessHost& process,
bool should_associate)
: process_(process),
should_associate_(should_associate),
receiver_(*this, should_associate),
receiver_(this),
mojo_remote_(should_associate) {
process_.AddObserver(this);
......@@ -370,15 +328,14 @@ void AgentSchedulingGroupHost::SetUpMojoIfNeeded() {
if (should_associate_) {
process_.GetRendererInterface()->CreateAssociatedAgentSchedulingGroup(
receiver_.BindNewEndpointAndPassRemote(),
mojo_remote_.BindNewEndpointAndPassReceiver());
} else {
process_.GetRendererInterface()->CreateAgentSchedulingGroup(
receiver_.BindNewPipeAndPassRemote(),
mojo_remote_.BindNewPipeAndPassReceiver());
}
mojo_remote_.get()->BindAssociatedRouteProvider(
mojo_remote_.get()->BindAssociatedInterfaces(
receiver_.BindNewEndpointAndPassRemote(),
route_provider_receiver_.BindNewEndpointAndPassRemote(),
remote_route_provider_.BindNewEndpointAndPassReceiver());
SetState(LifecycleState::kBound);
......
......@@ -106,47 +106,14 @@ class CONTENT_EXPORT AgentSchedulingGroupHost
friend StateTransitions<LifecycleState>;
friend std::ostream& operator<<(std::ostream& os, LifecycleState state);
// `MaybeAssociatedReceiver` and `MaybeAssociatedRemote` are temporary helper
// classes that allow us to switch between using associated and non-associated
// mojo interfaces. This behavior is controlled by the
// `kMbiDetachAgentSchedulingGroupFromChannel` feature flag.
// Associated interfaces are associated with the IPC channel (transitively,
// `MaybeAssociatedRemote` is a temporary helper class that allows us to
// switch between using an associated and non-associated remote. This behavior
// is controlled by the `kMbiDetachAgentSchedulingGroupFromChannel` feature
// flag. Associated remotes are associated with the IPC channel (transitively,
// via the `Renderer` interface), thus preserving cross-agent scheduling group
// message order. Non-associated interfaces are independent from each other
// and do not preserve message order between agent scheduling groups.
// TODO(crbug.com/1111231): Remove these once we can remove the flag.
class MaybeAssociatedReceiver {
public:
MaybeAssociatedReceiver(AgentSchedulingGroupHost& host,
bool should_associate);
~MaybeAssociatedReceiver();
mojo::PendingRemote<mojom::AgentSchedulingGroupHost>
BindNewPipeAndPassRemote() WARN_UNUSED_RESULT;
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
BindNewEndpointAndPassRemote() WARN_UNUSED_RESULT;
void reset();
bool is_bound();
private:
// This will hold the actual receiver pointed to by |receiver_|.
absl::variant<
// This is required to make the variant default constructible. After the
// ctor body finishes, the variant will never hold this alternative.
absl::monostate,
mojo::Receiver<mojom::AgentSchedulingGroupHost>,
mojo::AssociatedReceiver<mojom::AgentSchedulingGroupHost>>
receiver_or_monostate_;
// View of |receiver_or_monostate_| that "strips out" the `monostate`,
// allowing for easier handling of the underlying remote. Should be declared
// after |receiver_or_monostate_| so that it is destroyed first.
absl::variant<mojo::Receiver<mojom::AgentSchedulingGroupHost>*,
mojo::AssociatedReceiver<mojom::AgentSchedulingGroupHost>*>
receiver_;
};
// TODO(crbug.com/1111231): Remove this once we can remove the flag.
class MaybeAssociatedRemote {
public:
explicit MaybeAssociatedRemote(bool should_associate);
......@@ -209,7 +176,7 @@ class CONTENT_EXPORT AgentSchedulingGroupHost
// Implementation of `mojom::AgentSchedulingGroupHost`, used for responding to
// calls from the (renderer-side) `AgentSchedulingGroup`.
MaybeAssociatedReceiver receiver_;
mojo::AssociatedReceiver<mojom::AgentSchedulingGroupHost> receiver_;
// Remote stub of `mojom::AgentSchedulingGroup`, used for sending calls to the
// (renderer-side) `AgentSchedulingGroup`.
......
......@@ -221,20 +221,19 @@ interface AgentSchedulingGroupHost {
// AgentSchedulingGroupHost and the renderer process's AgentSchedulingGroup.
// Implemented by content::AgentSchedulingGroup (in the renderer process).
interface AgentSchedulingGroup {
// Tells the renderer to bind the receiver to a backing RouteProvider
// implementation, which in practice is itself. Also passes a remote to the
// RouteProvider interface owned by AgentSchedulingGroupHost so that we
// immediately have bidirectional RouteProvider communication between
// AgentSchedulingGroup <=> AgentSchedulingGroupHost. We have this as a
// method on this interface, as opposed to passing this remote/receiver pair
// over the method that creates the remote AgentSchedulingGroup. This is
// because we need the RouteProvider remote/receiver pair to be associated
// with the message pipe that the AgentSchedulingGroup is associated with,
// which may be different than the message pipe that we create the
// AgentSchedulingGroup over.
BindAssociatedRouteProvider(
pending_associated_remote<RouteProvider> remote,
pending_associated_receiver<RouteProvider> receiver);
// Tells the renderer to bind the AgentSchedulingGroup's associated
// interfaces. This includes the host remote (mojom::AgentSchedulingGroupHost)
// as well as RouteProvider remote/receiver pair.
// We have this as a method on this interface, as opposed to passing the
// pending interfaces over the method that creates the AgentSchedulingGroup.
// This is because we need these interfaces to be associated with the message
// pipe that the AgentSchedulingGroup is associated with, which may be
// different than the message pipe that we create the AgentSchedulingGroup
// over.
BindAssociatedInterfaces(
pending_associated_remote<AgentSchedulingGroupHost> remote_host,
pending_associated_remote<RouteProvider> remote_route_provider,
pending_associated_receiver<RouteProvider> route_provider_receiver);
// Tells the renderer to create a new view.
CreateView(CreateViewParams params);
......
......@@ -59,22 +59,19 @@ enum RenderProcessVisibleState {
// which need to retain FIFO with respect to legacy IPC messages.
interface Renderer {
// Tells the renderer to create a new AgentSchedulingGroup, that will
// communicate via the pending |agent_scheduling_group_host| and
// |agent_scheduling_group|. This will create "independent" interfaces that
// are not associated with the IPC channel, which will not guarantee any order
// across agent scheduling groups.
// listen to messages sent by the host via the pending
// |agent_scheduling_group|. This will create "independent" agent scheduling
// groups that are not associated with the IPC channel, which will not
// guarantee any order across agent scheduling groups.
CreateAgentSchedulingGroup(
pending_remote<AgentSchedulingGroupHost> agent_scheduling_group_host,
pending_receiver<AgentSchedulingGroup> agent_scheduling_group
);
// Tells the renderer to create a new AgentSchedulingGroup, that will
// communicate via the pending |agent_scheduling_group_host| and
// |agent_scheduling_group|. This will create channel-associated interfaces
// listen to messages sent by the host via the pending
// |agent_scheduling_group|. This will create a channel-associated interface
// that will preserve message ordering across agent scheduling groups.
CreateAssociatedAgentSchedulingGroup(
pending_associated_remote<AgentSchedulingGroupHost>
agent_scheduling_group_host,
pending_associated_receiver<AgentSchedulingGroup> agent_scheduling_group
);
......
......@@ -237,17 +237,7 @@ bool GetWindowsKeyCode(char ascii_character, int* key_code) {
std::unique_ptr<AgentSchedulingGroup> CreateAgentSchedulingGroup(
RenderThread& render_thread) {
// Fake mojos for the AgentSchedulingGroupHost interface.
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
agent_scheduling_group_host;
ignore_result(
agent_scheduling_group_host.InitWithNewEndpointAndPassReceiver());
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup>
agent_scheduling_group_receiver;
return std::make_unique<MockAgentSchedulingGroup>(
render_thread, std::move(agent_scheduling_group_host),
std::move(agent_scheduling_group_receiver));
return std::make_unique<MockAgentSchedulingGroup>(render_thread);
}
} // namespace
......
......@@ -58,33 +58,9 @@ AgentSchedulingGroup::MaybeAssociatedReceiver::MaybeAssociatedReceiver(
AgentSchedulingGroup::MaybeAssociatedReceiver::~MaybeAssociatedReceiver() =
default;
// MaybeAssociatedRemote:
AgentSchedulingGroup::MaybeAssociatedRemote::MaybeAssociatedRemote(
PendingRemote<mojom::AgentSchedulingGroupHost> host_remote,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: remote_(absl::in_place_type<Remote<mojom::AgentSchedulingGroupHost>>,
std::move(host_remote),
task_runner) {}
AgentSchedulingGroup::MaybeAssociatedRemote::MaybeAssociatedRemote(
PendingAssociatedRemote<mojom::AgentSchedulingGroupHost> host_remote,
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: remote_(absl::in_place_type<
AssociatedRemote<mojom::AgentSchedulingGroupHost>>,
std::move(host_remote),
task_runner) {}
AgentSchedulingGroup::MaybeAssociatedRemote::~MaybeAssociatedRemote() = default;
mojom::AgentSchedulingGroupHost*
AgentSchedulingGroup::MaybeAssociatedRemote::get() {
return absl::visit([](auto& r) { return r.get(); }, remote_);
}
// AgentSchedulingGroup:
AgentSchedulingGroup::AgentSchedulingGroup(
RenderThread& render_thread,
PendingRemote<mojom::AgentSchedulingGroupHost> host_remote,
PendingReceiver<mojom::AgentSchedulingGroup> receiver)
: agent_group_scheduler_(
blink::scheduler::WebThreadScheduler::MainThreadScheduler()
......@@ -92,9 +68,7 @@ AgentSchedulingGroup::AgentSchedulingGroup(
render_thread_(render_thread),
receiver_(*this,
std::move(receiver),
agent_group_scheduler_->DefaultTaskRunner()),
host_remote_(std::move(host_remote),
agent_group_scheduler_->DefaultTaskRunner()) {
agent_group_scheduler_->DefaultTaskRunner()) {
DCHECK(agent_group_scheduler_);
DCHECK(base::FeatureList::IsEnabled(
features::kMbiDetachAgentSchedulingGroupFromChannel));
......@@ -102,7 +76,6 @@ AgentSchedulingGroup::AgentSchedulingGroup(
AgentSchedulingGroup::AgentSchedulingGroup(
RenderThread& render_thread,
PendingAssociatedRemote<mojom::AgentSchedulingGroupHost> host_remote,
PendingAssociatedReceiver<mojom::AgentSchedulingGroup> receiver)
: agent_group_scheduler_(
blink::scheduler::WebThreadScheduler::MainThreadScheduler()
......@@ -110,9 +83,7 @@ AgentSchedulingGroup::AgentSchedulingGroup(
render_thread_(render_thread),
receiver_(*this,
std::move(receiver),
agent_group_scheduler_->DefaultTaskRunner()),
host_remote_(std::move(host_remote),
agent_group_scheduler_->DefaultTaskRunner()) {
agent_group_scheduler_->DefaultTaskRunner()) {
DCHECK(agent_group_scheduler_);
DCHECK(!base::FeatureList::IsEnabled(
features::kMbiDetachAgentSchedulingGroupFromChannel));
......@@ -203,12 +174,16 @@ void AgentSchedulingGroup::CreateFrameProxy(
parent_routing_id, replicated_state, frame_token, devtools_frame_token);
}
void AgentSchedulingGroup::BindAssociatedRouteProvider(
mojo::PendingAssociatedRemote<mojom::RouteProvider> remote,
mojo::PendingAssociatedReceiver<mojom::RouteProvider> receiver) {
remote_route_provider_.Bind(std::move(remote),
void AgentSchedulingGroup::BindAssociatedInterfaces(
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost> remote_host,
mojo::PendingAssociatedRemote<mojom::RouteProvider> remote_route_provider,
mojo::PendingAssociatedReceiver<mojom::RouteProvider>
route_provider_receiever) {
host_remote_.Bind(std::move(remote_host),
agent_group_scheduler_->DefaultTaskRunner());
remote_route_provider_.Bind(std::move(remote_route_provider),
agent_group_scheduler_->DefaultTaskRunner());
route_provider_receiver_.Bind(std::move(receiver),
route_provider_receiver_.Bind(std::move(route_provider_receiever),
agent_group_scheduler_->DefaultTaskRunner());
}
......
......@@ -39,12 +39,9 @@ class CONTENT_EXPORT AgentSchedulingGroup
public:
AgentSchedulingGroup(
RenderThread& render_thread,
mojo::PendingRemote<mojom::AgentSchedulingGroupHost> host_remote,
mojo::PendingReceiver<mojom::AgentSchedulingGroup> receiver);
AgentSchedulingGroup(
RenderThread& render_thread,
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
host_remote,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup> receiver);
~AgentSchedulingGroup() override;
......@@ -65,14 +62,14 @@ class CONTENT_EXPORT AgentSchedulingGroup
}
private:
// `MaybeAssociatedReceiver` and `MaybeAssociatedRemote` are temporary helper
// classes that allow us to switch between using associated and non-associated
// mojo interfaces. This behavior is controlled by the
// `kMbiDetachAgentSchedulingGroupFromChannel` feature flag.
// Associated interfaces are associated with the IPC channel (transitively,
// via the `Renderer` interface), thus preserving cross-agent scheduling group
// message order. Non-associated interfaces are independent from each other
// and do not preserve message order between agent scheduling groups.
// `MaybeAssociatedReceiver` is a temporary helper class that allows us to
// switch between using an associated and non-associated receiver. This
// behavior is controlled by the `kMbiDetachAgentSchedulingGroupFromChannel`
// feature flag. Associated receivers are associated with the IPC channel
// (transitively, via the `Renderer` interface), thus preserving cross-agent
// scheduling group message order. Non-associated receivers are independent
// from each other and do not preserve message order between agent scheduling
// groups.
// TODO(crbug.com/1111231): Remove these once we can remove the flag.
class MaybeAssociatedReceiver {
public:
......@@ -92,24 +89,6 @@ class CONTENT_EXPORT AgentSchedulingGroup
receiver_;
};
class MaybeAssociatedRemote {
public:
explicit MaybeAssociatedRemote(
mojo::PendingRemote<mojom::AgentSchedulingGroupHost> host_remote,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
explicit MaybeAssociatedRemote(
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
host_remote,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
~MaybeAssociatedRemote();
mojom::AgentSchedulingGroupHost* get();
private:
absl::variant<mojo::Remote<mojom::AgentSchedulingGroupHost>,
mojo::AssociatedRemote<mojom::AgentSchedulingGroupHost>>
remote_;
};
// mojom::AgentSchedulingGroup:
void CreateView(mojom::CreateViewParamsPtr params) override;
void DestroyView(int32_t view_id, DestroyViewCallback callback) override;
......@@ -122,9 +101,12 @@ class CONTENT_EXPORT AgentSchedulingGroup
const FrameReplicationState& replicated_state,
const base::UnguessableToken& frame_token,
const base::UnguessableToken& devtools_frame_token) override;
void BindAssociatedRouteProvider(
mojo::PendingAssociatedRemote<mojom::RouteProvider> remote,
mojo::PendingAssociatedReceiver<mojom::RouteProvider> receiever) override;
void BindAssociatedInterfaces(
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
remote_host,
mojo::PendingAssociatedRemote<mojom::RouteProvider> remote_route_provider,
mojo::PendingAssociatedReceiver<mojom::RouteProvider>
route_provider_receiever) override;
// mojom::RouteProvider
void GetRoute(
......@@ -155,7 +137,7 @@ class CONTENT_EXPORT AgentSchedulingGroup
// Remote stub of mojom::AgentSchedulingGroupHost, used for sending calls to
// the (browser-side) AgentSchedulingGroupHost.
MaybeAssociatedRemote host_remote_;
mojo::AssociatedRemote<mojom::AgentSchedulingGroupHost> host_remote_;
// The |mojom::RouteProvider| mojo pair to setup
// |blink::AssociatedInterfaceProvider| routes between us and the browser-side
......
......@@ -9,13 +9,10 @@
namespace content {
MockAgentSchedulingGroup::MockAgentSchedulingGroup(
RenderThread& render_thread,
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost> host_remote,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup> receiver)
: AgentSchedulingGroup(render_thread,
std::move(host_remote),
std::move(receiver)) {}
MockAgentSchedulingGroup::MockAgentSchedulingGroup(RenderThread& render_thread)
: AgentSchedulingGroup(
render_thread,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup>()) {}
mojom::RouteProvider* MockAgentSchedulingGroup::GetRemoteRouteProvider() {
DCHECK(!RenderThreadImpl::current());
......
......@@ -22,13 +22,7 @@ class RenderThread;
// in the browser process.
class MockAgentSchedulingGroup : public AgentSchedulingGroup {
public:
// `mojo_disconnect_handler` is an optional callback that will be called with
// `this` when `receiver` is disconnected.
MockAgentSchedulingGroup(
RenderThread& render_thread,
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
host_remote,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup> receiver);
explicit MockAgentSchedulingGroup(RenderThread& render_thread);
mojom::RouteProvider* GetRemoteRouteProvider() override;
};
......
......@@ -1642,22 +1642,16 @@ gpu::GpuChannelHost* RenderThreadImpl::GetGpuChannel() {
}
void RenderThreadImpl::CreateAgentSchedulingGroup(
mojo::PendingRemote<mojom::AgentSchedulingGroupHost>
agent_scheduling_group_host,
mojo::PendingReceiver<mojom::AgentSchedulingGroup> agent_scheduling_group) {
agent_scheduling_groups_.emplace(std::make_unique<AgentSchedulingGroup>(
*this, std::move(agent_scheduling_group_host),
std::move(agent_scheduling_group)));
*this, std::move(agent_scheduling_group)));
}
void RenderThreadImpl::CreateAssociatedAgentSchedulingGroup(
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
agent_scheduling_group_host,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup>
agent_scheduling_group) {
agent_scheduling_groups_.emplace(std::make_unique<AgentSchedulingGroup>(
*this, std::move(agent_scheduling_group_host),
std::move(agent_scheduling_group)));
*this, std::move(agent_scheduling_group)));
}
void RenderThreadImpl::OnNetworkConnectionChanged(
......
......@@ -439,13 +439,9 @@ class CONTENT_EXPORT RenderThreadImpl
// mojom::Renderer:
void CreateAgentSchedulingGroup(
mojo::PendingRemote<mojom::AgentSchedulingGroupHost>
agent_scheduling_group_host,
mojo::PendingReceiver<mojom::AgentSchedulingGroup> agent_scheduling_group)
override;
void CreateAssociatedAgentSchedulingGroup(
mojo::PendingAssociatedRemote<mojom::AgentSchedulingGroupHost>
agent_scheduling_group_host,
mojo::PendingAssociatedReceiver<mojom::AgentSchedulingGroup>
agent_scheduling_group) override;
void OnNetworkConnectionChanged(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment