Skip to content

GitLab

T
tangled
Commit 7e078eba authored by Keishi Hattori's avatar Keishi Hattori Committed by Commit Bot
Browse files
Options

Remove pre finalizer from HeapMojoRemote etc 

Removes pre finalizer from HeapMojoRemote. HeapMojoUniqueReceiverSet, and HeapMojoAssociatedRemote.

It turns out they weren't necessary as we do not store raw pointers to GCed objects inside mojo::Remote, UniqueReceiverSet, or AssociatedRemote.

Change-Id: Ie44a9ef1ab9398e58dd972993663470ac0581881
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2206059Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769982}
parent a95323a3
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 180 deletions
third_party/blink/renderer/platform/mojo/heap_mojo_associated_remote.h
View file @ 7e078eba
......@@ -84,7 +84,6 @@ class HeapMojoAssociatedRemote {
// Garbage collected wrapper class to add a prefinalizer.
class Wrapper final : public GarbageCollected<Wrapper>,
public ContextLifecycleObserver {
USING_PRE_FINALIZER(Wrapper, Dispose);
USING_GARBAGE_COLLECTED_MIXIN(Wrapper);
public:
......@@ -101,8 +100,6 @@ class HeapMojoAssociatedRemote {
ContextLifecycleObserver::Trace(visitor);
}
void Dispose() { associated_remote_.reset(); }
mojo::AssociatedRemote<Interface>& associated_remote() {
return associated_remote_;
}
......
third_party/blink/renderer/platform/mojo/heap_mojo_associated_remote_test.cc
View file @ 7e078eba
......@@ -82,39 +82,6 @@ class AssociatedRemoteOwner
HeapMojoAssociatedRemote<sample::blink::Service, Mode> associated_remote_;
};
template <HeapMojoWrapperMode Mode>
class HeapMojoAssociatedRemoteGCBaseTest : public TestSupportingGC {
public:
base::RunLoop& run_loop() { return run_loop_; }
bool& disconnected() { return disconnected_; }
void ClearOwner() { owner_ = nullptr; }
protected:
void SetUp() override {
CHECK(!disconnected_);
context_ = MakeGarbageCollected<MockContext>();
owner_ = MakeGarbageCollected<AssociatedRemoteOwner<Mode>>(context_);
scoped_refptr<base::NullTaskRunner> null_task_runner =
base::MakeRefCounted<base::NullTaskRunner>();
impl_.associated_receiver().Bind(
owner_->associated_remote().BindNewEndpointAndPassReceiver(
null_task_runner));
impl_.associated_receiver().set_disconnect_handler(WTF::Bind(
[](HeapMojoAssociatedRemoteGCBaseTest* associated_remote_test) {
associated_remote_test->run_loop().Quit();
associated_remote_test->disconnected() = true;
},
WTF::Unretained(this)));
}
ServiceImpl impl_;
Persistent<MockContext> context_;
Persistent<AssociatedRemoteOwner<Mode>> owner_;
base::RunLoop run_loop_;
bool disconnected_ = false;
};
template <HeapMojoWrapperMode Mode>
class HeapMojoAssociatedRemoteDestroyContextBaseTest : public TestSupportingGC {
protected:
......@@ -190,12 +157,6 @@ class HeapMojoAssociatedRemoteMoveBaseTest : public TestSupportingGC {
} // namespace
class HeapMojoAssociatedRemoteGCWithContextObserverTest
: public HeapMojoAssociatedRemoteGCBaseTest<
HeapMojoWrapperMode::kWithContextObserver> {};
class HeapMojoAssociatedRemoteGCWithoutContextObserverTest
: public HeapMojoAssociatedRemoteGCBaseTest<
HeapMojoWrapperMode::kWithoutContextObserver> {};
class HeapMojoAssociatedRemoteDestroyContextWithContextObserverTest
: public HeapMojoAssociatedRemoteDestroyContextBaseTest<
HeapMojoWrapperMode::kWithContextObserver> {};
......@@ -216,28 +177,6 @@ class HeapMojoAssociatedRemoteMoveWithoutContextObserverTest
: public HeapMojoAssociatedRemoteMoveBaseTest<
HeapMojoWrapperMode::kWithoutContextObserver> {};
// Make HeapAssociatedRemote with context observer garbage collected and check
// that the connection is disconnected right after the marking phase.
TEST_F(HeapMojoAssociatedRemoteGCWithContextObserverTest, ResetsOnGC) {
ClearOwner();
EXPECT_FALSE(disconnected());
PreciselyCollectGarbage();
run_loop().Run();
EXPECT_TRUE(disconnected());
CompleteSweepingIfNeeded();
}
// Make HeapAssociatedRemote without context observer garbage collected and
// check that the connection is disconnected right after the marking phase.
TEST_F(HeapMojoAssociatedRemoteGCWithoutContextObserverTest, ResetsOnGC) {
ClearOwner();
EXPECT_FALSE(disconnected());
PreciselyCollectGarbage();
run_loop().Run();
EXPECT_TRUE(disconnected());
CompleteSweepingIfNeeded();
}
// Destroy the context with context observer and check that the connection is
// disconnected.
TEST_F(HeapMojoAssociatedRemoteDestroyContextWithContextObserverTest,
......
third_party/blink/renderer/platform/mojo/heap_mojo_remote.h
View file @ 7e078eba
......@@ -18,8 +18,8 @@ namespace blink {
// garbage-collected object. Blink is expected to use HeapMojoRemote by
// default. HeapMojoRemote must be associated with context.
// HeapMojoRemote's constructor takes context as a mandatory parameter.
// HeapMojoRemote resets the mojo connection when 1) the owner object is
// garbage-collected and 2) the associated ExecutionContext is detached.
// HeapMojoRemote resets the mojo connection when the associated
// ExecutionContext is detached.
// TODO(crbug.com/1058076) HeapMojoWrapperMode should be removed once we ensure
// that the interface is not used after ContextDestroyed().
......@@ -73,10 +73,9 @@ class HeapMojoRemote {
void Trace(Visitor* visitor) const { visitor->Trace(wrapper_); }
private:
// Garbage collected wrapper class to add a prefinalizer.
// Garbage collected wrapper class to add ContextLifecycleObserver.
class Wrapper final : public GarbageCollected<Wrapper>,
public ContextLifecycleObserver {
USING_PRE_FINALIZER(Wrapper, Dispose);
USING_GARBAGE_COLLECTED_MIXIN(Wrapper);
public:
......@@ -92,8 +91,6 @@ class HeapMojoRemote {
ContextLifecycleObserver::Trace(visitor);
}
void Dispose() { remote_.reset(); }
mojo::Remote<Interface>& remote() { return remote_; }
// ContextLifecycleObserver methods
......
third_party/blink/renderer/platform/mojo/heap_mojo_remote_test.cc
View file @ 7e078eba
......@@ -79,38 +79,6 @@ class RemoteOwner : public GarbageCollected<RemoteOwner<Mode>> {
HeapMojoRemote<sample::blink::Service, Mode> remote_;
};
template <HeapMojoWrapperMode Mode>
class HeapMojoRemoteGCBaseTest : public TestSupportingGC {
public:
base::RunLoop& run_loop() { return run_loop_; }
bool& disconnected() { return disconnected_; }
void ClearOwner() { owner_ = nullptr; }
protected:
void SetUp() override {
CHECK(!disconnected_);
context_ = MakeGarbageCollected<MockContext>();
owner_ = MakeGarbageCollected<RemoteOwner<Mode>>(context_);
scoped_refptr<base::NullTaskRunner> null_task_runner =
base::MakeRefCounted<base::NullTaskRunner>();
impl_.receiver().Bind(
owner_->remote().BindNewPipeAndPassReceiver(null_task_runner));
impl_.receiver().set_disconnect_handler(WTF::Bind(
[](HeapMojoRemoteGCBaseTest* remote_test) {
remote_test->run_loop().Quit();
remote_test->disconnected() = true;
},
WTF::Unretained(this)));
}
ServiceImpl impl_;
Persistent<MockContext> context_;
Persistent<RemoteOwner<Mode>> owner_;
base::RunLoop run_loop_;
bool disconnected_ = false;
};
template <HeapMojoWrapperMode Mode>
class HeapMojoRemoteDestroyContextBaseTest : public TestSupportingGC {
protected:
......@@ -180,12 +148,6 @@ class HeapMojoRemoteMoveBaseTest : public TestSupportingGC {
} // namespace
class HeapMojoRemoteGCWithContextObserverTest
: public HeapMojoRemoteGCBaseTest<
HeapMojoWrapperMode::kWithContextObserver> {};
class HeapMojoRemoteGCWithoutContextObserverTest
: public HeapMojoRemoteGCBaseTest<
HeapMojoWrapperMode::kWithoutContextObserver> {};
class HeapMojoRemoteDestroyContextWithContextObserverTest
: public HeapMojoRemoteDestroyContextBaseTest<
HeapMojoWrapperMode::kWithContextObserver> {};
......@@ -205,28 +167,6 @@ class HeapMojoRemoteMoveWithoutContextObserverTest
: public HeapMojoRemoteMoveBaseTest<
HeapMojoWrapperMode::kWithoutContextObserver> {};
// Make HeapMojoRemote with context observer garbage collected and check that
// the connection is disconnected right after the marking phase.
TEST_F(HeapMojoRemoteGCWithContextObserverTest, ResetsOnGC) {
ClearOwner();
EXPECT_FALSE(disconnected());
PreciselyCollectGarbage();
run_loop().Run();
EXPECT_TRUE(disconnected());
CompleteSweepingIfNeeded();
}
// Make HeapMojoRemote without context observer garbage collected and check that
// the connection is disconnected right after the marking phase.
TEST_F(HeapMojoRemoteGCWithoutContextObserverTest, ResetsOnGC) {
ClearOwner();
EXPECT_FALSE(disconnected());
PreciselyCollectGarbage();
run_loop().Run();
EXPECT_TRUE(disconnected());
CompleteSweepingIfNeeded();
}
// Destroy the context with context observer and check that the connection is
// disconnected.
TEST_F(HeapMojoRemoteDestroyContextWithContextObserverTest,
......
third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set.h
View file @ 7e078eba
......@@ -18,8 +18,7 @@ namespace blink {
// HeapMojoUniqueReceiverSet by default. HeapMojoUniqueReceiverSet must be
// associated with context. HeapMojoUniqueReceiverSet's constructor takes
// context as a mandatory parameter. HeapMojoUniqueReceiverSet resets the mojo
// connection when 1) the owner object is garbage-collected or 2) the associated
// ExecutionContext is detached.
// connection when the associated ExecutionContext is detached.
template <typename Interface,
typename Deleter = std::default_delete<Interface>,
HeapMojoWrapperMode Mode = HeapMojoWrapperMode::kWithContextObserver>
......@@ -60,10 +59,9 @@ class HeapMojoUniqueReceiverSet {
void Trace(Visitor* visitor) const { visitor->Trace(wrapper_); }
private:
// Garbage collected wrapper class to add a prefinalizer.
// Garbage collected wrapper class to add ContextLifecycleObserver.
class Wrapper final : public GarbageCollected<Wrapper>,
public ContextLifecycleObserver {
USING_PRE_FINALIZER(Wrapper, Dispose);
USING_GARBAGE_COLLECTED_MIXIN(Wrapper);
public:
......@@ -75,8 +73,6 @@ class HeapMojoUniqueReceiverSet {
ContextLifecycleObserver::Trace(visitor);
}
void Dispose() { receiver_set_.Clear(); }
mojo::UniqueReceiverSet<Interface, void, Deleter>& receiver_set() {
return receiver_set_;
}
......
third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set_test.cc
View file @ 7e078eba
......@@ -125,50 +125,6 @@ class MockService : public sample::blink::Service {
} // namespace
// GC the HeapMojoUniqueReceiverSet with context observer and verify that the
// receiver is no longer part of the set, and that the service was deleted.
TEST_F(HeapMojoUniqueReceiverSetWithContextObserverTest, ResetsOnGC) {
auto& receiver_set = owner()->receiver_set();
auto service = std::make_unique<
MockService<HeapMojoUniqueReceiverSetWithContextObserverTest>>(this);
auto receiver = mojo::PendingReceiver<sample::blink::Service>(
mojo::MessagePipe().handle0);
mojo::ReceiverId rid =
receiver_set.Add(std::move(service), std::move(receiver), task_runner());
EXPECT_TRUE(receiver_set.HasReceiver(rid));
EXPECT_FALSE(service_deleted_);
ClearOwner();
PreciselyCollectGarbage();
EXPECT_TRUE(service_deleted_);
CompleteSweepingIfNeeded();
}
// GC the HeapMojoUniqueReceiverSet without context observer and verify that the
// receiver is no longer part of the set, and that the service was deleted.
TEST_F(HeapMojoUniqueReceiverSetWithoutContextObserverTest, ResetsOnGC) {
auto& receiver_set = owner()->receiver_set();
auto service = std::make_unique<
MockService<HeapMojoUniqueReceiverSetWithoutContextObserverTest>>(this);
auto receiver = mojo::PendingReceiver<sample::blink::Service>(
mojo::MessagePipe().handle0);
mojo::ReceiverId rid =
receiver_set.Add(std::move(service), std::move(receiver), task_runner());
EXPECT_TRUE(receiver_set.HasReceiver(rid));
EXPECT_FALSE(service_deleted_);
ClearOwner();
PreciselyCollectGarbage();
EXPECT_TRUE(service_deleted_);
CompleteSweepingIfNeeded();
}
// Destroy the context with context observer and verify that the receiver is no
// longer part of the set, and that the service was deleted.
TEST_F(HeapMojoUniqueReceiverSetWithContextObserverTest,
......
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