Refactor SignalStrategy so that it can be reused for multiple connections.

remoting/jingle_glue/signal_strategy.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.
 
 #ifndef REMOTING_JINGLE_GLUE_SIGNAL_STRATEGY_H_
 #define REMOTING_JINGLE_GLUE_SIGNAL_STRATEGY_H_
 
 #include <string>
 
 #include "base/basictypes.h"
 
 namespace buzz {
 class XmlElement;
 }  // namespace buzz
 
 namespace remoting {
 
 class SignalStrategy {
  public:
-  class StatusObserver {
-   public:
-    enum State {
-      START,
-      CONNECTING,
-      CONNECTED,
-      CLOSED,
-    };
+  enum State {
+    // Connection is being established.
+    CONNECTING,
 
-    // Called when state of the connection is changed.
-    virtual void OnStateChange(State state) = 0;
-    virtual void OnJidChange(const std::string& full_jid) = 0;
+    // Signalling is connected.
+    CONNECTED,
+
+    // Connection is closed due to an error or because Disconnect()
+    // was called.
+    CLOSED,

[Wez, 2011/12/21 23:35:30]

CONNECTED -> DISCONNECTED, or OPEN -> CLOSED

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

   };
 
   class Listener {
    public:
+    virtual ~Listener() {}
+
+    // Called after state of the connection has changed.
+    virtual void OnSignalStrategyStateChange(State state) = 0;

[Wez, 2011/12/21 23:35:30]

Is it safe to tear-down the SignallingStrategy from this callback?

[Wez, 2011/12/21 23:35:30]

Consider giving this a default empty implementation, since a number of callers
don't need to hook it.

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

+
     // Must return true if the stanza was handled, false otherwise.
-    virtual bool OnIncomingStanza(const buzz::XmlElement* stanza) = 0;
+    virtual bool OnSignalStrategyIncomingStanza(

[Wez, 2011/12/21 23:35:30]

Same here?

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

+        const buzz::XmlElement* stanza) = 0;
   };
 
   SignalStrategy() {}
   virtual ~SignalStrategy() {}
-  virtual void Init(StatusObserver* observer) = 0;
-  virtual void Close() = 0;
+
+  // Starts connection attempt. If connection is currently active
+  // disconnects it and opens a new connection. Connection is finished

[Wez, 2011/12/21 23:35:30]

Will the implicit disconnect trigger a CLOSED notification?

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

+  // asynchronously.
+  virtual void Connect() = 0;
+
+  // Disconnects current connection.
+  virtual void Disconnect() = 0;

[Wez, 2011/12/21 23:35:30]

Will the Listeners see CLOSED notifications?

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

+
+  // Returns current state.
+  virtual State GetState() const = 0;
+
+  // Returns local JID or an empty string when not connected.
+  virtual std::string GetLocalJid() const = 0;

[Wez, 2011/12/21 23:35:30]

nit: GetLocalEndpointId?  This interface is notionally not specific to the Talk
transport.

[Sergey Ulanov, 2011/12/22 21:45:10]

This interface is specific to XMPP signalling messages (though it may be
implemented with different types of XMPP transport), and XMPP uses JIDs to
identify endpoints.

 
   // Add a |listener| that can listen to all incoming
   // messages. Doesn't take ownership of the |listener|.

[Wez, 2011/12/21 23:35:30]

Do Listeners need removing before teardown?

[Sergey Ulanov, 2011/12/22 21:45:10]

Done.

   virtual void AddListener(Listener* listener) = 0;
 
   // Remove a |listener| previously added with AddListener().
   virtual void RemoveListener(Listener* listener) = 0;
 
   // Sends a raw XMPP stanza. Takes ownership of the |stanza|.
   virtual bool SendStanza(buzz::XmlElement* stanza) = 0;
 
   // Returns new ID that should be used for the next outgoing IQ
   // request.
   virtual std::string GetNextId() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SignalStrategy);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_JINGLE_GLUE_SIGNAL_STRATEGY_H_