Pass LB policy args as non-const and using std::move().

src/core/ext/filters/client_channel/lb_policy.cc

@@ -27,11 +27,11 @@ grpc_core::DebugOnlyTraceFlag grpc_trace_lb_policy_refcount(
 
 namespace grpc_core {
 
-LoadBalancingPolicy::LoadBalancingPolicy(const Args& args)
+LoadBalancingPolicy::LoadBalancingPolicy(Args args)
     : InternallyRefCounted(&grpc_trace_lb_policy_refcount),
       combiner_(GRPC_COMBINER_REF(args.combiner, "lb_policy")),
       client_channel_factory_(args.client_channel_factory),
-      subchannel_pool_(*args.subchannel_pool),
+      subchannel_pool_(std::move(args.subchannel_pool)),
       interested_parties_(grpc_pollset_set_create()),
       request_reresolution_(nullptr) {}
 

src/core/ext/filters/client_channel/lb_policy.h

@@ -55,7 +55,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
     /// Used to create channels and subchannels.
     grpc_client_channel_factory* client_channel_factory = nullptr;
     /// Subchannel pool.
-    RefCountedPtr<SubchannelPoolInterface>* subchannel_pool;
+    RefCountedPtr<SubchannelPoolInterface> subchannel_pool;
     /// Channel args from the resolver.
     /// Note that the LB policy gets the set of addresses from the
     /// GRPC_ARG_SERVER_ADDRESS_LIST channel arg.
@@ -187,10 +187,10 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
 
   grpc_pollset_set* interested_parties() const { return interested_parties_; }
 
-  /// Returns a pointer to the subchannel pool of type
-  /// RefCountedPtr<SubchannelPoolInterface>.
-  RefCountedPtr<SubchannelPoolInterface>* subchannel_pool() {
-    return &subchannel_pool_;
+  // Callers that need their own reference can call the returned
+  // object's Ref() method.
+  SubchannelPoolInterface* subchannel_pool() const {
+    return subchannel_pool_.get();
   }
 
   GRPC_ABSTRACT_BASE_CLASS
@@ -198,7 +198,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
  protected:
   GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE
 
-  explicit LoadBalancingPolicy(const Args& args);
+  explicit LoadBalancingPolicy(Args args);
   virtual ~LoadBalancingPolicy();
 
   grpc_combiner* combiner() const { return combiner_; }

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -125,7 +125,7 @@ constexpr char kGrpclb[] = "grpclb";
 
 class GrpcLb : public LoadBalancingPolicy {
  public:
-  explicit GrpcLb(const Args& args);
+  explicit GrpcLb(Args args);
 
   const char* name() const override { return kGrpclb; }
 
@@ -273,7 +273,7 @@ class GrpcLb : public LoadBalancingPolicy {
   // Methods for dealing with the RR policy.
   void CreateOrUpdateRoundRobinPolicyLocked();
   grpc_channel_args* CreateRoundRobinPolicyArgsLocked();
-  void CreateRoundRobinPolicyLocked(const Args& args);
+  void CreateRoundRobinPolicyLocked(Args args);
   bool PickFromRoundRobinPolicyLocked(bool force_async, PendingPick* pp,
                                       grpc_error** error);
   void UpdateConnectivityStateFromRoundRobinPolicyLocked(
@@ -973,8 +973,8 @@ grpc_channel_args* BuildBalancerChannelArgs(
 // ctor and dtor
 //
 
-GrpcLb::GrpcLb(const LoadBalancingPolicy::Args& args)
-    : LoadBalancingPolicy(args),
+GrpcLb::GrpcLb(LoadBalancingPolicy::Args args)
+    : LoadBalancingPolicy(std::move(args)),
       response_generator_(MakeRefCounted<FakeResolverResponseGenerator>()),
       lb_call_backoff_(
           BackOff::Options()
@@ -1588,10 +1588,10 @@ bool GrpcLb::PickFromRoundRobinPolicyLocked(bool force_async, PendingPick* pp,
   return pick_done;
 }
 
-void GrpcLb::CreateRoundRobinPolicyLocked(const Args& args) {
+void GrpcLb::CreateRoundRobinPolicyLocked(Args args) {
   GPR_ASSERT(rr_policy_ == nullptr);
   rr_policy_ = LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy(
-      "round_robin", args);
+      "round_robin", std::move(args));
   if (GPR_UNLIKELY(rr_policy_ == nullptr)) {
     gpr_log(GPR_ERROR, "[grpclb %p] Failure creating a RoundRobin policy",
             this);
@@ -1693,8 +1693,8 @@ void GrpcLb::CreateOrUpdateRoundRobinPolicyLocked() {
     lb_policy_args.combiner = combiner();
     lb_policy_args.client_channel_factory = client_channel_factory();
     lb_policy_args.args = args;
-    lb_policy_args.subchannel_pool = subchannel_pool();
-    CreateRoundRobinPolicyLocked(lb_policy_args);
+    lb_policy_args.subchannel_pool = subchannel_pool()->Ref();
+    CreateRoundRobinPolicyLocked(std::move(lb_policy_args));
   }
   grpc_channel_args_destroy(args);
 }
@@ -1802,7 +1802,7 @@ void GrpcLb::OnRoundRobinConnectivityChangedLocked(void* arg,
 class GrpcLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const LoadBalancingPolicy::Args& args) const override {
+      LoadBalancingPolicy::Args args) const override {
     /* Count the number of gRPC-LB addresses. There must be at least one. */
     const ServerAddressList* addresses =
         FindServerAddressListChannelArg(args.args);
@@ -1815,7 +1815,7 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory {
       }
     }
     if (!found_balancer) return nullptr;
-    return OrphanablePtr<LoadBalancingPolicy>(New<GrpcLb>(args));
+    return OrphanablePtr<LoadBalancingPolicy>(New<GrpcLb>(std::move(args)));
   }
 
   const char* name() const override { return kGrpclb; }

src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc

@@ -46,7 +46,7 @@ constexpr char kPickFirst[] = "pick_first";
 
 class PickFirst : public LoadBalancingPolicy {
  public:
-  explicit PickFirst(const Args& args);
+  explicit PickFirst(Args args);
 
   const char* name() const override { return kPickFirst; }
 
@@ -154,7 +154,7 @@ class PickFirst : public LoadBalancingPolicy {
   channelz::ChildRefsList child_channels_;
 };
 
-PickFirst::PickFirst(const Args& args) : LoadBalancingPolicy(args) {
+PickFirst::PickFirst(Args args) : LoadBalancingPolicy(std::move(args)) {
   GPR_ASSERT(args.client_channel_factory != nullptr);
   gpr_mu_init(&child_refs_mu_);
   grpc_connectivity_state_init(&state_tracker_, GRPC_CHANNEL_IDLE,
@@ -619,8 +619,8 @@ void PickFirst::PickFirstSubchannelData::
 class PickFirstFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const LoadBalancingPolicy::Args& args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<PickFirst>(args));
+      LoadBalancingPolicy::Args args) const override {
+    return OrphanablePtr<LoadBalancingPolicy>(New<PickFirst>(std::move(args)));
   }
 
   const char* name() const override { return kPickFirst; }

src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc

@@ -56,7 +56,7 @@ constexpr char kRoundRobin[] = "round_robin";
 
 class RoundRobin : public LoadBalancingPolicy {
  public:
-  explicit RoundRobin(const Args& args);
+  explicit RoundRobin(Args args);
 
   const char* name() const override { return kRoundRobin; }
 
@@ -210,7 +210,7 @@ class RoundRobin : public LoadBalancingPolicy {
   channelz::ChildRefsList child_channels_;
 };
 
-RoundRobin::RoundRobin(const Args& args) : LoadBalancingPolicy(args) {
+RoundRobin::RoundRobin(Args args) : LoadBalancingPolicy(std::move(args)) {
   GPR_ASSERT(args.client_channel_factory != nullptr);
   gpr_mu_init(&child_refs_mu_);
   grpc_connectivity_state_init(&state_tracker_, GRPC_CHANNEL_IDLE,
@@ -697,8 +697,8 @@ void RoundRobin::UpdateLocked(const grpc_channel_args& args,
 class RoundRobinFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const LoadBalancingPolicy::Args& args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<RoundRobin>(args));
+      LoadBalancingPolicy::Args args) const override {
+    return OrphanablePtr<LoadBalancingPolicy>(New<RoundRobin>(std::move(args)));
   }
 
   const char* name() const override { return kRoundRobin; }

src/core/ext/filters/client_channel/lb_policy/subchannel_list.h

@@ -514,8 +514,8 @@ SubchannelList<SubchannelListType, SubchannelDataType>::SubchannelList(
     // policy, which does not use a SubchannelList.
     GPR_ASSERT(!addresses[i].IsBalancer());
     InlinedVector<grpc_arg, 4> args_to_add;
-    args_to_add.emplace_back(SubchannelPoolInterface::CreateChannelArg(
-        policy_->subchannel_pool()->get()));
+    args_to_add.emplace_back(
+        SubchannelPoolInterface::CreateChannelArg(policy_->subchannel_pool()));
     const size_t subchannel_address_arg_index = args_to_add.size();
     args_to_add.emplace_back(
         grpc_create_subchannel_address_arg(&addresses[i].address()));

src/core/ext/filters/client_channel/lb_policy/xds/xds.cc

@@ -118,7 +118,7 @@ constexpr char kXds[] = "xds_experimental";
 
 class XdsLb : public LoadBalancingPolicy {
  public:
-  explicit XdsLb(const Args& args);
+  explicit XdsLb(Args args);
 
   const char* name() const override { return kXds; }
 
@@ -265,7 +265,7 @@ class XdsLb : public LoadBalancingPolicy {
   // Methods for dealing with the child policy.
   void CreateOrUpdateChildPolicyLocked();
   grpc_channel_args* CreateChildPolicyArgsLocked();
-  void CreateChildPolicyLocked(const Args& args);
+  void CreateChildPolicyLocked(Args args);
   bool PickFromChildPolicyLocked(bool force_async, PendingPick* pp,
                                  grpc_error** error);
   void UpdateConnectivityStateFromChildPolicyLocked(
@@ -892,8 +892,8 @@ grpc_channel_args* BuildBalancerChannelArgs(
 //
 
 // TODO(vishalpowar): Use lb_config in args to configure LB policy.
-XdsLb::XdsLb(const LoadBalancingPolicy::Args& args)
-    : LoadBalancingPolicy(args),
+XdsLb::XdsLb(LoadBalancingPolicy::Args args)
+    : LoadBalancingPolicy(std::move(args)),
       response_generator_(MakeRefCounted<FakeResolverResponseGenerator>()),
       lb_call_backoff_(
           BackOff::Options()
@@ -1436,10 +1436,10 @@ bool XdsLb::PickFromChildPolicyLocked(bool force_async, PendingPick* pp,
   return pick_done;
 }
 
-void XdsLb::CreateChildPolicyLocked(const Args& args) {
+void XdsLb::CreateChildPolicyLocked(Args args) {
   GPR_ASSERT(child_policy_ == nullptr);
   child_policy_ = LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy(
-      "round_robin", args);
+      "round_robin", std::move(args));
   if (GPR_UNLIKELY(child_policy_ == nullptr)) {
     gpr_log(GPR_ERROR, "[xdslb %p] Failure creating a child policy", this);
     return;
@@ -1523,9 +1523,9 @@ void XdsLb::CreateOrUpdateChildPolicyLocked() {
     LoadBalancingPolicy::Args lb_policy_args;
     lb_policy_args.combiner = combiner();
     lb_policy_args.client_channel_factory = client_channel_factory();
-    lb_policy_args.subchannel_pool = subchannel_pool();
+    lb_policy_args.subchannel_pool = subchannel_pool()->Ref();
     lb_policy_args.args = args;
-    CreateChildPolicyLocked(lb_policy_args);
+    CreateChildPolicyLocked(std::move(lb_policy_args));
     if (grpc_lb_xds_trace.enabled()) {
       gpr_log(GPR_INFO, "[xdslb %p] Created a new child policy %p", this,
               child_policy_.get());
@@ -1637,7 +1637,7 @@ void XdsLb::OnChildPolicyConnectivityChangedLocked(void* arg,
 class XdsFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const LoadBalancingPolicy::Args& args) const override {
+      LoadBalancingPolicy::Args args) const override {
     /* Count the number of gRPC-LB addresses. There must be at least one. */
     const ServerAddressList* addresses =
         FindServerAddressListChannelArg(args.args);
@@ -1650,7 +1650,7 @@ class XdsFactory : public LoadBalancingPolicyFactory {
       }
     }
     if (!found_balancer_address) return nullptr;
-    return OrphanablePtr<LoadBalancingPolicy>(New<XdsLb>(args));
+    return OrphanablePtr<LoadBalancingPolicy>(New<XdsLb>(std::move(args)));
   }
 
   const char* name() const override { return kXds; }

src/core/ext/filters/client_channel/lb_policy_factory.h

@@ -31,7 +31,12 @@ class LoadBalancingPolicyFactory {
  public:
   /// Returns a new LB policy instance.
   virtual OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const LoadBalancingPolicy::Args& args) const GRPC_ABSTRACT;
+      LoadBalancingPolicy::Args args) const {
+    std::move(args);  // Suppress clang-tidy complaint.
+    // The rest of this is copied from the GRPC_ABSTRACT macro.
+    gpr_log(GPR_ERROR, "Function marked GRPC_ABSTRACT was not implemented");
+    GPR_ASSERT(false);
+  }
 
   /// Returns the LB policy name that this factory provides.
   /// Caller does NOT take ownership of result.

src/core/ext/filters/client_channel/lb_policy_registry.cc

@@ -84,14 +84,14 @@ void LoadBalancingPolicyRegistry::Builder::RegisterLoadBalancingPolicyFactory(
 
 OrphanablePtr<LoadBalancingPolicy>
 LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy(
-    const char* name, const LoadBalancingPolicy::Args& args) {
+    const char* name, LoadBalancingPolicy::Args args) {
   GPR_ASSERT(g_state != nullptr);
   // Find factory.
   LoadBalancingPolicyFactory* factory =
       g_state->GetLoadBalancingPolicyFactory(name);
   if (factory == nullptr) return nullptr;  // Specified name not found.
   // Create policy via factory.
-  return factory->CreateLoadBalancingPolicy(args);
+  return factory->CreateLoadBalancingPolicy(std::move(args));
 }
 
 bool LoadBalancingPolicyRegistry::LoadBalancingPolicyExists(const char* name) {

src/core/ext/filters/client_channel/lb_policy_registry.h

@@ -46,7 +46,7 @@ class LoadBalancingPolicyRegistry {
 
   /// Creates an LB policy of the type specified by \a name.
   static OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
-      const char* name, const LoadBalancingPolicy::Args& args);
+      const char* name, LoadBalancingPolicy::Args args);
 
   /// Returns true if the LB policy factory specified by \a name exists in this
   /// registry.

src/core/ext/filters/client_channel/request_routing.cc

@@ -676,7 +676,7 @@ void RequestRouter::CreateNewLbPolicyLocked(
   LoadBalancingPolicy::Args lb_policy_args;
   lb_policy_args.combiner = combiner_;
   lb_policy_args.client_channel_factory = client_channel_factory_;
-  lb_policy_args.subchannel_pool = &subchannel_pool_;
+  lb_policy_args.subchannel_pool = subchannel_pool_;
   lb_policy_args.args = resolver_result_;
   lb_policy_args.lb_config = lb_config;
   OrphanablePtr<LoadBalancingPolicy> new_lb_policy =

test/core/util/test_lb_policies.cc

@@ -48,11 +48,17 @@ namespace {
 // A minimal forwarding class to avoid implementing a standalone test LB.
 class ForwardingLoadBalancingPolicy : public LoadBalancingPolicy {
  public:
-  ForwardingLoadBalancingPolicy(const Args& args,
+  ForwardingLoadBalancingPolicy(Args args,
                                 const std::string& delegate_policy_name)
-      : LoadBalancingPolicy(args) {
+      : LoadBalancingPolicy(std::move(args)) {
+    Args delegate_args;
+    delegate_args.combiner = combiner();
+    delegate_args.client_channel_factory = client_channel_factory();
+    delegate_args.subchannel_pool = subchannel_pool()->Ref();
+    delegate_args.args = args.args;
+    delegate_args.lb_config = args.lb_config;
     delegate_ = LoadBalancingPolicyRegistry::CreateLoadBalancingPolicy(
-        delegate_policy_name.c_str(), args);
+        delegate_policy_name.c_str(), std::move(delegate_args));
     grpc_pollset_set_add_pollset_set(delegate_->interested_parties(),
                                      interested_parties());
     // Give re-resolution closure to delegate.
@@ -143,9 +149,8 @@ class InterceptRecvTrailingMetadataLoadBalancingPolicy
     : public ForwardingLoadBalancingPolicy {
  public:
   InterceptRecvTrailingMetadataLoadBalancingPolicy(
-      const Args& args, InterceptRecvTrailingMetadataCallback cb,
-      void* user_data)
-      : ForwardingLoadBalancingPolicy(args,
+      Args args, InterceptRecvTrailingMetadataCallback cb, void* user_data)
+      : ForwardingLoadBalancingPolicy(std::move(args),
                                       /*delegate_lb_policy_name=*/"pick_first"),
         cb_(cb),
         user_data_(user_data) {}
@@ -212,10 +217,10 @@ class InterceptTrailingFactory : public LoadBalancingPolicyFactory {
 
   grpc_core::OrphanablePtr<grpc_core::LoadBalancingPolicy>
   CreateLoadBalancingPolicy(
-      const grpc_core::LoadBalancingPolicy::Args& args) const override {
+      grpc_core::LoadBalancingPolicy::Args args) const override {
     return grpc_core::OrphanablePtr<grpc_core::LoadBalancingPolicy>(
         grpc_core::New<InterceptRecvTrailingMetadataLoadBalancingPolicy>(
-            args, cb_, user_data_));
+            std::move(args), cb_, user_data_));
   }
 
   const char* name() const override {