Reviewer feedback; move lists to children

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

@@ -31,13 +31,10 @@ LoadBalancingPolicy::LoadBalancingPolicy(const Args& args)
       combiner_(GRPC_COMBINER_REF(args.combiner, "lb_policy")),
       client_channel_factory_(args.client_channel_factory),
       interested_parties_(grpc_pollset_set_create()),
-      request_reresolution_(nullptr) {
+      request_reresolution_(nullptr) {}
-  gpr_mu_init(&child_refs_mu_);
-}
 
 LoadBalancingPolicy::~LoadBalancingPolicy() {
   grpc_pollset_set_destroy(interested_parties_);
-  gpr_mu_destroy(&child_refs_mu_);
   GRPC_COMBINER_UNREF(combiner_, "lb_policy");
 }
 

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

@@ -180,9 +180,6 @@ class LoadBalancingPolicy
   grpc_client_channel_factory* client_channel_factory() const {
     return client_channel_factory_;
   }
-  gpr_mu* child_refs_mu() { return &child_refs_mu_; }
-  ChildRefsList* child_subchannels() { return &child_subchannels_; }
-  ChildRefsList* child_channels() { return &child_channels_; }
 
   /// Shuts down the policy.  Any pending picks that have not been
   /// handed off to a new policy via HandOffPendingPicksLocked() will be
@@ -202,11 +199,6 @@ class LoadBalancingPolicy
 
   /// Combiner under which LB policy actions take place.
   grpc_combiner* combiner_;
-  /// Lock and data used to capture snapshots of this channels child
-  /// channels and subchannels. This data is consumed by channelz.
-  gpr_mu child_refs_mu_;
-  ChildRefsList child_subchannels_;
-  ChildRefsList child_channels_;
   /// Client channel factory, used to create channels and subchannels.
   grpc_client_channel_factory* client_channel_factory_;
   /// Owned pointer to interested parties in load balancing decisions.

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

@@ -135,6 +135,8 @@ class GrpcLb : public LoadBalancingPolicy {
   void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override;
   void PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) override;
   void ExitIdleLocked() override;
+  void FillChildRefsForChannelz(ChildRefsList* child_subchannels,
+                                ChildRefsList* child_channels) override {}
 
  private:
   /// Linked list of pending pick requests. It stores all information needed to

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

@@ -58,6 +58,8 @@ class PickFirst : public LoadBalancingPolicy {
   void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override;
   void PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) override;
   void ExitIdleLocked() override;
+  void FillChildRefsForChannelz(ChildRefsList* child_subchannels,
+                                ChildRefsList* child_channels) override;
 
  private:
   ~PickFirst();
@@ -135,10 +137,17 @@ class PickFirst : public LoadBalancingPolicy {
   PickState* pending_picks_ = nullptr;
   // Our connectivity state tracker.
   grpc_connectivity_state_tracker state_tracker_;
+
+  /// Lock and data used to capture snapshots of this channels child
+  /// channels and subchannels. This data is consumed by channelz.
+  gpr_mu child_refs_mu_;
+  ChildRefsList child_subchannels_;
+  ChildRefsList child_channels_;
 };
 
 PickFirst::PickFirst(const Args& args) : LoadBalancingPolicy(args) {
   GPR_ASSERT(args.client_channel_factory != nullptr);
+  gpr_mu_init(&child_refs_mu_);
   grpc_connectivity_state_init(&state_tracker_, GRPC_CHANNEL_IDLE,
                                "pick_first");
   if (grpc_lb_pick_first_trace.enabled()) {
@@ -152,6 +161,7 @@ PickFirst::~PickFirst() {
   if (grpc_lb_pick_first_trace.enabled()) {
     gpr_log(GPR_INFO, "Destroying Pick First %p", this);
   }
+  gpr_mu_destroy(&child_refs_mu_);
   GPR_ASSERT(subchannel_list_ == nullptr);
   GPR_ASSERT(latest_pending_subchannel_list_ == nullptr);
   GPR_ASSERT(pending_picks_ == nullptr);
@@ -294,19 +304,31 @@ void PickFirst::PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) {
   }
 }
 
+void PickFirst::FillChildRefsForChannelz(ChildRefsList* child_subchannels,
+                                         ChildRefsList* child_channels) {
+  mu_guard guard(&child_refs_mu_);
+  // TODO, de dup these.
+  for (size_t i = 0; i < child_subchannels_.size(); ++i) {
+    child_subchannels->push_back(child_subchannels_[i]);
+  }
+  for (size_t i = 0; i < child_channels_.size(); ++i) {
+    child_channels->push_back(child_channels_[i]);
+  }
+}
+
 void PickFirst::UpdateChildRefsLocked() {
-  mu_guard guard(child_refs_mu());
+  mu_guard guard(&child_refs_mu_);
   // reset both lists
-  child_subchannels()->clear();
+  child_subchannels_.clear();
   // this will stay empty, because pick_first channels have no children
   // channels.
-  child_channels()->clear();
+  child_channels_.clear();
   // populate the subchannels with boths subchannels lists, they will be
   // deduped when the actual channelz query comes in.
   if (subchannel_list_ != nullptr) {
     for (size_t i = 0; i < subchannel_list_->num_subchannels(); ++i) {
       if (subchannel_list_->subchannel(i)->subchannel() != nullptr) {
-        child_subchannels()->push_back(grpc_subchannel_get_uuid(
+        child_subchannels_.push_back(grpc_subchannel_get_uuid(
             subchannel_list_->subchannel(i)->subchannel()));
       }
     }
@@ -316,7 +338,7 @@ void PickFirst::UpdateChildRefsLocked() {
          ++i) {
       if (latest_pending_subchannel_list_->subchannel(i)->subchannel() !=
           nullptr) {
-        child_subchannels()->push_back(grpc_subchannel_get_uuid(
+        child_subchannels_.push_back(grpc_subchannel_get_uuid(
             latest_pending_subchannel_list_->subchannel(i)->subchannel()));
       }
     }

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

@@ -69,6 +69,8 @@ class RoundRobin : public LoadBalancingPolicy {
   void HandOffPendingPicksLocked(LoadBalancingPolicy* new_policy) override;
   void PingOneLocked(grpc_closure* on_initiate, grpc_closure* on_ack) override;
   void ExitIdleLocked() override;
+  void FillChildRefsForChannelz(ChildRefsList* child_subchannels,
+                                ChildRefsList* child_channels) override {}
 
  private:
   ~RoundRobin();