Migrate NetworkChangeManager to new Mojo types

This CL applies pending_remote to RequestNotifications in
NetworkChagneManager interface.

  - Convert FooPtr to mojo::PendingRemote or mojo::Remote.
  - Convert mojo::Binding to mojo::Receiver

Bug: 955171
Change-Id: I3b730b082bdb64b8b49c7df181bb2fb3bd0e7eb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1883951
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710184}

media/mojo/clients/mojo_cdm.cc

@@ -305,23 +305,23 @@ Decryptor* MojoCdm::GetDecryptor() {
   if (decryptor_)
     return decryptor_.get();
 
-  mojom::DecryptorPtr decryptor_ptr;
+  mojo::PendingRemote<mojom::Decryptor> decryptor_remote;
 
   // Can be called on a different thread.
   if (decryptor_ptr_info_.is_valid()) {
     DVLOG(1) << __func__ << ": Using Decryptor exposed by the CDM directly";
-    decryptor_ptr.Bind(std::move(decryptor_ptr_info_));
+    decryptor_remote = std::move(decryptor_ptr_info_);
   } else if (interface_factory_ && cdm_id_ != CdmContext::kInvalidCdmId) {
     // TODO(xhwang): Pass back info on whether Decryptor is supported by the
     // remote CDM.
     DVLOG(1) << __func__ << ": Using Decryptor associated with CDM ID "
              << cdm_id_ << ", typically hosted by CdmProxy in MediaService";
-    interface_factory_->CreateDecryptor(cdm_id_,
-                                        mojo::MakeRequest(&decryptor_ptr));
+    interface_factory_->CreateDecryptor(
+        cdm_id_, decryptor_remote.InitWithNewPipeAndPassReceiver());
   }
 
-  if (decryptor_ptr)
-    decryptor_.reset(new MojoDecryptor(std::move(decryptor_ptr)));
+  if (decryptor_remote)
+    decryptor_.reset(new MojoDecryptor(std::move(decryptor_remote)));
 
   return decryptor_.get();
 }

media/mojo/clients/mojo_cdm.h

@@ -156,7 +156,7 @@ class MojoCdm : public ContentDecryptionModule,
   // The DecryptorPtrInfo exposed by the remote CDM. Set after initialization
   // is completed and cleared after |decryptor_| is created. May be invalid
   // after initialization if the CDM doesn't support a Decryptor.
-  mojom::DecryptorPtrInfo decryptor_ptr_info_;
+  mojo::PendingRemote<mojom::Decryptor> decryptor_ptr_info_;
 
   // Decryptor based on |decryptor_ptr_|, lazily created in GetDecryptor().
   // Since GetDecryptor() can be called on a different thread, use

media/mojo/clients/mojo_decryptor.cc

@@ -42,8 +42,9 @@ base::OnceCallback<T> ToOnceCallback(const base::RepeatingCallback<T>& cb) {
 
 // TODO(xhwang): Consider adding an Initialize() to reduce the amount of work
 // done in the constructor.
-MojoDecryptor::MojoDecryptor(mojom::DecryptorPtr remote_decryptor,
-                             uint32_t writer_capacity)
+MojoDecryptor::MojoDecryptor(
+    mojo::PendingRemote<mojom::Decryptor> remote_decryptor,
+    uint32_t writer_capacity)
     : remote_decryptor_(std::move(remote_decryptor)) {
   DVLOG(1) << __func__;
 
@@ -75,7 +76,7 @@ MojoDecryptor::MojoDecryptor(mojom::DecryptorPtr remote_decryptor,
       GetDefaultDecoderBufferConverterCapacity(DemuxerStream::VIDEO),
       &decrypted_producer_handle);
 
-  remote_decryptor_.set_connection_error_with_reason_handler(
+  remote_decryptor_.set_disconnect_with_reason_handler(
       base::Bind(&MojoDecryptor::OnConnectionError, base::Unretained(this)));
 
   // Pass the other end of each pipe to |remote_decryptor_|.

media/mojo/clients/mojo_decryptor.h

@@ -14,6 +14,8 @@
 #include "media/base/decryptor.h"
 #include "media/mojo/mojom/decryptor.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 namespace media {
 
@@ -28,7 +30,7 @@ class MojoDecryptor : public Decryptor {
  public:
   // |writer_capacity| can be used for testing. If 0, default writer capacity
   // will be used.
-  MojoDecryptor(mojom::DecryptorPtr remote_decryptor,
+  MojoDecryptor(mojo::PendingRemote<mojom::Decryptor> remote_decryptor,
                 uint32_t writer_capacity = 0);
   ~MojoDecryptor() final;
 
@@ -89,7 +91,7 @@ class MojoDecryptor : public Decryptor {
 
   base::ThreadChecker thread_checker_;
 
-  mojom::DecryptorPtr remote_decryptor_;
+  mojo::Remote<mojom::Decryptor> remote_decryptor_;
 
   // Helper class to send DecoderBuffer to the |remote_decryptor_| for
   // DecryptAndDecodeAudio(), DecryptAndDecodeVideo() and Decrypt().

media/mojo/clients/mojo_decryptor_unittest.cc

@@ -20,7 +20,7 @@
 #include "media/mojo/common/mojo_shared_buffer_video_frame.h"
 #include "media/mojo/mojom/decryptor.mojom.h"
 #include "media/mojo/services/mojo_decryptor_service.h"
-#include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -44,16 +44,15 @@ class MojoDecryptorTest : public ::testing::Test {
   void Initialize() {
     decryptor_.reset(new StrictMock<MockDecryptor>());
 
-    mojom::DecryptorPtr remote_decryptor;
     mojo_decryptor_service_.reset(
         new MojoDecryptorService(decryptor_.get(), nullptr));
-    binding_ = std::make_unique<mojo::Binding<mojom::Decryptor>>(
-        mojo_decryptor_service_.get(), MakeRequest(&remote_decryptor));
-    binding_->set_connection_error_handler(base::BindOnce(
-        &MojoDecryptorTest::OnConnectionClosed, base::Unretained(this)));
 
-    mojo_decryptor_.reset(
-        new MojoDecryptor(std::move(remote_decryptor), writer_capacity_));
+    receiver_ = std::make_unique<mojo::Receiver<mojom::Decryptor>>(
+        mojo_decryptor_service_.get());
+    mojo_decryptor_ = std::make_unique<MojoDecryptor>(
+        receiver_->BindNewPipeAndPassRemote(), writer_capacity_);
+    receiver_->set_disconnect_handler(base::BindOnce(
+        &MojoDecryptorTest::OnConnectionClosed, base::Unretained(this)));
   }
 
   void DestroyClient() {
@@ -64,7 +63,7 @@ class MojoDecryptorTest : public ::testing::Test {
   void DestroyService() {
     // MojoDecryptor has no way to notify callers that the connection is closed.
     // TODO(jrummell): Determine if notification is needed.
-    binding_.reset();
+    receiver_.reset();
     mojo_decryptor_service_.reset();
   }
 
@@ -122,7 +121,7 @@ class MojoDecryptorTest : public ::testing::Test {
 
   // The matching MojoDecryptorService for |mojo_decryptor_|.
   std::unique_ptr<MojoDecryptorService> mojo_decryptor_service_;
-  std::unique_ptr<mojo::Binding<mojom::Decryptor>> binding_;
+  std::unique_ptr<mojo::Receiver<mojom::Decryptor>> receiver_;
 
   // The actual Decryptor object used by |mojo_decryptor_service_|.
   std::unique_ptr<StrictMock<MockDecryptor>> decryptor_;

media/mojo/services/media_service_unittest.cc

@@ -233,10 +233,10 @@ class MediaServiceTest : public testing::Test {
 
   void CreateDecryptor(int cdm_id, bool expected_result) {
     base::RunLoop run_loop;
-    mojom::DecryptorPtr decryptor_ptr;
-    interface_factory_->CreateDecryptor(cdm_id,
-                                        mojo::MakeRequest(&decryptor_ptr));
-    MojoDecryptor mojo_decryptor(std::move(decryptor_ptr));
+    mojo::PendingRemote<mojom::Decryptor> decryptor_remote;
+    interface_factory_->CreateDecryptor(
+        cdm_id, decryptor_remote.InitWithNewPipeAndPassReceiver());
+    MojoDecryptor mojo_decryptor(std::move(decryptor_remote));
 
     // In the success case, there's no decryption key to decrypt the buffer so
     // we would expect no-key.

services/network/network_change_manager.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/network_change_notifier_posix.h"
 
@@ -32,17 +33,20 @@ void NetworkChangeManager::AddReceiver(
 }
 
 void NetworkChangeManager::RequestNotifications(
-    mojom::NetworkChangeManagerClientPtr client_ptr) {
-  client_ptr.set_connection_error_handler(
-      base::Bind(&NetworkChangeManager::NotificationPipeBroken,
-                 // base::Unretained is safe as destruction of the
-                 // NetworkChangeManager will also destroy the
-                 // |clients_| list (which this object will be
-                 // inserted into, below), which will destroy the
-                 // client_ptr, rendering this callback moot.
-                 base::Unretained(this), base::Unretained(client_ptr.get())));
-  client_ptr->OnInitialConnectionType(connection_type_);
-  clients_.push_back(std::move(client_ptr));
+    mojo::PendingRemote<mojom::NetworkChangeManagerClient>
+        client_pending_remote) {
+  mojo::Remote<mojom::NetworkChangeManagerClient> client_remote(
+      std::move(client_pending_remote));
+  client_remote.set_disconnect_handler(base::Bind(
+      &NetworkChangeManager::NotificationPipeBroken,
+      // base::Unretained is safe as destruction of the
+      // NetworkChangeManager will also destroy the
+      // |clients_| list (which this object will be
+      // inserted into, below), which will destroy the
+      // client_remote, rendering this callback moot.
+      base::Unretained(this), base::Unretained(client_remote.get())));
+  client_remote->OnInitialConnectionType(connection_type_);
+  clients_.push_back(std::move(client_remote));
 }
 
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
@@ -82,11 +86,11 @@ size_t NetworkChangeManager::GetNumClientsForTesting() const {
 
 void NetworkChangeManager::NotificationPipeBroken(
     mojom::NetworkChangeManagerClient* client) {
-  clients_.erase(
-      std::find_if(clients_.begin(), clients_.end(),
-                   [client](mojom::NetworkChangeManagerClientPtr& ptr) {
-                     return ptr.get() == client;
-                   }));
+  clients_.erase(std::find_if(
+      clients_.begin(), clients_.end(),
+      [client](mojo::Remote<mojom::NetworkChangeManagerClient>& remote) {
+        return remote.get() == client;
+      }));
 }
 
 void NetworkChangeManager::OnNetworkChanged(

services/network/network_change_manager.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/network_change_notifier.h"
 #include "services/network/public/mojom/network_change_manager.mojom.h"
 
@@ -39,7 +40,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkChangeManager
 
   // mojom::NetworkChangeManager implementation:
   void RequestNotifications(
-      mojom::NetworkChangeManagerClientPtr client_ptr) override;
+      mojo::PendingRemote<mojom::NetworkChangeManagerClient> client_remote)
+      override;
 
 #if defined(OS_CHROMEOS) || defined(OS_ANDROID)
   void OnNetworkChanged(
@@ -63,7 +65,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkChangeManager
 
   std::unique_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   mojo::ReceiverSet<mojom::NetworkChangeManager> receivers_;
-  std::vector<mojom::NetworkChangeManagerClientPtr> clients_;
+  std::vector<mojo::Remote<mojom::NetworkChangeManagerClient>> clients_;
   mojom::ConnectionType connection_type_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkChangeManager);

services/network/network_change_manager_unittest.cc

@@ -38,16 +38,13 @@ class TestNetworkChangeManagerClient
       : num_network_changed_(0),
         run_loop_(std::make_unique<base::RunLoop>()),
         notification_type_to_wait_(NONE),
-        connection_type_(mojom::ConnectionType::CONNECTION_UNKNOWN),
-        binding_(this) {
+        connection_type_(mojom::ConnectionType::CONNECTION_UNKNOWN) {
     mojo::Remote<mojom::NetworkChangeManager> manager;
     network_change_manager->AddReceiver(manager.BindNewPipeAndPassReceiver());
 
-    mojom::NetworkChangeManagerClientPtr client_ptr;
-    mojom::NetworkChangeManagerClientRequest client_request(
-        mojo::MakeRequest(&client_ptr));
-    binding_.Bind(std::move(client_request));
-    manager->RequestNotifications(std::move(client_ptr));
+    mojo::PendingRemote<mojom::NetworkChangeManagerClient> client_remote;
+    receiver_.Bind(client_remote.InitWithNewPipeAndPassReceiver());
+    manager->RequestNotifications(std::move(client_remote));
   }
 
   ~TestNetworkChangeManagerClient() override {}
@@ -82,7 +79,7 @@ class TestNetworkChangeManagerClient
   std::unique_ptr<base::RunLoop> run_loop_;
   NotificationType notification_type_to_wait_;
   mojom::ConnectionType connection_type_;
-  mojo::Binding<mojom::NetworkChangeManagerClient> binding_;
+  mojo::Receiver<mojom::NetworkChangeManagerClient> receiver_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeManagerClient);
 };

services/network/network_service_unittest.cc

@@ -1318,17 +1318,14 @@ class TestNetworkChangeManagerClient
  public:
   explicit TestNetworkChangeManagerClient(
       mojom::NetworkService* network_service)
-      : connection_type_(mojom::ConnectionType::CONNECTION_UNKNOWN),
-        binding_(this) {
+      : connection_type_(mojom::ConnectionType::CONNECTION_UNKNOWN) {
     mojo::Remote<mojom::NetworkChangeManager> manager_remote;
     network_service->GetNetworkChangeManager(
         manager_remote.BindNewPipeAndPassReceiver());
 
-    mojom::NetworkChangeManagerClientPtr client_ptr;
-    mojom::NetworkChangeManagerClientRequest client_request(
-        mojo::MakeRequest(&client_ptr));
-    binding_.Bind(std::move(client_request));
-    manager_remote->RequestNotifications(std::move(client_ptr));
+    mojo::PendingRemote<mojom::NetworkChangeManagerClient> client_remote;
+    receiver_.Bind(client_remote.InitWithNewPipeAndPassReceiver());
+    manager_remote->RequestNotifications(std::move(client_remote));
   }
 
   ~TestNetworkChangeManagerClient() override {}
@@ -1350,12 +1347,12 @@ class TestNetworkChangeManagerClient
     run_loop_.Run();
   }
 
-  void Flush() { binding_.FlushForTesting(); }
+  void Flush() { receiver_.FlushForTesting(); }
 
  private:
   base::RunLoop run_loop_;
   mojom::ConnectionType connection_type_;
-  mojo::Binding<mojom::NetworkChangeManagerClient> binding_;
+  mojo::Receiver<mojom::NetworkChangeManagerClient> receiver_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeManagerClient);
 };

services/network/public/cpp/network_connection_tracker.cc

@@ -9,6 +9,8 @@
 #include "base/bind.h"
 #include "base/task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/network/public/mojom/network_change_manager.mojom.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 
@@ -45,10 +47,9 @@ NetworkConnectionTracker::NetworkConnectionTracker(BindingCallback callback)
               base::ObserverListPolicy::EXISTING_ONLY)),
       leaky_network_change_observer_list_(
           new base::ObserverListThreadSafe<NetworkConnectionObserver>(
-              base::ObserverListPolicy::EXISTING_ONLY)),
-      binding_(this) {
+              base::ObserverListPolicy::EXISTING_ONLY)) {
   Initialize();
-  DCHECK(binding_.is_bound());
+  DCHECK(receiver_.is_bound());
 }
 
 NetworkConnectionTracker::~NetworkConnectionTracker() {
@@ -138,8 +139,7 @@ NetworkConnectionTracker::NetworkConnectionTracker()
               base::ObserverListPolicy::EXISTING_ONLY)),
       leaky_network_change_observer_list_(
           new base::ObserverListThreadSafe<NetworkConnectionObserver>(
-              base::ObserverListPolicy::EXISTING_ONLY)),
-      binding_(this) {}
+              base::ObserverListPolicy::EXISTING_ONLY)) {}
 
 void NetworkConnectionTracker::OnInitialConnectionType(
     network::mojom::ConnectionType type) {
@@ -164,30 +164,26 @@ void NetworkConnectionTracker::OnNetworkChanged(
 
 void NetworkConnectionTracker::Initialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(!binding_.is_bound());
-
-  // Get NetworkChangeManagerPtr.
-  network::mojom::NetworkChangeManagerPtr manager_ptr;
-  network::mojom::NetworkChangeManagerRequest request(
-      mojo::MakeRequest(&manager_ptr));
-  bind_request_callback_.Run(std::move(request));
-
-  // Request notification from NetworkChangeManagerPtr.
-  network::mojom::NetworkChangeManagerClientPtr client_ptr;
-  network::mojom::NetworkChangeManagerClientRequest client_request(
-      mojo::MakeRequest(&client_ptr));
-  binding_.Bind(std::move(client_request));
-  manager_ptr->RequestNotifications(std::move(client_ptr));
-
-  // base::Unretained is safe as |binding_| is owned by |this|.
-  binding_.set_connection_error_handler(base::BindRepeating(
+  DCHECK(!receiver_.is_bound());
+
+  // Get mojo::Remote<NetworkChangeManager>.
+  mojo::Remote<network::mojom::NetworkChangeManager> manager_remote;
+  bind_request_callback_.Run(manager_remote.BindNewPipeAndPassReceiver());
+
+  // Request notification from mojo::Remote<NetworkChangeManager>.
+  mojo::PendingRemote<network::mojom::NetworkChangeManagerClient> client_remote;
+  receiver_.Bind(client_remote.InitWithNewPipeAndPassReceiver());
+  manager_remote->RequestNotifications(std::move(client_remote));
+
+  // base::Unretained is safe as |receiver_| is owned by |this|.
+  receiver_.set_disconnect_handler(base::BindRepeating(
       &NetworkConnectionTracker::HandleNetworkServicePipeBroken,
       base::Unretained(this)));
 }
 
 void NetworkConnectionTracker::HandleNetworkServicePipeBroken() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  binding_.Close();
+  receiver_.reset();
   // Reset |connection_type_| to invalid, so future GetConnectionType() can be
   // delayed after network service has restarted, and that there isn't an
   // incorrectly cached state.

services/network/public/cpp/network_connection_tracker.h

@@ -16,7 +16,7 @@
 #include "base/observer_list_threadsafe.h"
 #include "base/sequence_checker.h"
 #include "base/synchronization/lock.h"
-#include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "services/network/public/mojom/network_change_manager.mojom.h"
 
 namespace network {
@@ -148,7 +148,7 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkConnectionTracker
   const scoped_refptr<base::ObserverListThreadSafe<NetworkConnectionObserver>>
       leaky_network_change_observer_list_;
 
-  mojo::Binding<network::mojom::NetworkChangeManagerClient> binding_;
+  mojo::Receiver<network::mojom::NetworkChangeManagerClient> receiver_{this};
 
   // Only the initialization and re-initialization of |this| are required to
   // be bound to the same sequence.

services/network/public/mojom/network_change_manager.mojom

@@ -84,7 +84,8 @@ interface NetworkChangeManagerClient {
 // An interface for facilitating notifications of network change events.
 interface NetworkChangeManager {
   // Requests to receive notification when there is a network change.
-  RequestNotifications(NetworkChangeManagerClient client_ptr);
+  RequestNotifications(
+      pending_remote<NetworkChangeManagerClient> client_remote);
 
   // Notifies the network service that the network configuration has changed.
   // This is needed for ChromeOS because only one process can listen for network