Add separate nonce version of connect call.

remoting/client/plugin/chromoting_scriptable_object.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This implements the JavaScript class entrypoint for the plugin instance.
 // The Javascript API is defined as follows.
 //
 // interface ChromotingScriptableObject {
 //
 //   // Dimension of the desktop area.
 //   readonly attribute int desktopWidth;
 //   readonly attribute int desktopHeight;
 //
 //   // 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;
 //   // Latency for capturing in milliseconds.
 //   readonly attribute int videoCaptureLatency;
 //   // Latency for video encoding in milliseconds.
 //   readonly attribute int videoEncodeLatency;
 //   // Latency for video decoding in milliseconds.
 //   readonly attribute int videoDecodeLatency;
 //   // Latency for rendering in milliseconds.
@@ skipping 40 matching lines @@
 //   // User of this object should respond with calling submitLoginInfo() when
 //   // username and password is available.
 //   //
 //   // This function will be called multiple times until login was successful
 //   // or the maximum number of login attempts has been reached. In the
 //   // later case |connection_status| is changed to STATUS_FAILED.
 //   attribute Function loginChallenge;
 //
 //   // 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.
 //   void submitLoginInfo(string username, string password);
 //
 //   // Method for setting scale-to-fit.
 //   void setScaleToFit(bool scale_to_fit);
 // }
 
 #ifndef REMOTING_CLIENT_PLUGIN_CHROMOTING_SCRIPTABLE_OBJECT_H_
@@ skipping 93 matching lines @@
   void AddMethod(const std::string& name, MethodHandler handler);
 
   // This should be called to signal the JS code that the connection status has
   // changed.
   void SignalConnectionInfoChange();
 
   // Signal the JS code that the desktop size has changed.
   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.
   pp::Var DoSubmitLogin(const std::vector<pp::Var>& args, pp::Var* exception);
 
   // 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);
 
   PropertyNameMap property_names_;
   std::vector<PropertyDescriptor> properties_;
   scoped_refptr<PepperXmppProxy> xmpp_proxy_;
 
   ChromotingInstance* instance_;
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_CLIENT_PLUGIN_CHROMOTING_SCRIPTABLE_OBJECT_H_