Migrate NetworkQualityEstimatorManager to the new Mojo types

This CL applies pending_remote to RequestNotifications function
in NetworkQualityEstimatorManager interface.

Bug: 955171
Change-Id: I902e22e76e2f0b023c726cde648b832317ee8758
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810166
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697879}

services/network/network_quality_estimator_manager.cc

@@ -117,12 +117,14 @@ void NetworkQualityEstimatorManager::AddRequest(
 }
 
 void NetworkQualityEstimatorManager::RequestNotifications(
-    mojom::NetworkQualityEstimatorManagerClientPtr client_ptr) {
+    mojo::PendingRemote<mojom::NetworkQualityEstimatorManagerClient>
+        pending_client) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  client_ptr->OnNetworkQualityChanged(effective_connection_type_, http_rtt_,
-                                      transport_rtt_,
-                                      downstream_throughput_kbps_);
-  clients_.AddPtr(std::move(client_ptr));
+  mojo::Remote<mojom::NetworkQualityEstimatorManagerClient> client(
+      std::move(pending_client));
+  client->OnNetworkQualityChanged(effective_connection_type_, http_rtt_,
+                                  transport_rtt_, downstream_throughput_kbps_);
+  clients_.Add(std::move(client));
 }
 
 void NetworkQualityEstimatorManager::OnEffectiveConnectionTypeChanged(
@@ -136,12 +138,10 @@ void NetworkQualityEstimatorManager::OnEffectiveConnectionTypeChanged(
     return;
 
   effective_connection_type_ = effective_connection_type;
-  clients_.ForAllPtrs([effective_connection_type, http_rtt, transport_rtt,
-                       downstream_throughput_kbps](
-                          mojom::NetworkQualityEstimatorManagerClient* client) {
+  for (auto& client : clients_) {
     client->OnNetworkQualityChanged(effective_connection_type, http_rtt,
                                     transport_rtt, downstream_throughput_kbps);
-  });
+  }
 }
 
 void NetworkQualityEstimatorManager::OnRTTOrThroughputEstimatesComputed(
@@ -168,12 +168,10 @@ void NetworkQualityEstimatorManager::OnRTTOrThroughputEstimatesComputed(
   transport_rtt_ = transport_rtt;
   downstream_throughput_kbps_ = downstream_throughput_kbps;
 
-  clients_.ForAllPtrs([effective_connection_type, http_rtt, transport_rtt,
-                       downstream_throughput_kbps](
-                          mojom::NetworkQualityEstimatorManagerClient* client) {
+  for (auto& client : clients_) {
     client->OnNetworkQualityChanged(effective_connection_type, http_rtt,
                                     transport_rtt, downstream_throughput_kbps);
-  });
+  }
 }
 
 net::NetworkQualityEstimator*

services/network/network_quality_estimator_manager.h

@@ -12,7 +12,7 @@
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
-#include "mojo/public/cpp/bindings/interface_ptr_set.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 #include "net/nqe/effective_connection_type.h"
 #include "net/nqe/effective_connection_type_observer.h"
 #include "net/nqe/rtt_throughput_estimates_observer.h"
@@ -46,7 +46,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkQualityEstimatorManager
 
   // mojom::NetworkQualityEstimatorManager implementation:
   void RequestNotifications(
-      mojom::NetworkQualityEstimatorManagerClientPtr client_ptr) override;
+      mojo::PendingRemote<mojom::NetworkQualityEstimatorManagerClient> client)
+      override;
 
   net::NetworkQualityEstimator* GetNetworkQualityEstimator() const;
 
@@ -62,7 +63,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkQualityEstimatorManager
 
   std::unique_ptr<net::NetworkQualityEstimator> network_quality_estimator_;
   mojo::BindingSet<mojom::NetworkQualityEstimatorManager> bindings_;
-  mojo::InterfacePtrSet<mojom::NetworkQualityEstimatorManagerClient> clients_;
+  mojo::RemoteSet<mojom::NetworkQualityEstimatorManagerClient> clients_;
   net::EffectiveConnectionType effective_connection_type_;
   base::TimeDelta http_rtt_;
   base::TimeDelta transport_rtt_;

services/network/network_quality_estimator_manager_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "net/log/test_net_log.h"
 #include "net/nqe/effective_connection_type.h"
 #include "net/nqe/network_quality_estimator.h"
@@ -32,18 +33,13 @@ class TestNetworkQualityEstimatorManagerClient
         effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
         http_rtt_(base::TimeDelta()),
         transport_rtt_(base::TimeDelta()),
-        downlink_bandwidth_kbps_(INT32_MAX),
-        binding_(this) {
+        downlink_bandwidth_kbps_(INT32_MAX) {
     mojom::NetworkQualityEstimatorManagerPtr manager_ptr;
     mojom::NetworkQualityEstimatorManagerRequest request(
         mojo::MakeRequest(&manager_ptr));
     network_quality_estimator_manager_->AddRequest(std::move(request));
 
-    mojom::NetworkQualityEstimatorManagerClientPtr client_ptr;
-    mojom::NetworkQualityEstimatorManagerClientRequest client_request(
-        mojo::MakeRequest(&client_ptr));
-    binding_.Bind(std::move(client_request));
-    manager_ptr->RequestNotifications(std::move(client_ptr));
+    manager_ptr->RequestNotifications(receiver_.BindNewPipeAndPassRemote());
   }
 
   ~TestNetworkQualityEstimatorManagerClient() override {}
@@ -92,7 +88,7 @@ class TestNetworkQualityEstimatorManagerClient
   base::TimeDelta http_rtt_;
   base::TimeDelta transport_rtt_;
   int32_t downlink_bandwidth_kbps_;
-  mojo::Binding<mojom::NetworkQualityEstimatorManagerClient> binding_;
+  mojo::Receiver<mojom::NetworkQualityEstimatorManagerClient> receiver_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkQualityEstimatorManagerClient);
 };

services/network/public/cpp/network_quality_tracker.cc

@@ -18,10 +18,9 @@ NetworkQualityTracker::NetworkQualityTracker(
     : get_network_service_callback_(callback),
       effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       downlink_bandwidth_kbps_(std::numeric_limits<int32_t>::max()),
-      network_quality_overridden_for_testing_(false),
-      binding_(this) {
+      network_quality_overridden_for_testing_(false) {
   InitializeMojoChannel();
-  DCHECK(binding_.is_bound());
+  DCHECK(receiver_.is_bound());
 }
 
 NetworkQualityTracker::~NetworkQualityTracker() {}
@@ -106,8 +105,7 @@ void NetworkQualityTracker::ReportRTTsAndThroughputForTesting(
 // For testing only.
 NetworkQualityTracker::NetworkQualityTracker()
     : effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
-      downlink_bandwidth_kbps_(std::numeric_limits<int32_t>::max()),
-      binding_(this) {}
+      downlink_bandwidth_kbps_(std::numeric_limits<int32_t>::max()) {}
 
 void NetworkQualityTracker::OnNetworkQualityChanged(
     net::EffectiveConnectionType effective_connection_type,
@@ -151,7 +149,7 @@ void NetworkQualityTracker::OnNetworkQualityChanged(
 
 void NetworkQualityTracker::InitializeMojoChannel() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(!binding_.is_bound());
+  DCHECK(!receiver_.is_bound());
 
   network::mojom::NetworkService* network_service =
       get_network_service_callback_.Run();
@@ -163,23 +161,18 @@ void NetworkQualityTracker::InitializeMojoChannel() {
       mojo::MakeRequest(&manager_ptr));
   network_service->GetNetworkQualityEstimatorManager(std::move(request));
 
-  // Request notification from NetworkQualityEstimatorManagerClientPtr.
-  network::mojom::NetworkQualityEstimatorManagerClientPtr client_ptr;
-  network::mojom::NetworkQualityEstimatorManagerClientRequest client_request(
-      mojo::MakeRequest(&client_ptr));
-  binding_.Bind(std::move(client_request));
-  manager_ptr->RequestNotifications(std::move(client_ptr));
+  manager_ptr->RequestNotifications(receiver_.BindNewPipeAndPassRemote());
 
   // base::Unretained is safe as destruction of the
-  // NetworkQualityTracker will also destroy the |binding_|.
-  binding_.set_connection_error_handler(base::BindRepeating(
+  // NetworkQualityTracker will also destroy the |receiver_|.
+  receiver_.set_disconnect_handler(base::BindRepeating(
       &NetworkQualityTracker::HandleNetworkServicePipeBroken,
       base::Unretained(this)));
 }
 
 void NetworkQualityTracker::HandleNetworkServicePipeBroken() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  binding_.Close();
+  receiver_.reset();
   InitializeMojoChannel();
 }
 

services/network/public/cpp/network_quality_tracker.h

@@ -12,8 +12,8 @@
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
-#include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "net/nqe/effective_connection_type.h"
 #include "services/network/public/mojom/network_quality_estimator_manager.mojom.h"
 #include "services/network/public/mojom/network_service.mojom-forward.h"
@@ -179,7 +179,8 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkQualityTracker
   base::ObserverList<RTTAndThroughputEstimatesObserver>::Unchecked
       rtt_and_throughput_observer_list_;
 
-  mojo::Binding<network::mojom::NetworkQualityEstimatorManagerClient> binding_;
+  mojo::Receiver<network::mojom::NetworkQualityEstimatorManagerClient>
+      receiver_{this};
 
   SEQUENCE_CHECKER(sequence_checker_);
 

services/network/public/mojom/network_quality_estimator_manager.mojom

@@ -36,5 +36,6 @@ interface NetworkQualityEstimatorManagerClient {
 interface NetworkQualityEstimatorManager {
   // Requests to receive notification when there is a change in the network
   // quality.
-  RequestNotifications(NetworkQualityEstimatorManagerClient client_ptr);
+  RequestNotifications(
+      pending_remote<NetworkQualityEstimatorManagerClient> client);
 };