Skip to content

Commit

Permalink
Convert Callback -> {Once,Repeating}Callback in mojo::core
Browse files Browse the repository at this point in the history 
Bug: 1007813
Change-Id: I0fbb86cd6d2fae4e9a8d2ead2079a706c8e57ae6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1898157
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713263}
  • Loading branch information
@akmistry
akmistry authored and Commit Bot committed Nov 7, 2019
1 parent 7fa6f1f commit b6558d9
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 82 deletions.
9 changes: 4 additions & 5 deletions mojo/core/core.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,8 @@ scoped_refptr<Dispatcher> Core::GetAndRemoveDispatcher(MojoHandle handle) {
return dispatcher;
}

void Core::SetDefaultProcessErrorCallback(
const ProcessErrorCallback& callback) {
default_process_error_callback_ = callback;
void Core::SetDefaultProcessErrorCallback(ProcessErrorCallback callback) {
default_process_error_callback_ = std::move(callback);
}

MojoHandle Core::CreatePartialMessagePipe(ports::PortRef* peer) {
Expand Down Expand Up @@ -252,8 +251,8 @@ void Core::ReleaseDispatchersForTransit(
handles_->CancelTransit(dispatchers);
}

void Core::RequestShutdown(const base::Closure& callback) {
GetNodeController()->RequestShutdown(callback);
void Core::RequestShutdown(base::OnceClosure callback) {
GetNodeController()->RequestShutdown(std::move(callback));
}

MojoHandle Core::ExtractMessagePipeFromInvitation(const std::string& name) {
Expand Down
4 changes: 2 additions & 2 deletions mojo/core/core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Core {
scoped_refptr<Dispatcher> GetDispatcher(MojoHandle handle);
scoped_refptr<Dispatcher> GetAndRemoveDispatcher(MojoHandle handle);

void SetDefaultProcessErrorCallback(const ProcessErrorCallback& callback);
void SetDefaultProcessErrorCallback(ProcessErrorCallback callback);

// Creates a message pipe endpoint with an unbound peer port returned in
// |*peer|. Useful for setting up cross-process bootstrap message pipes. The
Expand Down Expand Up @@ -136,7 +136,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Core {
// loop, and the calling thread must continue running a MessageLoop at least
// until the callback is called. If there is no running loop, the |callback|
// may be called from any thread. Beware!
void RequestShutdown(const base::Closure& callback);
void RequestShutdown(base::OnceClosure callback);

// ---------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion mojo/core/data_pipe_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1992,7 +1992,7 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(DataPipeStatusChangeInTransitClient,
{
base::RunLoop run_loop;
int count = 0;
auto callback = base::Bind(
auto callback = base::BindRepeating(
[](base::RunLoop* loop, int* count, MojoResult result) {
EXPECT_EQ(MOJO_RESULT_OK, result);
if (++*count == 2)
Expand Down
4 changes: 2 additions & 2 deletions mojo/core/embedder/embedder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ void Init() {
Init(Configuration());
}

void SetDefaultProcessErrorCallback(const ProcessErrorCallback& callback) {
Core::Get()->SetDefaultProcessErrorCallback(callback);
void SetDefaultProcessErrorCallback(ProcessErrorCallback callback) {
Core::Get()->SetDefaultProcessErrorCallback(std::move(callback));
}

scoped_refptr<base::TaskRunner> GetIOTaskRunner() {
Expand Down
5 changes: 3 additions & 2 deletions mojo/core/embedder/embedder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
namespace mojo {
namespace core {

using ProcessErrorCallback = base::Callback<void(const std::string& error)>;
using ProcessErrorCallback =
base::RepeatingCallback<void(const std::string& error)>;

// Basic configuration/initialization ------------------------------------------

Expand All @@ -38,7 +39,7 @@ COMPONENT_EXPORT(MOJO_CORE_EMBEDDER) void Init();
// Sets a default callback to invoke when an internal error is reported but
// cannot be associated with a specific child process. Calling this is optional.
COMPONENT_EXPORT(MOJO_CORE_EMBEDDER)
void SetDefaultProcessErrorCallback(const ProcessErrorCallback& callback);
void SetDefaultProcessErrorCallback(ProcessErrorCallback callback);

// Initialialization/shutdown for interprocess communication (IPC) -------------

Expand Down
8 changes: 4 additions & 4 deletions mojo/core/embedder/scoped_ipc_support.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ namespace core {

namespace {

void ShutdownIPCSupport(const base::Closure& callback) {
Core::Get()->RequestShutdown(callback);
void ShutdownIPCSupport(base::OnceClosure callback) {
Core::Get()->RequestShutdown(std::move(callback));
}

} // namespace
Expand All @@ -37,8 +37,8 @@ ScopedIPCSupport::~ScopedIPCSupport() {
base::WaitableEvent shutdown_event(
base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
ShutdownIPCSupport(base::Bind(&base::WaitableEvent::Signal,
base::Unretained(&shutdown_event)));
ShutdownIPCSupport(base::BindOnce(&base::WaitableEvent::Signal,
base::Unretained(&shutdown_event)));

base::ScopedAllowBaseSyncPrimitives allow_io;
shutdown_event.Wait();
Expand Down
45 changes: 23 additions & 22 deletions mojo/core/message_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ class TestMessageBase {
class NeverSerializedMessage : public TestMessageBase {
public:
NeverSerializedMessage(
const base::Closure& destruction_callback = base::Closure())
: destruction_callback_(destruction_callback) {}
base::OnceClosure destruction_callback = base::OnceClosure())
: destruction_callback_(std::move(destruction_callback)) {}
~NeverSerializedMessage() override {
if (destruction_callback_)
destruction_callback_.Run();
std::move(destruction_callback_).Run();
}

private:
Expand All @@ -115,20 +115,21 @@ class NeverSerializedMessage : public TestMessageBase {
void SerializeHandles(MojoHandle* handles) override { NOTREACHED(); }
void SerializePayload(void* buffer) override { NOTREACHED(); }

const base::Closure destruction_callback_;
base::OnceClosure destruction_callback_;

DISALLOW_COPY_AND_ASSIGN(NeverSerializedMessage);
};

class SimpleMessage : public TestMessageBase {
public:
SimpleMessage(const std::string& contents,
const base::Closure& destruction_callback = base::Closure())
: contents_(contents), destruction_callback_(destruction_callback) {}
base::OnceClosure destruction_callback = base::OnceClosure())
: contents_(contents),
destruction_callback_(std::move(destruction_callback)) {}

~SimpleMessage() override {
if (destruction_callback_)
destruction_callback_.Run();
std::move(destruction_callback_).Run();
}

void AddMessagePipe(mojo::ScopedMessagePipeHandle handle) {
Expand Down Expand Up @@ -156,7 +157,7 @@ class SimpleMessage : public TestMessageBase {
}

const std::string contents_;
const base::Closure destruction_callback_;
base::OnceClosure destruction_callback_;
std::vector<mojo::ScopedMessagePipeHandle> handles_;

DISALLOW_COPY_AND_ASSIGN(SimpleMessage);
Expand Down Expand Up @@ -212,8 +213,8 @@ TEST_F(MessageTest, SendLocalMessageWithContext) {
TEST_F(MessageTest, DestroyMessageWithContext) {
// Tests that |MojoDestroyMessage()| destroys any attached context.
bool was_deleted = false;
auto message = std::make_unique<NeverSerializedMessage>(
base::Bind([](bool* was_deleted) { *was_deleted = true; }, &was_deleted));
auto message = std::make_unique<NeverSerializedMessage>(base::BindOnce(
[](bool* was_deleted) { *was_deleted = true; }, &was_deleted));
MojoMessageHandle handle =
TestMessageBase::MakeMessageHandle(std::move(message));
EXPECT_FALSE(was_deleted);
Expand Down Expand Up @@ -371,8 +372,8 @@ TEST_F(MessageTest, DropUnreadLocalMessageWithContext) {
bool message_was_destroyed = false;
auto message = std::make_unique<SimpleMessage>(
kTestMessageWithContext1,
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));

mojo::MessagePipe pipe;
message->AddMessagePipe(std::move(pipe.handle0));
Expand Down Expand Up @@ -441,8 +442,8 @@ TEST_F(MessageTest, ReadMessageWithContextAsSerializedMessage) {
bool message_was_destroyed = false;
std::unique_ptr<TestMessageBase> message =
std::make_unique<NeverSerializedMessage>(
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));

MojoHandle a, b;
CreateMessagePipe(&a, &b);
Expand Down Expand Up @@ -493,8 +494,8 @@ TEST_F(MessageTest, ForceSerializeMessageWithContext) {
bool message_was_destroyed = false;
auto message = std::make_unique<SimpleMessage>(
kTestMessageWithContext1,
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
auto message_handle = TestMessageBase::MakeMessageHandle(std::move(message));
EXPECT_EQ(MOJO_RESULT_OK, MojoSerializeMessage(message_handle, nullptr));
EXPECT_TRUE(message_was_destroyed);
Expand All @@ -505,8 +506,8 @@ TEST_F(MessageTest, ForceSerializeMessageWithContext) {
message_was_destroyed = false;
message = std::make_unique<SimpleMessage>(
kTestMessageWithContext1,
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
MessagePipe pipe1;
message->AddMessagePipe(std::move(pipe1.handle0));
message_handle = TestMessageBase::MakeMessageHandle(std::move(message));
Expand All @@ -520,8 +521,8 @@ TEST_F(MessageTest, ForceSerializeMessageWithContext) {
message_was_destroyed = false;
message = std::make_unique<SimpleMessage>(
kTestMessageWithContext1,
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
MessagePipe pipe2;
message->AddMessagePipe(std::move(pipe2.handle0));
message_handle = TestMessageBase::MakeMessageHandle(std::move(message));
Expand Down Expand Up @@ -557,8 +558,8 @@ TEST_F(MessageTest, DoubleSerialize) {
bool message_was_destroyed = false;
auto message = std::make_unique<SimpleMessage>(
kTestMessageWithContext1,
base::Bind([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
base::BindOnce([](bool* was_destroyed) { *was_destroyed = true; },
&message_was_destroyed));
auto message_handle = TestMessageBase::MakeMessageHandle(std::move(message));

// Ensure we can safely call |MojoSerializeMessage()| twice on the same
Expand Down
2 changes: 1 addition & 1 deletion mojo/core/multiprocess_message_pipe_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(MessagePipeStatusChangeInTransitClient,
SimpleWatcher watcher(FROM_HERE, SimpleWatcher::ArmingPolicy::AUTOMATIC,
base::SequencedTaskRunnerHandle::Get());
watcher.Watch(Handle(handles[1]), MOJO_HANDLE_SIGNAL_PEER_CLOSED,
base::Bind(
base::BindRepeating(
[](base::RunLoop* loop, MojoResult result) {
EXPECT_EQ(MOJO_RESULT_OK, result);
loop->Quit();
Expand Down
29 changes: 14 additions & 15 deletions mojo/core/node_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,19 @@ class ThreadDestructionObserver
: public base::MessageLoopCurrent::DestructionObserver {
public:
static void Create(scoped_refptr<base::TaskRunner> task_runner,
const base::Closure& callback) {
base::OnceClosure callback) {
if (task_runner->RunsTasksInCurrentSequence()) {
// Owns itself.
new ThreadDestructionObserver(callback);
new ThreadDestructionObserver(std::move(callback));
} else {
task_runner->PostTask(FROM_HERE,
base::BindOnce(&Create, task_runner, callback));
task_runner->PostTask(
FROM_HERE, base::BindOnce(&Create, task_runner, std::move(callback)));
}
}

private:
explicit ThreadDestructionObserver(const base::Closure& callback)
: callback_(callback) {
explicit ThreadDestructionObserver(base::OnceClosure callback)
: callback_(std::move(callback)) {
base::MessageLoopCurrent::Get()->AddDestructionObserver(this);
}

Expand All @@ -133,11 +133,11 @@ class ThreadDestructionObserver

// base::MessageLoopCurrent::DestructionObserver:
void WillDestroyCurrentMessageLoop() override {
callback_.Run();
std::move(callback_).Run();
delete this;
}

const base::Closure callback_;
base::OnceClosure callback_;

DISALLOW_COPY_AND_ASSIGN(ThreadDestructionObserver);
};
Expand All @@ -158,7 +158,7 @@ void NodeController::SetIOTaskRunner(
io_task_runner_ = task_runner;
ThreadDestructionObserver::Create(
io_task_runner_,
base::Bind(&NodeController::DropAllPeers, base::Unretained(this)));
base::BindOnce(&NodeController::DropAllPeers, base::Unretained(this)));
}

void NodeController::SendBrokerClientInvitation(
Expand Down Expand Up @@ -312,10 +312,10 @@ base::WritableSharedMemoryRegion NodeController::CreateSharedBuffer(
return base::WritableSharedMemoryRegion::Create(num_bytes);
}

void NodeController::RequestShutdown(const base::Closure& callback) {
void NodeController::RequestShutdown(base::OnceClosure callback) {
{
base::AutoLock lock(shutdown_lock_);
shutdown_callback_ = callback;
shutdown_callback_ = std::move(callback);
shutdown_callback_flag_.Set(true);
}

Expand Down Expand Up @@ -1272,7 +1272,7 @@ void NodeController::AttemptShutdownIfRequested() {
if (!shutdown_callback_flag_)
return;

base::Closure callback;
base::OnceClosure callback;
{
base::AutoLock lock(shutdown_lock_);
if (shutdown_callback_.is_null())
Expand All @@ -1283,14 +1283,13 @@ void NodeController::AttemptShutdownIfRequested() {
return;
}

callback = shutdown_callback_;
shutdown_callback_.Reset();
callback = std::move(shutdown_callback_);
shutdown_callback_flag_.Set(false);
}

DCHECK(!callback.is_null());

callback.Run();
std::move(callback).Run();
}

void NodeController::ForceDisconnectProcessForTestingOnIOThread(
Expand Down
4 changes: 2 additions & 2 deletions mojo/core/node_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class MOJO_SYSTEM_IMPL_EXPORT NodeController : public ports::NodeDelegate,
// interface after requesting shutdown, you do so at your own risk and there
// is NO guarantee that new messages will be sent or ports will complete
// transfer.
void RequestShutdown(const base::Closure& callback);
void RequestShutdown(base::OnceClosure callback);

// Notifies the NodeController that we received a bad message from the given
// node.
Expand Down Expand Up @@ -309,7 +309,7 @@ class MOJO_SYSTEM_IMPL_EXPORT NodeController : public ports::NodeDelegate,
// Set by RequestShutdown(). If this is non-null, the controller will
// begin polling the Node to see if clean shutdown is possible any time the
// Node's state is modified by the controller.
base::Closure shutdown_callback_;
base::OnceClosure shutdown_callback_;
// Flag to fast-path checking |shutdown_callback_|.
AtomicFlag shutdown_callback_flag_;

Expand Down
9 changes: 4 additions & 5 deletions mojo/core/test/mojo_test_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class MojoTestBase : public testing::Test {
~MojoTestBase() override;

using LaunchType = MultiprocessTestHelper::LaunchType;
using HandlerCallback = base::Callback<void(ScopedMessagePipeHandle)>;

class ClientController {
public:
Expand Down Expand Up @@ -197,8 +196,8 @@ class MojoTestBase : public testing::Test {
::mojo::core::test::MultiprocessTestHelper::ChildSetup) { \
client_name##_MainFixture test; \
return ::mojo::core::test::MultiprocessTestHelper::RunClientMain( \
base::Bind(&client_name##_MainFixture::Main, \
base::Unretained(&test))); \
base::BindOnce(&client_name##_MainFixture::Main, \
base::Unretained(&test))); \
} \
int client_name##_MainFixture::Main(MojoHandle pipe_name)

Expand All @@ -216,8 +215,8 @@ class MojoTestBase : public testing::Test {
::mojo::core::test::MultiprocessTestHelper::ChildSetup) { \
client_name##_MainFixture test; \
return ::mojo::core::test::MultiprocessTestHelper::RunClientTestMain( \
base::Bind(&client_name##_MainFixture::Main, \
base::Unretained(&test))); \
base::BindOnce(&client_name##_MainFixture::Main, \
base::Unretained(&test))); \
} \
void client_name##_MainFixture::Main(MojoHandle pipe_name)
#else // !defined(OS_IOS)
Expand Down
Loading

0 comments on commit b6558d9

Please sign in to comment.