Merge pull request #8049 from markdroth/grpclb_resolver_changes2

Move LB policy instantiation from resolvers into client_channel.

13 files changed, 139 insertions, 115 deletions

diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c
index 6b779efe57..63797bfa1e 100644
--- a/src/core/ext/client_config/client_channel.c
+++ b/src/core/ext/client_config/client_channel.c
@@ -42,6 +42,7 @@
 #include <grpc/support/sync.h>
 #include <grpc/support/useful.h>
 
+#include "src/core/ext/client_config/lb_policy_registry.h"
 #include "src/core/ext/client_config/subchannel.h"
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/channel/connected_channel.h"
@@ -63,6 +64,8 @@ typedef struct client_channel_channel_data {
   grpc_resolver *resolver;
   /** have we started resolving this channel */
   bool started_resolving;
+  /** client channel factory */
+  grpc_client_channel_factory *client_channel_factory;
 
   /** mutex protecting client configuration, including all
       variables below in this data structure */
@@ -173,20 +176,26 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
   grpc_error *state_error = GRPC_ERROR_CREATE("No load balancing policy");
 
   if (chand->resolver_result != NULL) {
-    lb_policy = grpc_resolver_result_get_lb_policy(chand->resolver_result);
+    grpc_lb_policy_args lb_policy_args;
+    lb_policy_args.addresses =
+        grpc_resolver_result_get_addresses(chand->resolver_result);
+    lb_policy_args.additional_args =
+        grpc_resolver_result_get_lb_policy_args(chand->resolver_result);
+    lb_policy_args.client_channel_factory = chand->client_channel_factory;
+    lb_policy = grpc_lb_policy_create(
+        exec_ctx,
+        grpc_resolver_result_get_lb_policy_name(chand->resolver_result),
+        &lb_policy_args);
     if (lb_policy != NULL) {
-      GRPC_LB_POLICY_REF(lb_policy, "channel");
       GRPC_LB_POLICY_REF(lb_policy, "config_change");
       GRPC_ERROR_UNREF(state_error);
       state =
           grpc_lb_policy_check_connectivity(exec_ctx, lb_policy, &state_error);
     }
-
     grpc_resolver_result_unref(exec_ctx, chand->resolver_result);
+    chand->resolver_result = NULL;
   }
 
-  chand->resolver_result = NULL;
-
   if (lb_policy != NULL) {
     grpc_pollset_set_add_pollset_set(exec_ctx, lb_policy->interested_parties,
                                      chand->interested_parties);
@@ -346,6 +355,9 @@ static void cc_destroy_channel_elem(grpc_exec_ctx *exec_ctx,
     grpc_resolver_shutdown(exec_ctx, chand->resolver);
     GRPC_RESOLVER_UNREF(exec_ctx, chand->resolver, "channel");
   }
+  if (chand->client_channel_factory != NULL) {
+    grpc_client_channel_factory_unref(exec_ctx, chand->client_channel_factory);
+  }
   if (chand->lb_policy != NULL) {
     grpc_pollset_set_del_pollset_set(exec_ctx,
                                      chand->lb_policy->interested_parties,
@@ -767,10 +779,12 @@ const grpc_channel_filter grpc_client_channel_filter = {
     "client-channel",
 };
 
-void grpc_client_channel_set_resolver(grpc_exec_ctx *exec_ctx,
-                                      grpc_channel_stack *channel_stack,
-                                      grpc_resolver *resolver) {
+void grpc_client_channel_finish_initialization(
+    grpc_exec_ctx *exec_ctx, grpc_channel_stack *channel_stack,
+    grpc_resolver *resolver,
+    grpc_client_channel_factory *client_channel_factory) {
   /* post construction initialization: set the transport setup pointer */
+  GPR_ASSERT(client_channel_factory != NULL);
   grpc_channel_element *elem = grpc_channel_stack_last_element(channel_stack);
   channel_data *chand = elem->channel_data;
   gpr_mu_lock(&chand->mu);
@@ -784,6 +798,8 @@ void grpc_client_channel_set_resolver(grpc_exec_ctx *exec_ctx,
     grpc_resolver_next(exec_ctx, resolver, &chand->resolver_result,
                        &chand->on_resolver_result_changed);
   }
+  chand->client_channel_factory = client_channel_factory;
+  grpc_client_channel_factory_ref(client_channel_factory);
   gpr_mu_unlock(&chand->mu);
 }
 
diff --git a/src/core/ext/client_config/client_channel.h b/src/core/ext/client_config/client_channel.h
index 1e47ad34ad..abb5ac4d87 100644
--- a/src/core/ext/client_config/client_channel.h
+++ b/src/core/ext/client_config/client_channel.h
@@ -34,6 +34,7 @@
 #ifndef GRPC_CORE_EXT_CLIENT_CONFIG_CLIENT_CHANNEL_H
 #define GRPC_CORE_EXT_CLIENT_CONFIG_CLIENT_CHANNEL_H
 
+#include "src/core/ext/client_config/client_channel_factory.h"
 #include "src/core/ext/client_config/resolver.h"
 #include "src/core/lib/channel/channel_stack.h"
 
@@ -46,12 +47,12 @@
 
 extern const grpc_channel_filter grpc_client_channel_filter;
 
-/* post-construction initializer to let the client channel know which
-   transport setup it should cancel upon destruction, or initiate when it needs
-   a connection */
-void grpc_client_channel_set_resolver(grpc_exec_ctx *exec_ctx,
-                                      grpc_channel_stack *channel_stack,
-                                      grpc_resolver *resolver);
+/* Post-construction initializer to give the client channel its resolver
+   and factory. */
+void grpc_client_channel_finish_initialization(
+    grpc_exec_ctx *exec_ctx, grpc_channel_stack *channel_stack,
+    grpc_resolver *resolver,
+    grpc_client_channel_factory *client_channel_factory);
 
 grpc_connectivity_state grpc_client_channel_check_connectivity_state(
     grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, int try_to_connect);
diff --git a/src/core/ext/client_config/lb_policy_factory.c b/src/core/ext/client_config/lb_policy_factory.c
index f86117aa38..c17af91a09 100644
--- a/src/core/ext/client_config/lb_policy_factory.c
+++ b/src/core/ext/client_config/lb_policy_factory.c
@@ -34,6 +34,7 @@
 #include <string.h>
 
 #include <grpc/support/alloc.h>
+#include <grpc/support/string_util.h>
 
 #include "src/core/ext/client_config/lb_policy_factory.h"
 
@@ -46,6 +47,25 @@ grpc_lb_addresses* grpc_lb_addresses_create(size_t num_addresses) {
   return addresses;
 }
 
+grpc_lb_addresses* grpc_lb_addresses_copy(grpc_lb_addresses* addresses,
+                                          void* (*user_data_copy)(void*)) {
+  grpc_lb_addresses* new_addresses =
+      grpc_lb_addresses_create(addresses->num_addresses);
+  memcpy(new_addresses->addresses, addresses->addresses,
+         sizeof(grpc_lb_address) * addresses->num_addresses);
+  for (size_t i = 0; i < addresses->num_addresses; ++i) {
+    if (new_addresses->addresses[i].balancer_name != NULL) {
+      new_addresses->addresses[i].balancer_name =
+          gpr_strdup(new_addresses->addresses[i].balancer_name);
+    }
+    if (user_data_copy != NULL) {
+      new_addresses->addresses[i].user_data =
+          user_data_copy(new_addresses->addresses[i].user_data);
+    }
+  }
+  return new_addresses;
+}
+
 void grpc_lb_addresses_set_address(grpc_lb_addresses* addresses, size_t index,
                                    void* address, size_t address_len,
                                    bool is_balancer, char* balancer_name,
diff --git a/src/core/ext/client_config/lb_policy_factory.h b/src/core/ext/client_config/lb_policy_factory.h
index b31179cf80..54408c6308 100644
--- a/src/core/ext/client_config/lb_policy_factory.h
+++ b/src/core/ext/client_config/lb_policy_factory.h
@@ -36,9 +36,9 @@
 
 #include "src/core/ext/client_config/client_channel_factory.h"
 #include "src/core/ext/client_config/lb_policy.h"
-#include "src/core/ext/client_config/resolver_result.h"
 
 #include "src/core/lib/iomgr/exec_ctx.h"
+#include "src/core/lib/iomgr/resolve_address.h"
 
 typedef struct grpc_lb_policy_factory grpc_lb_policy_factory;
 typedef struct grpc_lb_policy_factory_vtable grpc_lb_policy_factory_vtable;
@@ -68,6 +68,11 @@ typedef struct grpc_lb_addresses {
  * \a num_addresses addresses. */
 grpc_lb_addresses *grpc_lb_addresses_create(size_t num_addresses);
 
+/** Creates a copy of \a addresses.  If \a user_data_copy is not NULL,
+ * it will be invoked to copy the \a user_data field of each address. */
+grpc_lb_addresses *grpc_lb_addresses_copy(grpc_lb_addresses *addresses,
+                                          void *(*user_data_copy)(void *));
+
 /** Sets the value of the address at index \a index of \a addresses.
  * \a address is a socket address of length \a address_len.
  * Takes ownership of \a balancer_name. */
@@ -82,9 +87,13 @@ void grpc_lb_addresses_destroy(grpc_lb_addresses *addresses,
                                void (*user_data_destroy)(void *));
 
 /** Arguments passed to LB policies. */
+/* TODO(roth, ctiller): Consider replacing this struct with
+   grpc_channel_args.  See comment in resolver_result.h for details. */
 typedef struct grpc_lb_policy_args {
   grpc_lb_addresses *addresses;
   grpc_client_channel_factory *client_channel_factory;
+  /* Can be used to pass implementation-specific parameters to the LB policy. */
+  grpc_channel_args *additional_args;
 } grpc_lb_policy_args;
 
 struct grpc_lb_policy_factory_vtable {
diff --git a/src/core/ext/client_config/resolver_factory.h b/src/core/ext/client_config/resolver_factory.h
index f69bf79564..9ec5b9a70e 100644
--- a/src/core/ext/client_config/resolver_factory.h
+++ b/src/core/ext/client_config/resolver_factory.h
@@ -47,10 +47,7 @@ struct grpc_resolver_factory {
   const grpc_resolver_factory_vtable *vtable;
 };
 
-typedef struct grpc_resolver_args {
-  grpc_uri *uri;
-  grpc_client_channel_factory *client_channel_factory;
-} grpc_resolver_args;
+typedef struct grpc_resolver_args { grpc_uri *uri; } grpc_resolver_args;
 
 struct grpc_resolver_factory_vtable {
   void (*ref)(grpc_resolver_factory *factory);
diff --git a/src/core/ext/client_config/resolver_registry.c b/src/core/ext/client_config/resolver_registry.c
index e7a4abd568..b5308a140c 100644
--- a/src/core/ext/client_config/resolver_registry.c
+++ b/src/core/ext/client_config/resolver_registry.c
@@ -128,15 +128,13 @@ static grpc_resolver_factory *resolve_factory(const char *target,
   return factory;
 }
 
-grpc_resolver *grpc_resolver_create(
-    const char *target, grpc_client_channel_factory *client_channel_factory) {
+grpc_resolver *grpc_resolver_create(const char *target) {
   grpc_uri *uri = NULL;
   grpc_resolver_factory *factory = resolve_factory(target, &uri);
   grpc_resolver *resolver;
   grpc_resolver_args args;
   memset(&args, 0, sizeof(args));
   args.uri = uri;
-  args.client_channel_factory = client_channel_factory;
   resolver = grpc_resolver_factory_create_resolver(factory, &args);
   grpc_uri_destroy(uri);
   return resolver;
diff --git a/src/core/ext/client_config/resolver_registry.h b/src/core/ext/client_config/resolver_registry.h
index 5ef1383cd3..92e248d548 100644
--- a/src/core/ext/client_config/resolver_registry.h
+++ b/src/core/ext/client_config/resolver_registry.h
@@ -55,8 +55,7 @@ void grpc_register_resolver_type(grpc_resolver_factory *factory);
     If a resolver factory was found, use it to instantiate a resolver and
     return it.
     If a resolver factory was not found, return NULL. */
-grpc_resolver *grpc_resolver_create(
-    const char *target, grpc_client_channel_factory *client_channel_factory);
+grpc_resolver *grpc_resolver_create(const char *target);
 
 /** Find a resolver factory given a name and return an (owned-by-the-caller)
  *  reference to it */
diff --git a/src/core/ext/client_config/resolver_result.c b/src/core/ext/client_config/resolver_result.c
index 242989a0da..59c9e7dc25 100644
--- a/src/core/ext/client_config/resolver_result.c
+++ b/src/core/ext/client_config/resolver_result.c
@@ -34,40 +34,54 @@
 #include <string.h>
 
 #include <grpc/support/alloc.h>
+#include <grpc/support/string_util.h>
+
+#include "src/core/lib/channel/channel_args.h"
 
 struct grpc_resolver_result {
   gpr_refcount refs;
-  grpc_lb_policy* lb_policy;
+  grpc_lb_addresses* addresses;
+  char* lb_policy_name;
+  grpc_channel_args* lb_policy_args;
 };
 
-grpc_resolver_result* grpc_resolver_result_create() {
-  grpc_resolver_result* c = gpr_malloc(sizeof(*c));
-  memset(c, 0, sizeof(*c));
-  gpr_ref_init(&c->refs, 1);
-  return c;
+grpc_resolver_result* grpc_resolver_result_create(
+    grpc_lb_addresses* addresses, const char* lb_policy_name,
+    grpc_channel_args* lb_policy_args) {
+  grpc_resolver_result* result = gpr_malloc(sizeof(*result));
+  memset(result, 0, sizeof(*result));
+  gpr_ref_init(&result->refs, 1);
+  result->addresses = addresses;
+  result->lb_policy_name = gpr_strdup(lb_policy_name);
+  result->lb_policy_args = lb_policy_args;
+  return result;
 }
 
-void grpc_resolver_result_ref(grpc_resolver_result* c) { gpr_ref(&c->refs); }
+void grpc_resolver_result_ref(grpc_resolver_result* result) {
+  gpr_ref(&result->refs);
+}
 
 void grpc_resolver_result_unref(grpc_exec_ctx* exec_ctx,
-                                grpc_resolver_result* c) {
-  if (gpr_unref(&c->refs)) {
-    if (c->lb_policy != NULL) {
-      GRPC_LB_POLICY_UNREF(exec_ctx, c->lb_policy, "resolver_result");
-    }
-    gpr_free(c);
+                                grpc_resolver_result* result) {
+  if (gpr_unref(&result->refs)) {
+    grpc_lb_addresses_destroy(result->addresses, NULL /* user_data_destroy */);
+    gpr_free(result->lb_policy_name);
+    grpc_channel_args_destroy(result->lb_policy_args);
+    gpr_free(result);
   }
 }
 
-void grpc_resolver_result_set_lb_policy(grpc_resolver_result* c,
-                                        grpc_lb_policy* lb_policy) {
-  GPR_ASSERT(c->lb_policy == NULL);
-  if (lb_policy) {
-    GRPC_LB_POLICY_REF(lb_policy, "resolver_result");
-  }
-  c->lb_policy = lb_policy;
+grpc_lb_addresses* grpc_resolver_result_get_addresses(
+    grpc_resolver_result* result) {
+  return result->addresses;
+}
+
+const char* grpc_resolver_result_get_lb_policy_name(
+    grpc_resolver_result* result) {
+  return result->lb_policy_name;
 }
 
-grpc_lb_policy* grpc_resolver_result_get_lb_policy(grpc_resolver_result* c) {
-  return c->lb_policy;
+grpc_channel_args* grpc_resolver_result_get_lb_policy_args(
+    grpc_resolver_result* result) {
+  return result->lb_policy_args;
 }
diff --git a/src/core/ext/client_config/resolver_result.h b/src/core/ext/client_config/resolver_result.h
index 5a69d81990..d4118b90e8 100644
--- a/src/core/ext/client_config/resolver_result.h
+++ b/src/core/ext/client_config/resolver_result.h
@@ -32,22 +32,40 @@
 #ifndef GRPC_CORE_EXT_CLIENT_CONFIG_RESOLVER_RESULT_H
 #define GRPC_CORE_EXT_CLIENT_CONFIG_RESOLVER_RESULT_H
 
-#include <stdbool.h>
-
-#include "src/core/ext/client_config/lb_policy.h"
+#include "src/core/ext/client_config/lb_policy_factory.h"
 #include "src/core/lib/iomgr/resolve_address.h"
 
+// TODO(roth, ctiller): In the long term, we are considering replacing
+// the resolver_result data structure with grpc_channel_args.  The idea is
+// that the resolver will return a set of channel args that contains the
+// information that is currently in the resolver_result struct.  For
+// example, there will be specific args indicating the set of addresses
+// and the name of the LB policy to instantiate.  Note that if we did
+// this, we would probably want to change the data structure of
+// grpc_channel_args such to a hash table or AVL or some other data
+// structure that does not require linear search to find keys.
+
 /// Results reported from a grpc_resolver.
 typedef struct grpc_resolver_result grpc_resolver_result;
 
-grpc_resolver_result* grpc_resolver_result_create();
+/// Takes ownership of \a addresses and \a lb_policy_args.
+grpc_resolver_result* grpc_resolver_result_create(
+    grpc_lb_addresses* addresses, const char* lb_policy_name,
+    grpc_channel_args* lb_policy_args);
 void grpc_resolver_result_ref(grpc_resolver_result* result);
 void grpc_resolver_result_unref(grpc_exec_ctx* exec_ctx,
                                 grpc_resolver_result* result);
 
-void grpc_resolver_result_set_lb_policy(grpc_resolver_result* result,
-                                        grpc_lb_policy* lb_policy);
-grpc_lb_policy* grpc_resolver_result_get_lb_policy(
+/// Caller does NOT take ownership of result.
+grpc_lb_addresses* grpc_resolver_result_get_addresses(
+    grpc_resolver_result* result);
+
+/// Caller does NOT take ownership of result.
+const char* grpc_resolver_result_get_lb_policy_name(
+    grpc_resolver_result* result);
+
+/// Caller does NOT take ownership of result.
+grpc_channel_args* grpc_resolver_result_get_lb_policy_args(
     grpc_resolver_result* result);
 
 #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_RESOLVER_RESULT_H */
diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c
index 63682db7de..1b85d2d634 100644
--- a/src/core/ext/resolver/dns/native/dns_resolver.c
+++ b/src/core/ext/resolver/dns/native/dns_resolver.c
@@ -37,7 +37,6 @@
 #include <grpc/support/host_port.h>
 #include <grpc/support/string_util.h>
 
-#include "src/core/ext/client_config/lb_policy_registry.h"
 #include "src/core/ext/client_config/resolver_registry.h"
 #include "src/core/lib/iomgr/resolve_address.h"
 #include "src/core/lib/iomgr/timer.h"
@@ -58,8 +57,6 @@ typedef struct {
   char *name;
   /** default port to use */
   char *default_port;
-  /** subchannel factory */
-  grpc_client_channel_factory *client_channel_factory;
   /** load balancing policy name */
   char *lb_policy_name;
 
@@ -166,31 +163,20 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg,
                             grpc_error *error) {
   dns_resolver *r = arg;
   grpc_resolver_result *result = NULL;
-  grpc_lb_policy *lb_policy;
   gpr_mu_lock(&r->mu);
   GPR_ASSERT(r->resolving);
   r->resolving = 0;
   if (r->addresses != NULL) {
-    grpc_lb_policy_args lb_policy_args;
-    memset(&lb_policy_args, 0, sizeof(lb_policy_args));
-    lb_policy_args.addresses = grpc_lb_addresses_create(r->addresses->naddrs);
+    grpc_lb_addresses *addresses =
+        grpc_lb_addresses_create(r->addresses->naddrs);
     for (size_t i = 0; i < r->addresses->naddrs; ++i) {
       grpc_lb_addresses_set_address(
-          lb_policy_args.addresses, i, &r->addresses->addrs[i].addr,
+          addresses, i, &r->addresses->addrs[i].addr,
           r->addresses->addrs[i].len, false /* is_balancer */,
           NULL /* balancer_name */, NULL /* user_data */);
     }
     grpc_resolved_addresses_destroy(r->addresses);
-    lb_policy_args.client_channel_factory = r->client_channel_factory;
-    lb_policy =
-        grpc_lb_policy_create(exec_ctx, r->lb_policy_name, &lb_policy_args);
-    grpc_lb_addresses_destroy(lb_policy_args.addresses,
-                              NULL /* user_data_destroy */);
-    result = grpc_resolver_result_create();
-    if (lb_policy != NULL) {
-      grpc_resolver_result_set_lb_policy(result, lb_policy);
-      GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "construction");
-    }
+    result = grpc_resolver_result_create(addresses, r->lb_policy_name, NULL);
   } else {
     gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
     gpr_timespec next_try = gpr_backoff_step(&r->backoff_state, now);
@@ -251,7 +237,6 @@ static void dns_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) {
   if (r->resolved_result) {
     grpc_resolver_result_unref(exec_ctx, r->resolved_result);
   }
-  grpc_client_channel_factory_unref(exec_ctx, r->client_channel_factory);
   gpr_free(r->name);
   gpr_free(r->default_port);
   gpr_free(r->lb_policy_name);
@@ -278,10 +263,8 @@ static grpc_resolver *dns_create(grpc_resolver_args *args,
   grpc_resolver_init(&r->base, &dns_resolver_vtable);
   r->name = gpr_strdup(path);
   r->default_port = gpr_strdup(default_port);
-  r->client_channel_factory = args->client_channel_factory;
   gpr_backoff_init(&r->backoff_state, BACKOFF_MULTIPLIER, BACKOFF_JITTER,
                    BACKOFF_MIN_SECONDS * 1000, BACKOFF_MAX_SECONDS * 1000);
-  grpc_client_channel_factory_ref(r->client_channel_factory);
   r->lb_policy_name = gpr_strdup(lb_policy_name);
   return &r->base;
 }
diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c
index fbfe5d774b..ced4fb024c 100644
--- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c
+++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c
@@ -40,7 +40,6 @@
 #include <grpc/support/port_platform.h>
 #include <grpc/support/string_util.h>
 
-#include "src/core/ext/client_config/lb_policy_registry.h"
 #include "src/core/ext/client_config/parse_address.h"
 #include "src/core/ext/client_config/resolver_registry.h"
 #include "src/core/lib/iomgr/resolve_address.h"
@@ -52,8 +51,6 @@ typedef struct {
   grpc_resolver base;
   /** refcount */
   gpr_refcount refs;
-  /** subchannel factory */
-  grpc_client_channel_factory *client_channel_factory;
   /** load balancing policy name */
   char *lb_policy_name;
 
@@ -122,17 +119,10 @@ static void sockaddr_next(grpc_exec_ctx *exec_ctx, grpc_resolver *resolver,
 static void sockaddr_maybe_finish_next_locked(grpc_exec_ctx *exec_ctx,
                                               sockaddr_resolver *r) {
   if (r->next_completion != NULL && !r->published) {
-    grpc_resolver_result *result = grpc_resolver_result_create();
-    grpc_lb_policy_args lb_policy_args;
-    memset(&lb_policy_args, 0, sizeof(lb_policy_args));
-    lb_policy_args.addresses = r->addresses;
-    lb_policy_args.client_channel_factory = r->client_channel_factory;
-    grpc_lb_policy *lb_policy =
-        grpc_lb_policy_create(exec_ctx, r->lb_policy_name, &lb_policy_args);
-    grpc_resolver_result_set_lb_policy(result, lb_policy);
-    GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "sockaddr");
     r->published = true;
-    *r->target_result = result;
+    *r->target_result = grpc_resolver_result_create(
+        grpc_lb_addresses_copy(r->addresses, NULL /* user_data_copy */),
+        r->lb_policy_name, NULL);
     grpc_exec_ctx_sched(exec_ctx, r->next_completion, GRPC_ERROR_NONE, NULL);
     r->next_completion = NULL;
   }
@@ -141,7 +131,6 @@ static void sockaddr_maybe_finish_next_locked(grpc_exec_ctx *exec_ctx,
 static void sockaddr_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) {
   sockaddr_resolver *r = (sockaddr_resolver *)gr;
   gpr_mu_destroy(&r->mu);
-  grpc_client_channel_factory_unref(exec_ctx, r->client_channel_factory);
   grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */);
   gpr_free(r->lb_policy_name);
   gpr_free(r);
@@ -243,8 +232,6 @@ static grpc_resolver *sockaddr_create(
   gpr_ref_init(&r->refs, 1);
   gpr_mu_init(&r->mu);
   grpc_resolver_init(&r->base, &sockaddr_resolver_vtable);
-  r->client_channel_factory = args->client_channel_factory;
-  grpc_client_channel_factory_ref(r->client_channel_factory);
 
   return &r->base;
 }
diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.c b/src/core/ext/transport/chttp2/client/insecure/channel_create.c
index cbaa75a90a..ddc00bd79f 100644
--- a/src/core/ext/transport/chttp2/client/insecure/channel_create.c
+++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.c
@@ -160,7 +160,6 @@ typedef struct {
   grpc_client_channel_factory base;
   gpr_refcount refs;
   grpc_channel_args *merge_args;
-  grpc_channel *master;
 } client_channel_factory;
 
 static void client_channel_factory_ref(
@@ -173,10 +172,6 @@ static void client_channel_factory_unref(
     grpc_exec_ctx *exec_ctx, grpc_client_channel_factory *cc_factory) {
   client_channel_factory *f = (client_channel_factory *)cc_factory;
   if (gpr_unref(&f->refs)) {
-    if (f->master != NULL) {
-      GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, f->master,
-                                  "client_channel_factory");
-    }
     grpc_channel_args_destroy(f->merge_args);
     gpr_free(f);
   }
@@ -210,15 +205,15 @@ static grpc_channel *client_channel_factory_create_channel(
   grpc_channel *channel = grpc_channel_create(exec_ctx, target, final_args,
                                               GRPC_CLIENT_CHANNEL, NULL);
   grpc_channel_args_destroy(final_args);
-  grpc_resolver *resolver = grpc_resolver_create(target, &f->base);
+  grpc_resolver *resolver = grpc_resolver_create(target);
   if (!resolver) {
     GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, channel,
                                 "client_channel_factory_create_channel");
     return NULL;
   }
 
-  grpc_client_channel_set_resolver(
-      exec_ctx, grpc_channel_get_channel_stack(channel), resolver);
+  grpc_client_channel_finish_initialization(
+      exec_ctx, grpc_channel_get_channel_stack(channel), resolver, &f->base);
   GRPC_RESOLVER_UNREF(exec_ctx, resolver, "create_channel");
 
   return channel;
@@ -250,12 +245,8 @@ grpc_channel *grpc_insecure_channel_create(const char *target,
 
   grpc_channel *channel = client_channel_factory_create_channel(
       &exec_ctx, &f->base, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR, NULL);
-  if (channel != NULL) {
-    f->master = channel;
-    GRPC_CHANNEL_INTERNAL_REF(f->master, "grpc_insecure_channel_create");
-  }
-  grpc_client_channel_factory_unref(&exec_ctx, &f->base);
 
+  grpc_client_channel_factory_unref(&exec_ctx, &f->base);
   grpc_exec_ctx_finish(&exec_ctx);
 
   return channel != NULL ? channel : grpc_lame_client_channel_create(
diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
index 9e2bdd758f..f36fbbfc57 100644
--- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
+++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
@@ -220,7 +220,6 @@ typedef struct {
   gpr_refcount refs;
   grpc_channel_args *merge_args;
   grpc_channel_security_connector *security_connector;
-  grpc_channel *master;
 } client_channel_factory;
 
 static void client_channel_factory_ref(
@@ -235,10 +234,6 @@ static void client_channel_factory_unref(
   if (gpr_unref(&f->refs)) {
     GRPC_SECURITY_CONNECTOR_UNREF(&f->security_connector->base,
                                   "client_channel_factory");
-    if (f->master != NULL) {
-      GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, f->master,
-                                  "client_channel_factory");
-    }
     grpc_channel_args_destroy(f->merge_args);
     gpr_free(f);
   }
@@ -276,10 +271,10 @@ static grpc_channel *client_channel_factory_create_channel(
                                               GRPC_CLIENT_CHANNEL, NULL);
   grpc_channel_args_destroy(final_args);
 
-  grpc_resolver *resolver = grpc_resolver_create(target, &f->base);
+  grpc_resolver *resolver = grpc_resolver_create(target);
   if (resolver != NULL) {
-    grpc_client_channel_set_resolver(
-        exec_ctx, grpc_channel_get_channel_stack(channel), resolver);
+    grpc_client_channel_finish_initialization(
+        exec_ctx, grpc_channel_get_channel_stack(channel), resolver, &f->base);
     GRPC_RESOLVER_UNREF(exec_ctx, resolver, "create");
   } else {
     GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, channel,
@@ -356,10 +351,6 @@ grpc_channel *grpc_secure_channel_create(grpc_channel_credentials *creds,
 
   grpc_channel *channel = client_channel_factory_create_channel(
       &exec_ctx, &f->base, target, GRPC_CLIENT_CHANNEL_TYPE_REGULAR, NULL);
-  if (channel != NULL) {
-    f->master = channel;
-    GRPC_CHANNEL_INTERNAL_REF(f->master, "grpc_secure_channel_create");
-  }
 
   grpc_client_channel_factory_unref(&exec_ctx, &f->base);
   grpc_exec_ctx_finish(&exec_ctx);