Move the routing-info inside the jingle element.

Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.

BUG=618143
Committed: https://crrev.com/046534d38813b82d7f3bcb61b7f8c2fdc7f9cc74
Cr-Commit-Position: refs/heads/master@{#398383}

[kelvinp, 2016-06-03 22:23:29]

kelvinp@chromium.org changed reviewers:
+ sergeyu@chromium.org

[kelvinp, 2016-06-03 22:23:29]

PTAL

[kelvinp, 2016-06-03 22:24:03]

Description was changed from

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route the
error message back to the client.
==========

to

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.
==========

[Sergey Ulanov, 2016-06-04 17:10:31]

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
File remoting/protocol/jingle_messages.cc (right):

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages.cc:243: XmlElement* jingle =
EnsureJingleTag(iq);
I suggest passing a pointer to <jingle> explicitly as an argument for
SetAddress(). And then change JingleMessageReply to create <jingle> explicitly.

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages.cc:244:
jingle->ClearAttr(GetQNameByField(Field::CHANNEL, from));
I this necessary? SetAddress() is always called with a newly created message, so
these attributes are never set as far as I can tell

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
File remoting/protocol/jingle_messages_unittest.cc (right):

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages_unittest.cc:509:
"to-endpoint-id='from@gmail.com/AbCdEf1234='></jingle></iq>",
Nit: replace "></jingle>" with "/>"

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages_unittest.cc:518: LOG(INFO) << "Running test
case: " << i;
To avoid spamming the console, replace this with SCOPED_TRACE(i);

[kelvinp, 2016-06-06 20:43:41]

PTAL

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
File remoting/protocol/jingle_messages.cc (right):

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages.cc:243: XmlElement* jingle =
EnsureJingleTag(iq);
On 2016/06/04 17:10:31, Sergey Ulanov wrote:
> I suggest passing a pointer to <jingle> explicitly as an argument for
> SetAddress(). And then change JingleMessageReply to create <jingle>
explicitly.

Done.

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages.cc:244:
jingle->ClearAttr(GetQNameByField(Field::CHANNEL, from));
On 2016/06/04 17:10:31, Sergey Ulanov wrote:
> I this necessary? SetAddress() is always called with a newly created message,
so
> these attributes are never set as far as I can tell

This is true.  For the sake of future maintenance, I think SetAddress should
always clear previously set address and return a message with the correct
address representation regardless of what happens a priori.

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
File remoting/protocol/jingle_messages_unittest.cc (right):

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages_unittest.cc:509:
"to-endpoint-id='from@gmail.com/AbCdEf1234='></jingle></iq>",
On 2016/06/04 17:10:31, Sergey Ulanov wrote:
> Nit: replace "></jingle>" with "/>"

Done.

https://codereview.chromium.org/2042513002/diff/1/remoting/protocol/jingle_me...
remoting/protocol/jingle_messages_unittest.cc:518: LOG(INFO) << "Running test
case: " << i;
On 2016/06/04 17:10:31, Sergey Ulanov wrote:
> To avoid spamming the console, replace this with SCOPED_TRACE(i);

Done.

[Sergey Ulanov, 2016-06-07 16:10:14]

lgtm

[kelvinp, 2016-06-07 20:13:54]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2016-06-07 20:32:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2042513002/20001

[commit-bot: I haz the power, 2016-06-07 21:38:00]

Description was changed from

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.
==========

to

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.
==========

[commit-bot: I haz the power, 2016-06-07 21:38:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-07 21:39:37]

Description was changed from

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.
==========

to

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.

Committed: https://crrev.com/046534d38813b82d7f3bcb61b7f8c2fdc7f9cc74
Cr-Commit-Position: refs/heads/master@{#398383}
==========

[commit-bot: I haz the power, 2016-06-07 21:39:38]

Patchset 2 (id:??) landed as
https://crrev.com/046534d38813b82d7f3bcb61b7f8c2fdc7f9cc74
Cr-Commit-Position: refs/heads/master@{#398383}

[kelvinp, 2016-06-07 23:44:40]

Description was changed from

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.

Committed: https://crrev.com/046534d38813b82d7f3bcb61b7f8c2fdc7f9cc74
Cr-Commit-Position: refs/heads/master@{#398383}
==========

to

==========
Move the routing-info inside the jingle element.

Previously, routing info are encoded in the iq node.  When the host is
offline, the talk network responds with an error IQ that doesn't get
relayed to the client because it doesn't have the routing info that the
signaling service needs.

Since an error IQ always contain the original request, by moving the
routing info inside the original jingle request, we can reliably route
the error message back to the client.

BUG=618143

Committed: https://crrev.com/046534d38813b82d7f3bcb61b7f8c2fdc7f9cc74
Cr-Commit-Position: refs/heads/master@{#398383}
==========