Move LB policy instantiation from resolvers into client_channel.

7 files changed, 68 insertions, 52 deletions

diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c
index 61e012578e..2e6f253d38 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,24 @@ 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.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 +353,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,
@@ -759,9 +769,10 @@ 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_set_resolver_and_client_channel_factory(
+    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 */
   grpc_channel_element *elem = grpc_channel_stack_last_element(channel_stack);
   channel_data *chand = elem->channel_data;
@@ -776,6 +787,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..b3a9bc7995 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_set_resolver_and_client_channel_factory(
+    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/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 e14f761f05..ba21349d2d 100644
--- a/src/core/ext/client_config/resolver_result.c
+++ b/src/core/ext/client_config/resolver_result.c
@@ -36,6 +36,7 @@
 #include <string.h>
 
 #include <grpc/support/alloc.h>
+#include <grpc/support/string_util.h>
 
 grpc_addresses *grpc_addresses_create(size_t num_addresses) {
   grpc_addresses *addresses = gpr_malloc(sizeof(grpc_addresses));
@@ -63,37 +64,39 @@ void grpc_addresses_destroy(grpc_addresses *addresses) {
 
 struct grpc_resolver_result {
   gpr_refcount refs;
-  grpc_lb_policy *lb_policy;
+  grpc_addresses *addresses;
+  char *lb_policy_name;
 };
 
-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_addresses *addresses,
+                                                  const char *lb_policy_name) {
+  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);
+  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_addresses_destroy(result->addresses);
+    gpr_free(result->lb_policy_name);
+    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_addresses *grpc_resolver_result_get_addresses(
+    grpc_resolver_result *result) {
+  return result->addresses;
 }
 
-grpc_lb_policy *grpc_resolver_result_get_lb_policy(grpc_resolver_result *c) {
-  return c->lb_policy;
+const char *grpc_resolver_result_get_lb_policy_name(
+    grpc_resolver_result *result) {
+  return result->lb_policy_name;
 }
diff --git a/src/core/ext/client_config/resolver_result.h b/src/core/ext/client_config/resolver_result.h
index 4199ef512a..df92bec984 100644
--- a/src/core/ext/client_config/resolver_result.h
+++ b/src/core/ext/client_config/resolver_result.h
@@ -63,14 +63,19 @@ void grpc_addresses_destroy(grpc_addresses *addresses);
 /** Results reported from a grpc_resolver. */
 typedef struct grpc_resolver_result grpc_resolver_result;
 
-grpc_resolver_result *grpc_resolver_result_create();
-void grpc_resolver_result_ref(grpc_resolver_result *client_config);
+/** Takes ownership of \a addresses. */
+grpc_resolver_result *grpc_resolver_result_create(grpc_addresses *addresses,
+                                                  const char *lb_policy_name);
+void grpc_resolver_result_ref(grpc_resolver_result *result);
 void grpc_resolver_result_unref(grpc_exec_ctx *exec_ctx,
-                                grpc_resolver_result *client_config);
+                                grpc_resolver_result *result);
 
-void grpc_resolver_result_set_lb_policy(grpc_resolver_result *client_config,
-                                        grpc_lb_policy *lb_policy);
-grpc_lb_policy *grpc_resolver_result_get_lb_policy(
-    grpc_resolver_result *client_config);
+/** Caller does NOT take ownership of result. */
+grpc_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);
 
 #endif /* GRPC_CORE_EXT_CLIENT_CONFIG_RESOLVER_RESULT_H */