Migrate SharedWorkerClientHolder to use GC mojo wrappers.

1. Introduce HeapMojoUniqueReceiverSet and unit test (based on
   HeapMojoReceiverSet).
2. Change usages of mojo::{Remote, UniqueReceiverSet} to
   HeapMojo{Remote, UniqueReceiverSet}.

Bug: 1049056
Change-Id: I821aebfb6bd48082543dce0e2d224ebc6e31bb5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079721
Commit-Queue: Tal Pressman <talp@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748549}

third_party/blink/renderer/core/workers/shared_worker_client_holder.cc

@@ -72,7 +72,8 @@ SharedWorkerClientHolder* SharedWorkerClientHolder::From(Document& document) {
 }
 
 SharedWorkerClientHolder::SharedWorkerClientHolder(Document& document)
-    : ExecutionContextLifecycleObserver(&document),
+    : connector_(document.ToExecutionContext()),
+      client_receivers_(document.ToExecutionContext()),
       task_runner_(document.GetTaskRunner(blink::TaskType::kDOMManipulation)) {
   DCHECK(IsMainThread());
   document.GetBrowserInterfaceBroker().GetInterface(
@@ -138,16 +139,10 @@ void SharedWorkerClientHolder::Connect(
           blob_url_token.PassPipe(), mojom::blink::BlobURLToken::Version_));
 }
 
-void SharedWorkerClientHolder::ContextDestroyed() {
-  DCHECK(IsMainThread());
-  // Close mojo connections which will signal disinterest in the associated
-  // shared worker.
-  client_receivers_.Clear();
-}
-
 void SharedWorkerClientHolder::Trace(Visitor* visitor) {
+  visitor->Trace(connector_);
+  visitor->Trace(client_receivers_);
   Supplement<Document>::Trace(visitor);
-  ExecutionContextLifecycleObserver::Trace(visitor);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/workers/shared_worker_client_holder.h

@@ -36,16 +36,16 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
-#include "mojo/public/cpp/bindings/remote.h"
-#include "mojo/public/cpp/bindings/unique_receiver_set.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom-blink-forward.h"
-#include "third_party/blink/public/mojom/worker/shared_worker_client.mojom-blink-forward.h"
+#include "third_party/blink/public/mojom/worker/shared_worker_client.mojom-blink.h"
 #include "third_party/blink/public/mojom/worker/shared_worker_connector.mojom-blink.h"
 #include "third_party/blink/public/mojom/worker/shared_worker_info.mojom-blink.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/mojo/heap_mojo_remote.h"
+#include "third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set.h"
 #include "third_party/blink/renderer/platform/wtf/forward.h"
 
 namespace blink {
@@ -63,8 +63,7 @@ class SharedWorker;
 // Supplement<Document>.
 class CORE_EXPORT SharedWorkerClientHolder final
     : public GarbageCollected<SharedWorkerClientHolder>,
-      public Supplement<Document>,
-      public ExecutionContextLifecycleObserver {
+      public Supplement<Document> {
   USING_GARBAGE_COLLECTED_MIXIN(SharedWorkerClientHolder);
 
  public:
@@ -81,14 +80,11 @@ class CORE_EXPORT SharedWorkerClientHolder final
                mojo::PendingRemote<mojom::blink::BlobURLToken>,
                mojom::blink::WorkerOptionsPtr options);
 
-  // Overrides ExecutionContextLifecycleObserver.
-  void ContextDestroyed() override;
-
   void Trace(Visitor* visitor) override;
 
  private:
-  mojo::Remote<mojom::blink::SharedWorkerConnector> connector_;
-  mojo::UniqueReceiverSet<mojom::blink::SharedWorkerClient> client_receivers_;
+  HeapMojoRemote<mojom::blink::SharedWorkerConnector> connector_;
+  HeapMojoUniqueReceiverSet<mojom::blink::SharedWorkerClient> client_receivers_;
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 

third_party/blink/renderer/platform/BUILD.gn

@@ -1228,11 +1228,9 @@ jumbo_component("platform") {
     "mojo/bluetooth_mojom_traits.h",
     "mojo/fetch_api_request_headers_mojom_traits.h",
     "mojo/heap_mojo_receiver.h",
-    "mojo/heap_mojo_receiver.h",
-    "mojo/heap_mojo_receiver_set.h",
     "mojo/heap_mojo_receiver_set.h",
     "mojo/heap_mojo_remote.h",
-    "mojo/heap_mojo_remote.h",
+    "mojo/heap_mojo_unique_receiver_set.h",
     "mojo/kurl_mojom_traits.h",
     "mojo/mojo_helper.h",
     "mojo/security_origin_mojom_traits.h",
@@ -1875,6 +1873,7 @@ jumbo_source_set("blink_platform_unittests_sources") {
     "mojo/big_string_mojom_traits_test.cc",
     "mojo/heap_mojo_receiver_test.cc",
     "mojo/heap_mojo_remote_test.cc",
+    "mojo/heap_mojo_unique_receiver_set_test.cc",
     "mojo/kurl_security_origin_test.cc",
     "mojo/string16_mojom_traits_test.cc",
     "p2p/filtering_network_manager_test.cc",

third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set.h

+// Copyright 2020 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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_MOJO_HEAP_MOJO_UNIQUE_RECEIVER_SET_H_
+#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_MOJO_HEAP_MOJO_UNIQUE_RECEIVER_SET_H_
+
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/unique_receiver_set.h"
+#include "third_party/blink/renderer/platform/context_lifecycle_observer.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
+
+namespace blink {
+
+// HeapMojoUniqueReceiverSet is a wrapper for mojo::UniqueReceiverSet to be
+// owned by a garbage-collected object. Blink is expected to use
+// 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.
+template <typename Interface, typename Deleter = std::default_delete<Interface>>
+class HeapMojoUniqueReceiverSet {
+  DISALLOW_NEW();
+
+ public:
+  using ImplPointerType = typename mojo::Receiver<
+      Interface,
+      mojo::UniquePtrImplRefTraits<Interface, Deleter>>::ImplPointerType;
+
+  explicit HeapMojoUniqueReceiverSet(ContextLifecycleNotifier* context)
+      : wrapper_(MakeGarbageCollected<Wrapper>(context)) {
+    DCHECK(context);
+  }
+
+  // Methods to redirect to mojo::ReceiverSet:
+  mojo::ReceiverId Add(ImplPointerType impl,
+                       mojo::PendingReceiver<Interface> receiver,
+                       scoped_refptr<base::SequencedTaskRunner> task_runner) {
+    return wrapper_->receiver_set().Add(std::move(impl), std::move(receiver),
+                                        task_runner);
+  }
+
+  bool Remove(mojo::ReceiverId id) {
+    return wrapper_->receiver_set().Remove(id);
+  }
+
+  void Clear() { wrapper_->receiver_set().Clear(); }
+
+  bool HasReceiver(mojo::ReceiverId id) {
+    return wrapper_->receiver_set().HasReceiver(id);
+  }
+
+  void Trace(Visitor* visitor) { visitor->Trace(wrapper_); }
+
+ private:
+  // 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:
+    explicit Wrapper(ContextLifecycleNotifier* notifier) {
+      SetContextLifecycleNotifier(notifier);
+    }
+
+    void Trace(Visitor* visitor) override {
+      ContextLifecycleObserver::Trace(visitor);
+    }
+
+    void Dispose() { receiver_set_.Clear(); }
+
+    mojo::UniqueReceiverSet<Interface, void, Deleter>& receiver_set() {
+      return receiver_set_;
+    }
+
+    // ContextLifecycleObserver methods
+    void ContextDestroyed() override { receiver_set_.Clear(); }
+
+   private:
+    mojo::UniqueReceiverSet<Interface, void, Deleter> receiver_set_;
+  };
+
+  Member<Wrapper> wrapper_;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_MOJO_HEAP_MOJO_UNIQUE_RECEIVER_SET_H_

third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set_test.cc

+// Copyright 2020 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 "third_party/blink/renderer/platform/mojo/heap_mojo_unique_receiver_set.h"
+#include "base/test/null_task_runner.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/interfaces/bindings/tests/sample_service.mojom-blink.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/platform/context_lifecycle_notifier.h"
+#include "third_party/blink/renderer/platform/heap/heap_test_utilities.h"
+#include "third_party/blink/renderer/platform/heap/persistent.h"
+#include "third_party/blink/renderer/platform/heap_observer_list.h"
+
+namespace blink {
+
+class FakeContextNotifier final : public GarbageCollected<FakeContextNotifier>,
+                                  public ContextLifecycleNotifier {
+  USING_GARBAGE_COLLECTED_MIXIN(FakeContextNotifier);
+
+ public:
+  FakeContextNotifier() = default;
+
+  void AddContextLifecycleObserver(
+      ContextLifecycleObserver* observer) override {
+    observers_.AddObserver(observer);
+  }
+  void RemoveContextLifecycleObserver(
+      ContextLifecycleObserver* observer) override {
+    observers_.RemoveObserver(observer);
+  }
+
+  void NotifyContextDestroyed() {
+    observers_.ForEachObserver([](ContextLifecycleObserver* observer) {
+      observer->ContextDestroyed();
+    });
+  }
+
+  void Trace(Visitor* visitor) override {
+    visitor->Trace(observers_);
+    ContextLifecycleNotifier::Trace(visitor);
+  }
+
+ private:
+  HeapObserverList<ContextLifecycleObserver> observers_;
+};
+
+class GCOwner : public GarbageCollected<GCOwner> {
+ public:
+  explicit GCOwner(FakeContextNotifier* context) : receiver_set_(context) {}
+  void Trace(Visitor* visitor) { visitor->Trace(receiver_set_); }
+
+  HeapMojoUniqueReceiverSet<sample::blink::Service>& receiver_set() {
+    return receiver_set_;
+  }
+
+ private:
+  HeapMojoUniqueReceiverSet<sample::blink::Service> receiver_set_;
+};
+
+class HeapMojoUniqueReceiverSetTest : public TestSupportingGC {
+ public:
+  FakeContextNotifier* context() { return context_; }
+  scoped_refptr<base::NullTaskRunner> task_runner() {
+    return null_task_runner_;
+  }
+  GCOwner* owner() { return owner_; }
+
+  void ClearOwner() { owner_ = nullptr; }
+
+  void MarkServiceDeleted() { service_deleted_ = true; }
+
+ protected:
+  void SetUp() override {
+    context_ = MakeGarbageCollected<FakeContextNotifier>();
+    owner_ = MakeGarbageCollected<GCOwner>(context());
+  }
+  void TearDown() override {}
+
+  Persistent<FakeContextNotifier> context_;
+  Persistent<GCOwner> owner_;
+  scoped_refptr<base::NullTaskRunner> null_task_runner_ =
+      base::MakeRefCounted<base::NullTaskRunner>();
+  bool service_deleted_ = false;
+};
+
+class MockService : public sample::blink::Service {
+ public:
+  explicit MockService(HeapMojoUniqueReceiverSetTest* test) : test_(test) {}
+  // Notify the test when the service is deleted by the UniqueReceiverSet.
+  ~MockService() override { test_->MarkServiceDeleted(); }
+
+  void Frobinate(sample::blink::FooPtr foo,
+                 Service::BazOptions baz,
+                 mojo::PendingRemote<sample::blink::Port> port,
+                 FrobinateCallback callback) override {}
+  void GetPort(mojo::PendingReceiver<sample::blink::Port> receiver) override {}
+
+ private:
+  HeapMojoUniqueReceiverSetTest* test_;
+};
+
+// GC the HeapMojoUniqueReceiverSet and verify that the receiver is no longer
+// part of the set, and that the service was deleted.
+TEST_F(HeapMojoUniqueReceiverSetTest, ResetsOnGC) {
+  auto receiver_set = owner()->receiver_set();
+  auto service = std::make_unique<MockService>(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(BlinkGC::kConcurrentAndLazySweeping);
+
+  EXPECT_TRUE(service_deleted_);
+
+  CompleteSweepingIfNeeded();
+}
+
+// Destroy the context and verify that the receiver is no longer
+// part of the set, and that the service was deleted.
+TEST_F(HeapMojoUniqueReceiverSetTest, ResetsOnContextDestroyed) {
+  HeapMojoUniqueReceiverSet<sample::blink::Service> receiver_set(context());
+  auto service = std::make_unique<MockService>(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_);
+
+  context_->NotifyContextDestroyed();
+
+  EXPECT_FALSE(receiver_set.HasReceiver(rid));
+  EXPECT_TRUE(service_deleted_);
+}
+
+}  // namespace blink