[Chromoting] Add SessionPlugin in JingleSession

remoting/protocol/session_plugin.h

+// Copyright 2016 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_PROTOCOL_SESSION_PLUGIN_H_
+#define REMOTING_PROTOCOL_SESSION_PLUGIN_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "remoting/protocol/jingle_messages.h"
+#include "remoting/protocol/session.h"
+#include "third_party/webrtc/libjingle/xmllite/xmlelement.h"
+
+namespace remoting {
+namespace protocol {
+
+// An interface to attach data on an outgoing message or handle an incoming of
+// message. The plugin data are designed to be optional, so this class won't
+// impact a session generation.
+class SessionPlugin {
+ public:
+  SessionPlugin();
+  virtual ~SessionPlugin();
+
+  // Attachs messages in |attachments|. This function will be called after other

[Jamie, 2016/12/20 23:29:21]

s/Attachs/Attaches/

[Hzj_jie, 2016/12/21 01:54:44]

Done.

+  // components have finished generating messages, and set the state and action.

[Jamie, 2016/12/20 23:29:21]

s/and set/and have set/

[Hzj_jie, 2016/12/21 01:54:44]

Done.

+  // So |state| is the current session state, |action| is the current ActionType
+  // in message.
+  virtual void OnSending(Session::State state,

[Jamie, 2016/12/20 23:29:21]

I think On[Incoming|Outgoing]Message would be better names for these, as you've
used elsewhere.

[Hzj_jie, 2016/12/21 01:54:44]

Done.

+                         JingleMessage::ActionType action,
+                         std::unique_ptr<buzz::XmlElement>* attachments) = 0;

[Jamie, 2016/12/20 23:29:21]

Passing a pointer to a unique_ptr is a bit strange; why not a raw pointer like
you used in ExecutePluginsOnOutgoingMessage? Or, if you believe that this is
only useful for adding attributes to the "attachments" field, maybe a better
design would be to return the new attributes, which would also mean you could
get rid of the NewAttachments method, and avoid duplicating the (admittedly
minor) code duplication where every implementation has to create that field if
it's null.

[Hzj_jie, 2016/12/21 01:54:44]

This function is a little bit different than ExecutePluginsOnOutgoingMessage(),
which accepts JingleMessage as parameter. This class is a SessionPlugin, it
should not depend on a JingleMessage. And in JingleMessage, if the attachments
field is an empty unique_ptr, the entire node will be ignored in ToXml()
function.
So here, the scenario becomes a little bit complex, we should not create an
attachments node before one of the plugins decides to insert something into the
attachments node.
Unfortunately XmlElement has no IsEmpty() or similar function to indicate
whether this node contains something really useful. Though FirstChild() /
FirstAttr() can be use, it looks a little bit weird to me to check whether an
XmlElement is empty by checking the first child or attribute.

Returning an XmlElement is a workable solution. I have updated the code. I used
to think by returning an XmlElement, one SessionPlugin cannot depend on the data
coming from another SessionPlugin. But we can still pass the data between two
SessionPlugins through other mechanism.

+
+  // Handles messages in |attachments|. This function will be called before
+  // other components have handled the incoming message. So |state| is the last
+  // session state, |action| is the current ActionType in message.
+  virtual void OnReceiving(
+      Session::State state,
+      JingleMessage::ActionType action,
+      const std::unique_ptr<buzz::XmlElement>& attachments) = 0;
+
+  // Finds "attachments" tag from |tag|, returns null if the tag is not found.
+  static const buzz::XmlElement* FindAttachments(const buzz::XmlElement& tag);
+
+  // Creates a new "attachments" tag.
+  static std::unique_ptr<buzz::XmlElement> NewAttachments();
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_SESSION_PLUGIN_H_