PairingAuthenticator implementation and plumbing.

remoting/host/remoting_me2me_host.cc

Index: remoting/host/remoting_me2me_host.cc
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 6ee42b9ff8d15d1e8cc16b47949b21f3d2e84cd5..f6e4ee9459930798b988d7b505decd10df9d1d34 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -67,6 +67,7 @@
 #include "remoting/host/usage_stats_consent.h"
 #include "remoting/jingle_glue/xmpp_signal_strategy.h"
 #include "remoting/protocol/me2me_host_authenticator_factory.h"
+#include "remoting/protocol/pairing_registry.h"
 
 #if defined(OS_POSIX)
 #include <signal.h>
@@ -465,9 +466,13 @@ void HostProcess::CreateAuthenticatorFactory() {
   }
   scoped_ptr<protocol::AuthenticatorFactory> factory;
 
+  // TODO(jamiewalch): Add a proper pairing registry here once all the code
+  // is committed.
+  scoped_refptr<remoting::protocol::PairingRegistry> pairing_registry;

[rmsousa, 2013/05/15 01:25:16]

Nit: Please either pass NULL explicitly (and move the comment down), or create
an actual (possibly dummy) instance. Passing an implicitly null pointer is a bit
harder to read.

(Note, not necessarily related to this CL: while looking at whether it was
possible to create a dummy instance, I noticed that
NotImplementedPairingRegistryDelegate is a bit too literal :) - please take the
opportunity to either remove the declaration or add a definition for ::Save). 

[Jamie, 2013/05/15 23:41:08]

It says quite clearly that it's not implemented. What more do you want? ;)

I've actually already fixed this in https://codereview.chromium.org/14995019/.
Regarding passing NULL, the reason I'm doing that for now is that I don't want
the UI to present the option of pairing until it's fully implemented. The
simplest way of doing that is by not creating the registry. It will also make it
easy to support a policy to disable pairing. Creating a dummy implementation
doesn't really make sense because it's not an interface. Even if I made it into
an interface, it would still need something along the lines of an IsImplemented
method in order to prevent pairing from being offered. This seems like overkill
to me.

I've added a comment to NegotiatingHostAuthenticator::CreateWithSharedSecret to
explain.

[rmsousa, 2013/05/16 00:46:25]

I see, I agree passing NULL is the right move in this case. But since that's the
case, [nit:] I think actually passing NULL in the parameter list makes the
intention more readable.

[Jamie, 2013/05/16 19:30:16]

The reason I made it separate is that the pairing registry is just one of many
parameters, so a comment above the CreateWithSharedSecret wouldn't be as clear.
If you still think I should change it, I don't mind, but I think this approach
is clearer.

[rmsousa, 2013/05/16 20:11:02]

I see what you mean, good point.

+
   if (token_url_.is_empty() && token_validation_url_.is_empty()) {
     factory = protocol::Me2MeHostAuthenticatorFactory::CreateWithSharedSecret(
-        local_certificate, key_pair_, host_secret_hash_);
+        local_certificate, key_pair_, host_secret_hash_, pairing_registry);
   } else if (token_url_.is_valid() && token_validation_url_.is_valid()) {
     scoped_ptr<protocol::ThirdPartyHostAuthenticator::TokenValidatorFactory>
         token_validator_factory(new TokenValidatorFactoryImpl(