The current implementations of xDS over gRPC and ADS are hard for someone new to the code to understand, or at least, hard for this particular new someone :)

Here is the situation: there is GrpcMuxImpl, GrpcSubscriptionImpl, GrpcMuxSubscriptionImpl, and GrpcManagedMuxSubscriptionImpl. GrpcSubscriptionImpl owns both a GrpcMuxImpl and a GrpcManagedMuxSubscriptionImpl. GrpcManagedMuxSubsciptionImpl holds a reference to the GrpcMuxImpl its owner owns, and also owns a GrpcMuxSubscriptionImpl.

What does someone who wants to write code that uses an xDS client, or ADS client, do? The answer is use GrpcSubscriptionImpl or GrpcMuxImpl, respectively, but I had a very hard time figuring that out, even after talking with people who already got it.

The Subscription interface sounds like it should be the conceptual attachment point for all users of xDS. That made me expect ADS would also operate through GrpcSubscriptionImpl: that you would create something like a GrpcMuxImpl as the single object actually communicating with the server, but then you would associate your Subscriptions to it and interact only with them. I was therefore confused when I realized that every newly created GrpcSubscriptionImpl would create a new gRPC stream.

The GrpcMux abstract interface has comments saying it can be used for either single xDS or ADS, but that's not quite true: single xDS uses GrpcSubscriptionImpl. That class is basically a wrapper around GrpcMux, but that's an implementation detail that should be hidden from the user. Since you wouldn't expect something called mux to need to be involved when you only want to work with a single thing, it's confusing to see this class suggested for xDS.

GrpcSubscriptionImpl is just a wrapper around GrpcMuxSubscriptionImpl. That is to mediate between two cases: whether the Subscription should own the GrpcMux (single xDS), or it should not (ADS). I don't think a wrapper class is the right way to do that. Rather, maybe GrpcSubscriptionImpl could have an additional absl::optional field, and the grpc_mux_ field that exists in both classes could be a reference in the new single class. (Or it could just be a shared_ptr; not sure if that would be considered justified in this case).

Adding comments to the implementations will certainly help a lot. However, I think an overhaul of the class names would also greatly help, along with the GrpcMuxSubscription refactor mentioned above. I opened this as an issue rather than just getting started on making changes so we could discuss 1) if the renames/refactor are a good idea, and if so 2) what exactly to rename to. To get things started, I propose
GrpcMuxImpl->AdsMuxImpl
GrpcSubscriptionImpl->XdsSubscriptionImpl
GrpcMuxSubscriptionImpl->deleted
GrpcManagedMuxSubsciptionImpl->not yet sure, but I suspect it can be pulled into XdsSubscriptionImpl in the same way I'm proposing for GrpcMuxSubscriptionImpl.

Also, I wonder: could GrpcMuxCallbacks and SubscriptionCallbacks be merged? The only difference is that SubscriptionCallbacks' onConfigUpdate takes typed resource protos. What if we had only SubscriptionCallbacks, and ADS gave GrpcMux several of them, one for each type? I think they would fit naturally as a field of ApiState. Maybe this idea should be considered separately, though.

I know that is a lot to digest, but please let me know what you think!