Add separate nonce version of connect call.

remoting/client/plugin/chromoting_scriptable_object.h

Index: remoting/client/plugin/chromoting_scriptable_object.h
diff --git a/remoting/client/plugin/chromoting_scriptable_object.h b/remoting/client/plugin/chromoting_scriptable_object.h
index 8ee2d12ff8d047e10415ed426365aa9fb05ea5c4..ef7d5e1e178b626af2e52245ac4cdb22ad943018 100644
--- a/remoting/client/plugin/chromoting_scriptable_object.h
+++ b/remoting/client/plugin/chromoting_scriptable_object.h
@@ -14,6 +14,11 @@
 //   // Connection status.
 //   readonly attribute unsigned short status;
 //
+//   // Sandbox enabled.
+//   // TODO(garykac): Remove this once we no longer need non-sandbox for
+//   // debugging.
+//   readonly attribute bool sandboxed;

[Jamie, 2011/05/20 22:07:58]

I don't think I like this. An attribute would make sense if it controlled the
behaviour of the connect function without needing to supply different arguments.
However, in this case, I have to set sandboxed and then call an appropriate
version of connect. If I'm going to have to call a different version of connect
anyway, why have the attribute?

[Wez, 2011/05/20 22:38:59]

The aim was to have the attribute control whether or not we make a sandboxed
connection, and then have a single "connect" call whose arguments differ
depending on the auth scheme the app uses.

[garykac, 2011/05/23 16:58:06]

I agree with Jamie here. Having a separate signature depending on whether some
global is set doesn't feel right. This is something that I hadn't considered
when we talked about moving 'sandboxed' into an attribute.

+//
 //   // Statistics.
 //   // Video Bandwidth in bytes per second.
 //   readonly attribute float videoBandwidth;
@@ -74,12 +79,24 @@
 //
 //   // Methods for establishing a Chromoting connection.
 //   //
-//   // Either use connect() or connectSandboxed(), not both. If using
-//   // connectSandboxed(), sendIq must be set, and responses to calls on
-//   // sendIq must be piped back into onIq().
-//   void connect(string username, string host_jid, string auth_token,
-//                string nonce);
-//   void connectSandboxed(string your_jid, string host_jid, string nonce);
+//   // When using the sandboxed versions, sendIq must be set and responses to
+//   // calls on sendIq must be piped back into onIq().
+//   // TODO(garykac): Remove the non-sandboxed versions once we no longer
+//   // need them for debugging/testing.
+//   //
+//   // Sandboxed. auth_type="AUTHENTICATE" (me2me)
+//   void connect(string host_jid, string auth_type, string client_jid);
+//   // Sandboxed. auth_type="ACCESS_CODE" (me2mom)
+//   void connect(string host_jid, string auth_type, string client_jid,
+//                string access_code);
+//   // Not-sandboxed. auth_type="AUTHENTICATE" (me2me)
+//   void connect(string host_jid, string auth_type, string username,
+//                string xmpp_token);
+//   // Not-sandboxed. auth_type="ACCESS_CODE" (me2mom)
+//   void connect(string host_jid, string auth_type, string username,
+//                string xmpp_token, string access_code);
+//

[Jamie, 2011/05/20 22:07:58]

From what I can see, the only difference between me2me and me2mom is that in
me2mom we pass an access code. Can we make that parameter optional and go back
to two methods for the connect (we call the one we're getting rid of
connectUnsandboxed if we want, so that the correct approach has the more
sensible name). This seems cleaner to me, and means we can get rid of the
sandboxed attribute and the auth_type parameter.

[Wez, 2011/05/20 22:38:59]

I'd prefer a single connect() API with "name=value" pair strings, or a connect()
call that gets passed a JS dictionary to pull values from, I think.

[Jamie, 2011/05/23 16:50:18]

I think we should make the sandboxed API look the way we ultimately want it to
so that it doesn't have to change when we remove the unsandboxed version. I
don't particularly care what the unsandboxed API looks like as it's only
temporary. I think I'd go with:

connect(string host_jid, string client_jid, optional string access_code);
connectUnsandboxed(string host_jid, string username,
                   string xmpp_token, optional string access_code);

[garykac, 2011/05/23 16:58:06]

I agree with this.

+//   // Terminating a Chromoting connection.
 //   void disconnect();
 //
 //   // Method for submitting login information.
@@ -193,8 +210,6 @@ class ChromotingScriptableObject
   void SignalDesktopSizeChange();
 
   pp::Var DoConnect(const std::vector<pp::Var>& args, pp::Var* exception);
-  pp::Var DoConnectSandboxed(const std::vector<pp::Var>& args,
-                             pp::Var* exception);
   pp::Var DoDisconnect(const std::vector<pp::Var>& args, pp::Var* exception);
 
   // This method is called by JS to provide login information.
@@ -203,6 +218,12 @@ class ChromotingScriptableObject
   // This method is called by JS to set scale-to-fit.
   pp::Var DoSetScaleToFit(const std::vector<pp::Var>& args, pp::Var* exception);
 
+  // This method is called by JS to set if we should connect from within a
+  // sandbox. By default, all connections are made from within a sandbox, so
+  // this is useful to turn off the sandbox for debugging or testing.
+  // TODO(garykac): Remove this once it is no longer needed for debug/test.
+  pp::Var DoSetSandboxed(const std::vector<pp::Var>& args, pp::Var* exception);
+
   // This method is caleld by Javascript to provide responses to sendIq()
   // requests when establishing a sandboxed Chromoting connection.
   pp::Var DoOnIq(const std::vector<pp::Var>& args, pp::Var* exception);