Merge pull request #20405 from markdroth/orphanable_polymorphism

Fix polymorphism for UniquePtr<> and OrphanablePtr<>.

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

@@ -1515,9 +1515,7 @@ void ChannelData::CreateResolvingLoadBalancingPolicyLocked() {
   // Instantiate resolving LB policy.
   LoadBalancingPolicy::Args lb_args;
   lb_args.combiner = combiner_;
-  lb_args.channel_control_helper =
-      UniquePtr<LoadBalancingPolicy::ChannelControlHelper>(
-          New<ClientChannelControlHelper>(this));
+  lb_args.channel_control_helper = MakeUnique<ClientChannelControlHelper>(this);
   lb_args.args = channel_args_;
   UniquePtr<char> target_uri(gpr_strdup(target_uri_.get()));
   resolving_lb_policy_.reset(New<ResolvingLoadBalancingPolicy>(
@@ -1791,9 +1789,8 @@ void ChannelData::StartTransportOpLocked(void* arg, grpc_error* ignored) {
                                      MemoryOrder::RELEASE);
       chand->UpdateStateAndPickerLocked(
           GRPC_CHANNEL_SHUTDOWN, "shutdown from API",
-          UniquePtr<LoadBalancingPolicy::SubchannelPicker>(
-              New<LoadBalancingPolicy::TransientFailurePicker>(
-                  GRPC_ERROR_REF(op->disconnect_with_error))));
+          MakeUnique<LoadBalancingPolicy::TransientFailurePicker>(
+              GRPC_ERROR_REF(op->disconnect_with_error)));
     }
   }
   GRPC_CHANNEL_STACK_UNREF(chand->owning_stack_, "start_transport_op");

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

@@ -356,5 +356,5 @@ void grpc_http_connect_register_handshaker_factory() {
   using namespace grpc_core;
   HandshakerRegistry::RegisterHandshakerFactory(
       true /* at_start */, HANDSHAKER_CLIENT,
-      UniquePtr<HandshakerFactory>(New<HttpConnectHandshakerFactory>()));
+      MakeUnique<HttpConnectHandshakerFactory>());
 }

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

@@ -716,9 +716,8 @@ void GrpcLb::Helper::UpdateState(grpc_connectivity_state state,
     client_stats = parent_->lb_calld_->client_stats()->Ref();
   }
   parent_->channel_control_helper()->UpdateState(
-      state, UniquePtr<SubchannelPicker>(
-                 New<Picker>(parent_.get(), parent_->serverlist_,
-                             std::move(picker), std::move(client_stats))));
+      state, MakeUnique<Picker>(parent_.get(), parent_->serverlist_,
+                                std::move(picker), std::move(client_stats)));
 }
 
 void GrpcLb::Helper::RequestReresolution() {
@@ -1794,7 +1793,7 @@ class GrpcLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<GrpcLb>(std::move(args)));
+    return MakeOrphanable<GrpcLb>(std::move(args));
   }
 
   const char* name() const override { return kGrpclb; }
@@ -1869,8 +1868,7 @@ bool maybe_add_client_load_reporting_filter(grpc_channel_stack_builder* builder,
 void grpc_lb_policy_grpclb_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::UniquePtr<grpc_core::LoadBalancingPolicyFactory>(
-              grpc_core::New<grpc_core::GrpcLbFactory>()));
+          grpc_core::MakeUnique<grpc_core::GrpcLbFactory>());
   grpc_channel_init_register_stage(GRPC_CLIENT_SUBCHANNEL,
                                    GRPC_CHANNEL_INIT_BUILTIN_PRIORITY,
                                    maybe_add_client_load_reporting_filter,

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

@@ -201,7 +201,7 @@ void PickFirst::AttemptToConnectUsingLatestUpdateArgsLocked() {
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+        MakeUnique<TransientFailurePicker>(error));
     return;
   }
   // If one of the subchannels in the new list is already in state
@@ -319,12 +319,11 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
             GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
         p->channel_control_helper()->UpdateState(
             GRPC_CHANNEL_TRANSIENT_FAILURE,
-            UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+            MakeUnique<TransientFailurePicker>(error));
       } else {
         p->channel_control_helper()->UpdateState(
             GRPC_CHANNEL_CONNECTING,
-            UniquePtr<SubchannelPicker>(
-                New<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker"))));
+            MakeUnique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
       }
     } else {
       if (connectivity_state == GRPC_CHANNEL_TRANSIENT_FAILURE) {
@@ -339,20 +338,19 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
         p->selected_ = nullptr;
         p->subchannel_list_.reset();
         p->channel_control_helper()->UpdateState(
-            GRPC_CHANNEL_IDLE, UniquePtr<SubchannelPicker>(New<QueuePicker>(
-                                   p->Ref(DEBUG_LOCATION, "QueuePicker"))));
+            GRPC_CHANNEL_IDLE,
+            MakeUnique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
       } else {
         // This is unlikely but can happen when a subchannel has been asked
         // to reconnect by a different channel and this channel has dropped
         // some connectivity state notifications.
         if (connectivity_state == GRPC_CHANNEL_READY) {
           p->channel_control_helper()->UpdateState(
-              GRPC_CHANNEL_READY,
-              UniquePtr<SubchannelPicker>(New<Picker>(subchannel()->Ref())));
+              GRPC_CHANNEL_READY, MakeUnique<Picker>(subchannel()->Ref()));
         } else {  // CONNECTING
           p->channel_control_helper()->UpdateState(
-              connectivity_state, UniquePtr<SubchannelPicker>(New<QueuePicker>(
-                                      p->Ref(DEBUG_LOCATION, "QueuePicker"))));
+              connectivity_state,
+              MakeUnique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
         }
       }
     }
@@ -396,7 +394,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
               GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
           p->channel_control_helper()->UpdateState(
               GRPC_CHANNEL_TRANSIENT_FAILURE,
-              UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+              MakeUnique<TransientFailurePicker>(error));
         }
       }
       sd->CheckConnectivityStateAndStartWatchingLocked();
@@ -408,8 +406,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
       if (subchannel_list() == p->subchannel_list_.get()) {
         p->channel_control_helper()->UpdateState(
             GRPC_CHANNEL_CONNECTING,
-            UniquePtr<SubchannelPicker>(
-                New<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker"))));
+            MakeUnique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
       }
       break;
     }
@@ -448,8 +445,7 @@ void PickFirst::PickFirstSubchannelData::ProcessUnselectedReadyLocked() {
   }
   p->selected_ = this;
   p->channel_control_helper()->UpdateState(
-      GRPC_CHANNEL_READY,
-      UniquePtr<SubchannelPicker>(New<Picker>(subchannel()->Ref())));
+      GRPC_CHANNEL_READY, MakeUnique<Picker>(subchannel()->Ref()));
   for (size_t i = 0; i < subchannel_list()->num_subchannels(); ++i) {
     if (i != Index()) {
       subchannel_list()->subchannel(i)->ShutdownLocked();
@@ -488,7 +484,7 @@ class PickFirstFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<PickFirst>(std::move(args)));
+    return MakeOrphanable<PickFirst>(std::move(args));
   }
 
   const char* name() const override { return kPickFirst; }
@@ -510,8 +506,7 @@ class PickFirstFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_pick_first_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::UniquePtr<grpc_core::LoadBalancingPolicyFactory>(
-              grpc_core::New<grpc_core::PickFirstFactory>()));
+          grpc_core::MakeUnique<grpc_core::PickFirstFactory>());
 }
 
 void grpc_lb_policy_pick_first_shutdown() {}

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

@@ -321,13 +321,13 @@ void RoundRobin::RoundRobinSubchannelList::
    */
   if (num_ready_ > 0) {
     /* 1) READY */
-    p->channel_control_helper()->UpdateState(
-        GRPC_CHANNEL_READY, UniquePtr<SubchannelPicker>(New<Picker>(p, this)));
+    p->channel_control_helper()->UpdateState(GRPC_CHANNEL_READY,
+                                             MakeUnique<Picker>(p, this));
   } else if (num_connecting_ > 0) {
     /* 2) CONNECTING */
     p->channel_control_helper()->UpdateState(
-        GRPC_CHANNEL_CONNECTING, UniquePtr<SubchannelPicker>(New<QueuePicker>(
-                                     p->Ref(DEBUG_LOCATION, "QueuePicker"))));
+        GRPC_CHANNEL_CONNECTING,
+        MakeUnique<QueuePicker>(p->Ref(DEBUG_LOCATION, "QueuePicker")));
   } else if (num_transient_failure_ == num_subchannels()) {
     /* 3) TRANSIENT_FAILURE */
     grpc_error* error =
@@ -336,7 +336,7 @@ void RoundRobin::RoundRobinSubchannelList::
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     p->channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+        MakeUnique<TransientFailurePicker>(error));
   }
 }
 
@@ -453,7 +453,7 @@ void RoundRobin::UpdateLocked(UpdateArgs args) {
                            GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+        MakeUnique<TransientFailurePicker>(error));
     subchannel_list_ = std::move(latest_pending_subchannel_list_);
   } else if (subchannel_list_ == nullptr) {
     // If there is no current list, immediately promote the new list to
@@ -480,7 +480,7 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<RoundRobin>(std::move(args)));
+    return MakeOrphanable<RoundRobin>(std::move(args));
   }
 
   const char* name() const override { return kRoundRobin; }
@@ -502,8 +502,7 @@ class RoundRobinFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_round_robin_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::UniquePtr<grpc_core::LoadBalancingPolicyFactory>(
-              grpc_core::New<grpc_core::RoundRobinFactory>()));
+          grpc_core::MakeUnique<grpc_core::RoundRobinFactory>());
 }
 
 void grpc_lb_policy_round_robin_shutdown() {}

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

@@ -2179,7 +2179,7 @@ void XdsLb::PriorityList::UpdateXdsPickerLocked() {
         GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
     xds_policy_->channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(error)));
+        MakeUnique<TransientFailurePicker>(error));
     return;
   }
   priorities_[current_priority_]->UpdateXdsPickerLocked();
@@ -2272,9 +2272,8 @@ XdsLb::PriorityList::LocalityMap::LocalityMap(RefCountedPtr<XdsLb> xds_policy,
   // This is the first locality map ever created, report CONNECTING.
   if (priority_ == 0) {
     xds_policy_->channel_control_helper()->UpdateState(
-        GRPC_CHANNEL_CONNECTING,
-        UniquePtr<SubchannelPicker>(
-            New<QueuePicker>(xds_policy_->Ref(DEBUG_LOCATION, "QueuePicker"))));
+        GRPC_CHANNEL_CONNECTING, MakeUnique<QueuePicker>(xds_policy_->Ref(
+                                     DEBUG_LOCATION, "QueuePicker")));
   }
 }
 
@@ -2347,9 +2346,9 @@ void XdsLb::PriorityList::LocalityMap::UpdateXdsPickerLocked() {
     picker_list.push_back(MakePair(end, locality->picker_wrapper()));
   }
   xds_policy()->channel_control_helper()->UpdateState(
-      GRPC_CHANNEL_READY, UniquePtr<SubchannelPicker>(New<Picker>(
-                              xds_policy_->Ref(DEBUG_LOCATION, "XdsLb+Picker"),
-                              std::move(picker_list))));
+      GRPC_CHANNEL_READY,
+      MakeUnique<Picker>(xds_policy_->Ref(DEBUG_LOCATION, "XdsLb+Picker"),
+                         std::move(picker_list)));
 }
 
 OrphanablePtr<XdsLb::PriorityList::LocalityMap::Locality>
@@ -2898,7 +2897,7 @@ class XdsFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(New<XdsLb>(std::move(args)));
+    return MakeOrphanable<XdsLb>(std::move(args));
   }
 
   const char* name() const override { return kXds; }
@@ -2982,8 +2981,7 @@ class XdsFactory : public LoadBalancingPolicyFactory {
 void grpc_lb_policy_xds_init() {
   grpc_core::LoadBalancingPolicyRegistry::Builder::
       RegisterLoadBalancingPolicyFactory(
-          grpc_core::UniquePtr<grpc_core::LoadBalancingPolicyFactory>(
-              grpc_core::New<grpc_core::XdsFactory>()));
+          grpc_core::MakeUnique<grpc_core::XdsFactory>());
 }
 
 void grpc_lb_policy_xds_shutdown() {}

src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc

@@ -436,7 +436,7 @@ class AresDnsResolverFactory : public ResolverFactory {
   bool IsValidUri(const grpc_uri* uri) const override { return true; }
 
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
-    return OrphanablePtr<Resolver>(New<AresDnsResolver>(std::move(args)));
+    return MakeOrphanable<AresDnsResolver>(std::move(args));
   }
 
   const char* scheme() const override { return "dns"; }
@@ -494,8 +494,7 @@ void grpc_resolver_dns_ares_init() {
     }
     grpc_set_resolver_impl(&ares_resolver);
     grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-        grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-            grpc_core::New<grpc_core::AresDnsResolverFactory>()));
+        grpc_core::MakeUnique<grpc_core::AresDnsResolverFactory>());
   } else {
     g_use_ares_dns_resolver = false;
   }

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_libuv.cc

@@ -171,7 +171,7 @@ class GrpcPolledFdFactoryLibuv : public GrpcPolledFdFactory {
 };
 
 UniquePtr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(grpc_combiner* combiner) {
-  return UniquePtr<GrpcPolledFdFactory>(New<GrpcPolledFdFactoryLibuv>());
+  return MakeUnique<GrpcPolledFdFactoryLibuv>();
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_posix.cc

@@ -98,7 +98,7 @@ class GrpcPolledFdFactoryPosix : public GrpcPolledFdFactory {
 };
 
 UniquePtr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(grpc_combiner* combiner) {
-  return UniquePtr<GrpcPolledFdFactory>(New<GrpcPolledFdFactoryPosix>());
+  return MakeUnique<GrpcPolledFdFactoryPosix>();
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver_windows.cc

@@ -904,8 +904,7 @@ class GrpcPolledFdFactoryWindows : public GrpcPolledFdFactory {
 };
 
 UniquePtr<GrpcPolledFdFactory> NewGrpcPolledFdFactory(grpc_combiner* combiner) {
-  return UniquePtr<GrpcPolledFdFactory>(
-      New<GrpcPolledFdFactoryWindows>(combiner));
+  return MakeUnique<GrpcPolledFdFactoryWindows>(combiner);
 }
 
 }  // namespace grpc_core

src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc

@@ -268,7 +268,7 @@ class NativeDnsResolverFactory : public ResolverFactory {
 
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     if (!IsValidUri(args.uri)) return nullptr;
-    return OrphanablePtr<Resolver>(New<NativeDnsResolver>(std::move(args)));
+    return MakeOrphanable<NativeDnsResolver>(std::move(args));
   }
 
   const char* scheme() const override { return "dns"; }
@@ -284,8 +284,7 @@ void grpc_resolver_dns_native_init() {
   if (gpr_stricmp(resolver.get(), "native") == 0) {
     gpr_log(GPR_DEBUG, "Using native dns resolver");
     grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-        grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-            grpc_core::New<grpc_core::NativeDnsResolverFactory>()));
+        grpc_core::MakeUnique<grpc_core::NativeDnsResolverFactory>());
   } else {
     grpc_core::ResolverRegistry::Builder::InitRegistry();
     grpc_core::ResolverFactory* existing_factory =
@@ -293,8 +292,7 @@ void grpc_resolver_dns_native_init() {
     if (existing_factory == nullptr) {
       gpr_log(GPR_DEBUG, "Using native dns resolver");
       grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-          grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-              grpc_core::New<grpc_core::NativeDnsResolverFactory>()));
+          grpc_core::MakeUnique<grpc_core::NativeDnsResolverFactory>());
     }
   }
 }

src/core/ext/filters/client_channel/resolver/fake/fake_resolver.cc

@@ -382,7 +382,7 @@ class FakeResolverFactory : public ResolverFactory {
   bool IsValidUri(const grpc_uri* uri) const override { return true; }
 
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
-    return OrphanablePtr<Resolver>(New<FakeResolver>(std::move(args)));
+    return MakeOrphanable<FakeResolver>(std::move(args));
   }
 
   const char* scheme() const override { return "fake"; }
@@ -394,8 +394,7 @@ class FakeResolverFactory : public ResolverFactory {
 
 void grpc_resolver_fake_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-          grpc_core::New<grpc_core::FakeResolverFactory>()));
+      grpc_core::MakeUnique<grpc_core::FakeResolverFactory>());
 }
 
 void grpc_resolver_fake_shutdown() {}

src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.cc

@@ -119,8 +119,8 @@ OrphanablePtr<Resolver> CreateSockaddrResolver(
   ServerAddressList addresses;
   if (!ParseUri(args.uri, parse, &addresses)) return nullptr;
   // Instantiate resolver.
-  return OrphanablePtr<Resolver>(
-      New<SockaddrResolver>(std::move(addresses), std::move(args)));
+  return MakeOrphanable<SockaddrResolver>(std::move(addresses),
+                                          std::move(args));
 }
 
 class IPv4ResolverFactory : public ResolverFactory {
@@ -174,15 +174,12 @@ class UnixResolverFactory : public ResolverFactory {
 
 void grpc_resolver_sockaddr_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-          grpc_core::New<grpc_core::IPv4ResolverFactory>()));
+      grpc_core::MakeUnique<grpc_core::IPv4ResolverFactory>());
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-          grpc_core::New<grpc_core::IPv6ResolverFactory>()));
+      grpc_core::MakeUnique<grpc_core::IPv6ResolverFactory>());
 #ifdef GRPC_HAVE_UNIX_SOCKET
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-          grpc_core::New<grpc_core::UnixResolverFactory>()));
+      grpc_core::MakeUnique<grpc_core::UnixResolverFactory>());
 #endif
 }
 

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -70,7 +70,7 @@ class XdsResolverFactory : public ResolverFactory {
 
   OrphanablePtr<Resolver> CreateResolver(ResolverArgs args) const override {
     if (!IsValidUri(args.uri)) return nullptr;
-    return OrphanablePtr<Resolver>(New<XdsResolver>(std::move(args)));
+    return MakeOrphanable<XdsResolver>(std::move(args));
   }
 
   const char* scheme() const override { return "xds-experimental"; }
@@ -82,8 +82,7 @@ class XdsResolverFactory : public ResolverFactory {
 
 void grpc_resolver_xds_init() {
   grpc_core::ResolverRegistry::Builder::RegisterResolverFactory(
-      grpc_core::UniquePtr<grpc_core::ResolverFactory>(
-          grpc_core::New<grpc_core::XdsResolverFactory>()));
+      grpc_core::MakeUnique<grpc_core::XdsResolverFactory>());
 }
 
 void grpc_resolver_xds_shutdown() {}

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

@@ -53,9 +53,8 @@ size_t ClientChannelServiceConfigParser::ParserIndex() {
 }
 
 void ClientChannelServiceConfigParser::Register() {
-  g_client_channel_service_config_parser_index =
-      ServiceConfig::RegisterParser(UniquePtr<ServiceConfig::Parser>(
-          New<ClientChannelServiceConfigParser>()));
+  g_client_channel_service_config_parser_index = ServiceConfig::RegisterParser(
+      MakeUnique<ClientChannelServiceConfigParser>());
 }
 
 namespace {
@@ -439,10 +438,9 @@ ClientChannelServiceConfigParser::ParseGlobalParams(const grpc_json* json,
   *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel global parser",
                                          &error_list);
   if (*error == GRPC_ERROR_NONE) {
-    return UniquePtr<ServiceConfig::ParsedConfig>(
-        New<ClientChannelGlobalParsedConfig>(
-            std::move(parsed_lb_config), std::move(lb_policy_name),
-            retry_throttling, health_check_service_name));
+    return MakeUnique<ClientChannelGlobalParsedConfig>(
+        std::move(parsed_lb_config), std::move(lb_policy_name),
+        retry_throttling, health_check_service_name);
   }
   return nullptr;
 }
@@ -493,9 +491,8 @@ ClientChannelServiceConfigParser::ParsePerMethodParams(const grpc_json* json,
   }
   *error = GRPC_ERROR_CREATE_FROM_VECTOR("Client channel parser", &error_list);
   if (*error == GRPC_ERROR_NONE) {
-    return UniquePtr<ServiceConfig::ParsedConfig>(
-        New<ClientChannelMethodParsedConfig>(timeout, wait_for_ready,
-                                             std::move(retry_policy)));
+    return MakeUnique<ClientChannelMethodParsedConfig>(timeout, wait_for_ready,
+                                                       std::move(retry_policy));
   }
   return nullptr;
 }

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

@@ -187,16 +187,15 @@ ResolvingLoadBalancingPolicy::ResolvingLoadBalancingPolicy(
   GPR_ASSERT(process_resolver_result != nullptr);
   resolver_ = ResolverRegistry::CreateResolver(
       target_uri_.get(), args.args, interested_parties(), combiner(),
-      UniquePtr<Resolver::ResultHandler>(New<ResolverResultHandler>(Ref())));
+      MakeUnique<ResolverResultHandler>(Ref()));
   // Since the validity of args has been checked when create the channel,
   // CreateResolver() must return a non-null result.
   GPR_ASSERT(resolver_ != nullptr);
   if (GRPC_TRACE_FLAG_ENABLED(*tracer_)) {
     gpr_log(GPR_INFO, "resolving_lb=%p: starting name resolution", this);
   }
-  channel_control_helper()->UpdateState(
-      GRPC_CHANNEL_CONNECTING,
-      UniquePtr<SubchannelPicker>(New<QueuePicker>(Ref())));
+  channel_control_helper()->UpdateState(GRPC_CHANNEL_CONNECTING,
+                                        MakeUnique<QueuePicker>(Ref()));
   resolver_->StartLocked();
 }
 
@@ -262,7 +261,7 @@ void ResolvingLoadBalancingPolicy::OnResolverError(grpc_error* error) {
         "Resolver transient failure", &error, 1);
     channel_control_helper()->UpdateState(
         GRPC_CHANNEL_TRANSIENT_FAILURE,
-        UniquePtr<SubchannelPicker>(New<TransientFailurePicker>(state_error)));
+        MakeUnique<TransientFailurePicker>(state_error));
   }
   GRPC_ERROR_UNREF(error);
 }

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

@@ -1071,8 +1071,7 @@ bool Subchannel::PublishTransportLocked() {
   }
   // Start watching connected subchannel.
   connected_subchannel_->StartWatch(
-      pollset_set_, OrphanablePtr<grpc_core::ConnectivityStateWatcherInterface>(
-                        New<ConnectedSubchannelStateWatcher>(this)));
+      pollset_set_, MakeOrphanable<ConnectedSubchannelStateWatcher>(this));
   // Report initial state.
   SetConnectivityStateLocked(GRPC_CHANNEL_READY);
   return true;

src/core/ext/filters/message_size/message_size_filter.cc

@@ -88,13 +88,13 @@ UniquePtr<ServiceConfig::ParsedConfig> MessageSizeParser::ParsePerMethodParams(
     *error = GRPC_ERROR_CREATE_FROM_VECTOR("Message size parser", &error_list);
     return nullptr;
   }
-  return UniquePtr<ServiceConfig::ParsedConfig>(New<MessageSizeParsedConfig>(
-      max_request_message_bytes, max_response_message_bytes));
+  return MakeUnique<MessageSizeParsedConfig>(max_request_message_bytes,
+                                             max_response_message_bytes);
 }
 
 void MessageSizeParser::Register() {
-  g_message_size_parser_index = ServiceConfig::RegisterParser(
-      UniquePtr<ServiceConfig::Parser>(New<MessageSizeParser>()));
+  g_message_size_parser_index =
+      ServiceConfig::RegisterParser(MakeUnique<MessageSizeParser>());
 }
 
 size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; }

src/core/lib/gprpp/memory.h

@@ -33,10 +33,8 @@
 // Should not be used in new code.
 // TODO(juanlishen): Remove this macro, and instead comment that the public dtor
 // should not be used directly.
-#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE         \
-  template <typename _Delete_T, bool _Delete_can_be_null> \
-  friend void ::grpc_core::Delete(_Delete_T*);            \
-  template <typename _Delete_T>                           \
+#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \
+  template <typename _Delete_T>                   \
   friend void ::grpc_core::Delete(_Delete_T*);
 
 // Add this to a class that want to use New(), but has a private or
@@ -58,33 +56,28 @@ inline T* New(Args&&... args) {
 }
 
 // Alternative to delete, since we cannot use it (for fear of libstdc++)
-// We cannot add a default value for can_be_null, because they are used as
-// as friend template methods where we cannot define a default value.
-// Instead we simply define two variants, one with and one without the boolean
-// argument.
-template <typename T, bool can_be_null>
+template <typename T>
 inline void Delete(T* p) {
-  GPR_DEBUG_ASSERT(can_be_null || p != nullptr);
-  if (can_be_null && p == nullptr) return;
+  if (p == nullptr) return;
   p->~T();
   gpr_free(p);
 }
-template <typename T>
-inline void Delete(T* p) {
-  Delete<T, /*can_be_null=*/true>(p);
-}
 
-template <typename T>
 class DefaultDelete {
  public:
+  template <typename T>
   void operator()(T* p) {
-    // std::unique_ptr is gauranteed not to call the deleter
-    // if the pointer is nullptr.
-    Delete<T, /*can_be_null=*/false>(p);
+    // Delete() checks whether the value is null, but std::unique_ptr<> is
+    // gauranteed not to call the deleter if the pointer is nullptr
+    // (i.e., it already does this check for us), and we don't want to
+    // do the check twice.  So, instead of calling Delete() here, we
+    // manually call the object's dtor and free it.
+    p->~T();
+    gpr_free(p);
   }
 };
 
-template <typename T, typename Deleter = DefaultDelete<T>>
+template <typename T, typename Deleter = DefaultDelete>
 using UniquePtr = std::unique_ptr<T, Deleter>;
 
 template <typename T, typename... Args>

src/core/lib/gprpp/orphanable.h

@@ -59,13 +59,15 @@ class Orphanable {
   virtual ~Orphanable() {}
 };
 
-template <typename T>
 class OrphanableDelete {
  public:
-  void operator()(T* p) { p->Orphan(); }
+  template <typename T>
+  void operator()(T* p) {
+    p->Orphan();
+  }
 };
 
-template <typename T, typename Deleter = OrphanableDelete<T>>
+template <typename T, typename Deleter = OrphanableDelete>
 using OrphanablePtr = std::unique_ptr<T, Deleter>;
 
 template <typename T, typename... Args>

src/core/lib/security/transport/security_handshaker.cc

@@ -515,10 +515,10 @@ RefCountedPtr<Handshaker> SecurityHandshakerCreate(
 void SecurityRegisterHandshakerFactories() {
   HandshakerRegistry::RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_CLIENT,
-      UniquePtr<HandshakerFactory>(New<ClientSecurityHandshakerFactory>()));
+      MakeUnique<ClientSecurityHandshakerFactory>());
   HandshakerRegistry::RegisterHandshakerFactory(
       false /* at_start */, HANDSHAKER_SERVER,
-      UniquePtr<HandshakerFactory>(New<ServerSecurityHandshakerFactory>()));
+      MakeUnique<ServerSecurityHandshakerFactory>());
 }
 
 }  // namespace grpc_core

src/core/tsi/ssl/session_cache/ssl_session_boringssl.cc

@@ -49,8 +49,7 @@ class BoringSslCachedSession : public SslCachedSession {
 
 grpc_core::UniquePtr<SslCachedSession> SslCachedSession::Create(
     SslSessionPtr session) {
-  return grpc_core::UniquePtr<SslCachedSession>(
-      grpc_core::New<BoringSslCachedSession>(std::move(session)));
+  return grpc_core::MakeUnique<BoringSslCachedSession>(std::move(session));
 }
 
 }  // namespace tsi

src/core/tsi/ssl/session_cache/ssl_session_openssl.cc

@@ -67,8 +67,7 @@ class OpenSslCachedSession : public SslCachedSession {
 
 grpc_core::UniquePtr<SslCachedSession> SslCachedSession::Create(
     SslSessionPtr session) {
-  return grpc_core::UniquePtr<SslCachedSession>(
-      grpc_core::New<OpenSslCachedSession>(std::move(session)));
+  return grpc_core::MakeUnique<OpenSslCachedSession>(std::move(session));
 }
 
 }  // namespace tsi

test/core/client_channel/resolvers/dns_resolver_test.cc

@@ -35,12 +35,6 @@ class TestResultHandler : public grpc_core::Resolver::ResultHandler {
   void ReturnError(grpc_error* error) override {}
 };
 
-static grpc_core::UniquePtr<grpc_core::Resolver::ResultHandler>
-create_test_result_handler() {
-  return grpc_core::UniquePtr<grpc_core::Resolver::ResultHandler>(
-      grpc_core::New<TestResultHandler>());
-}
-
 static void test_succeeds(grpc_core::ResolverFactory* factory,
                           const char* string) {
   gpr_log(GPR_DEBUG, "test: '%s' should be valid for '%s'", string,
@@ -51,7 +45,7 @@ static void test_succeeds(grpc_core::ResolverFactory* factory,
   grpc_core::ResolverArgs args;
   args.uri = uri;
   args.combiner = g_combiner;
-  args.result_handler = create_test_result_handler();
+  args.result_handler = grpc_core::MakeUnique<TestResultHandler>();
   grpc_core::OrphanablePtr<grpc_core::Resolver> resolver =
       factory->CreateResolver(std::move(args));
   GPR_ASSERT(resolver != nullptr);
@@ -68,7 +62,7 @@ static void test_fails(grpc_core::ResolverFactory* factory,
   grpc_core::ResolverArgs args;
   args.uri = uri;
   args.combiner = g_combiner;
-  args.result_handler = create_test_result_handler();
+  args.result_handler = grpc_core::MakeUnique<TestResultHandler>();
   grpc_core::OrphanablePtr<grpc_core::Resolver> resolver =
       factory->CreateResolver(std::move(args));
   GPR_ASSERT(resolver == nullptr);

test/core/client_channel/resolvers/sockaddr_resolver_test.cc

@@ -47,9 +47,7 @@ static void test_succeeds(grpc_core::ResolverFactory* factory,
   grpc_core::ResolverArgs args;
   args.uri = uri;
   args.combiner = g_combiner;
-  args.result_handler =
-      grpc_core::UniquePtr<grpc_core::Resolver::ResultHandler>(
-          grpc_core::New<ResultHandler>());
+  args.result_handler = grpc_core::MakeUnique<ResultHandler>();
   grpc_core::OrphanablePtr<grpc_core::Resolver> resolver =
       factory->CreateResolver(std::move(args));
   GPR_ASSERT(resolver != nullptr);
@@ -70,9 +68,7 @@ static void test_fails(grpc_core::ResolverFactory* factory,
   grpc_core::ResolverArgs args;
   args.uri = uri;
   args.combiner = g_combiner;
-  args.result_handler =
-      grpc_core::UniquePtr<grpc_core::Resolver::ResultHandler>(
-          grpc_core::New<ResultHandler>());
+  args.result_handler = grpc_core::MakeUnique<ResultHandler>();
   grpc_core::OrphanablePtr<grpc_core::Resolver> resolver =
       factory->CreateResolver(std::move(args));
   GPR_ASSERT(resolver == nullptr);

test/core/client_channel/service_config_test.cc

@@ -60,8 +60,7 @@ class TestParser1 : public ServiceConfig::Parser {
               GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage());
           return nullptr;
         }
-        return UniquePtr<ServiceConfig::ParsedConfig>(
-            New<TestParsedConfig1>(value));
+        return MakeUnique<TestParsedConfig1>(value);
       }
     }
     return nullptr;
@@ -98,8 +97,7 @@ class TestParser2 : public ServiceConfig::Parser {
               GRPC_ERROR_CREATE_FROM_STATIC_STRING(InvalidValueErrorMessage());
           return nullptr;
         }
-        return UniquePtr<ServiceConfig::ParsedConfig>(
-            New<TestParsedConfig1>(value));
+        return MakeUnique<TestParsedConfig1>(value);
       }
     }
     return nullptr;
@@ -148,10 +146,8 @@ class ServiceConfigTest : public ::testing::Test {
   void SetUp() override {
     ServiceConfig::Shutdown();
     ServiceConfig::Init();
-    EXPECT_TRUE(ServiceConfig::RegisterParser(
-                    UniquePtr<ServiceConfig::Parser>(New<TestParser1>())) == 0);
-    EXPECT_TRUE(ServiceConfig::RegisterParser(
-                    UniquePtr<ServiceConfig::Parser>(New<TestParser2>())) == 1);
+    EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique<TestParser1>()) == 0);
+    EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique<TestParser2>()) == 1);
   }
 };
 
@@ -312,10 +308,8 @@ class ErroredParsersScopingTest : public ::testing::Test {
   void SetUp() override {
     ServiceConfig::Shutdown();
     ServiceConfig::Init();
-    EXPECT_TRUE(ServiceConfig::RegisterParser(
-                    UniquePtr<ServiceConfig::Parser>(New<ErrorParser>())) == 0);
-    EXPECT_TRUE(ServiceConfig::RegisterParser(
-                    UniquePtr<ServiceConfig::Parser>(New<ErrorParser>())) == 1);
+    EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique<ErrorParser>()) == 0);
+    EXPECT_TRUE(ServiceConfig::RegisterParser(MakeUnique<ErrorParser>()) == 1);
   }
 };
 
@@ -359,10 +353,9 @@ class ClientChannelParserTest : public ::testing::Test {
   void SetUp() override {
     ServiceConfig::Shutdown();
     ServiceConfig::Init();
-    EXPECT_TRUE(
-        ServiceConfig::RegisterParser(UniquePtr<ServiceConfig::Parser>(
-            New<grpc_core::internal::ClientChannelServiceConfigParser>())) ==
-        0);
+    EXPECT_TRUE(ServiceConfig::RegisterParser(
+                    MakeUnique<internal::ClientChannelServiceConfigParser>()) ==
+                0);
   }
 };
 
@@ -929,8 +922,8 @@ class MessageSizeParserTest : public ::testing::Test {
   void SetUp() override {
     ServiceConfig::Shutdown();
     ServiceConfig::Init();
-    EXPECT_TRUE(ServiceConfig::RegisterParser(UniquePtr<ServiceConfig::Parser>(
-                    New<MessageSizeParser>())) == 0);
+    EXPECT_TRUE(
+        ServiceConfig::RegisterParser(MakeUnique<MessageSizeParser>()) == 0);
   }
 };
 

test/core/handshake/readahead_handshaker_server_ssl.cc

@@ -81,7 +81,7 @@ int main(int argc, char* argv[]) {
   grpc_init();
   HandshakerRegistry::RegisterHandshakerFactory(
       true /* at_start */, HANDSHAKER_SERVER,
-      UniquePtr<HandshakerFactory>(New<ReadAheadHandshakerFactory>()));
+      MakeUnique<ReadAheadHandshakerFactory>());
   const char* full_alpn_list[] = {"grpc-exp", "h2"};
   GPR_ASSERT(server_ssl_test(full_alpn_list, 2, "grpc-exp"));
   grpc_shutdown_blocking();

test/core/surface/lame_client_test.cc

@@ -44,9 +44,7 @@ static void do_nothing(void* arg, grpc_error* error) {}
 void test_transport_op(grpc_channel* channel) {
   grpc_core::ExecCtx exec_ctx;
   grpc_transport_op* op = grpc_make_transport_op(nullptr);
-  op->start_connectivity_watch =
-      grpc_core::OrphanablePtr<grpc_core::ConnectivityStateWatcherInterface>(
-          grpc_core::New<Watcher>());
+  op->start_connectivity_watch = grpc_core::MakeOrphanable<Watcher>();
   grpc_channel_element* elem =
       grpc_channel_stack_element(grpc_channel_get_channel_stack(channel), 0);
   elem->filter->start_transport_op(elem, op);

test/core/transport/connectivity_state_test.cc

@@ -73,8 +73,7 @@ TEST(StateTracker, NotificationUponAddingWatcher) {
   grpc_connectivity_state state = GRPC_CHANNEL_IDLE;
   ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_CONNECTING);
   tracker.AddWatcher(GRPC_CHANNEL_IDLE,
-                     OrphanablePtr<ConnectivityStateWatcherInterface>(
-                         New<Watcher>(&count, &state)));
+                     MakeOrphanable<Watcher>(&count, &state));
   EXPECT_EQ(count, 1);
   EXPECT_EQ(state, GRPC_CHANNEL_CONNECTING);
 }
@@ -84,8 +83,7 @@ TEST(StateTracker, NotificationUponStateChange) {
   grpc_connectivity_state state = GRPC_CHANNEL_IDLE;
   ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_IDLE);
   tracker.AddWatcher(GRPC_CHANNEL_IDLE,
-                     OrphanablePtr<ConnectivityStateWatcherInterface>(
-                         New<Watcher>(&count, &state)));
+                     MakeOrphanable<Watcher>(&count, &state));
   EXPECT_EQ(count, 0);
   EXPECT_EQ(state, GRPC_CHANNEL_IDLE);
   tracker.SetState(GRPC_CHANNEL_CONNECTING, "whee");
@@ -153,8 +151,7 @@ TEST(StateTracker, NotifyShutdownAtDestruction) {
   {
     ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_IDLE);
     tracker.AddWatcher(GRPC_CHANNEL_IDLE,
-                       OrphanablePtr<ConnectivityStateWatcherInterface>(
-                           New<Watcher>(&count, &state)));
+                       MakeOrphanable<Watcher>(&count, &state));
     // No initial notification, since we started the watch from the
     // current state.
     EXPECT_EQ(count, 0);
@@ -171,8 +168,7 @@ TEST(StateTracker, DoNotNotifyShutdownAtDestructionIfAlreadyInShutdown) {
   {
     ConnectivityStateTracker tracker("xxx", GRPC_CHANNEL_SHUTDOWN);
     tracker.AddWatcher(GRPC_CHANNEL_SHUTDOWN,
-                       OrphanablePtr<ConnectivityStateWatcherInterface>(
-                           New<Watcher>(&count, &state)));
+                       MakeOrphanable<Watcher>(&count, &state));
     // No initial notification, since we started the watch from the
     // current state.
     EXPECT_EQ(count, 0);

test/core/util/test_lb_policies.cc

@@ -219,9 +219,8 @@ class InterceptTrailingFactory : public LoadBalancingPolicyFactory {
 
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    return OrphanablePtr<LoadBalancingPolicy>(
-        New<InterceptRecvTrailingMetadataLoadBalancingPolicy>(std::move(args),
-                                                              cb_, user_data_));
+    return MakeOrphanable<InterceptRecvTrailingMetadataLoadBalancingPolicy>(
+        std::move(args), cb_, user_data_);
   }
 
   const char* name() const override {