Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1999443004:
Clear the channel and endpoint_id in SetAddress for the XMPP channel. (Closed)
DescriptionClear the channel and endpoint_id in SetAddress for the XMPP channel.
This will handle the case when a channel is mutated from LCS
to XMPP. The host does not have this use case yet so
this change is more for overall correctness and to match the
implementation on the server.
BUG=612588
Committed: https://crrev.com/dfa10498a47c3862fdbc13cbaa80b147d47d4523
Cr-Commit-Position: refs/heads/master@{#395106}
Patch Set 1 : #
Total comments: 1
Patch Set 2 : Reviewer's feedback #Messages
Total messages: 17 (11 generated)
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. BUG=612588 ==========
Patchset #2 (id:20001) has been deleted
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. BUG=612588 ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. We don't have such use case yet but just modifying the code here for overall correctness. BUG=612588 ==========
kelvinp@chromium.org changed reviewers: + jamiewalch@chromium.org
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. We don't have such use case yet but just modifying the code here for overall correctness. BUG=612588 ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet but so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ==========
Patchset #1 (id:1) has been deleted
PTAL
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet but so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ==========
lgtm https://codereview.chromium.org/1999443004/diff/40001/remoting/protocol/jingl... File remoting/protocol/jingle_messages.cc (right): https://codereview.chromium.org/1999443004/diff/40001/remoting/protocol/jingl... remoting/protocol/jingle_messages.cc:207: iqElement->ClearAttr(GetQNameByField(Field::ENDPOINT_ID, from)); I think it might be clearer to clear everything up-front with a comment explaining that you want to start from a fresh state, regardless of the previous address format. Then add in the components that make sense for the new format.
FYI
The CQ bit was checked by kelvinp@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jamiewalch@chromium.org Link to the patchset: https://codereview.chromium.org/1999443004/#ps60001 (title: "Reviewer's feedback")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1999443004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1999443004/60001
Message was sent while issue was closed.
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet so this change is more for overall correctness and to match the implementation on the server. BUG=612588 ========== to ========== Clear the channel and endpoint_id in SetAddress for the XMPP channel. This will handle the case when a channel is mutated from LCS to XMPP. The host does not have this use case yet so this change is more for overall correctness and to match the implementation on the server. BUG=612588 Committed: https://crrev.com/dfa10498a47c3862fdbc13cbaa80b147d47d4523 Cr-Commit-Position: refs/heads/master@{#395106} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/dfa10498a47c3862fdbc13cbaa80b147d47d4523 Cr-Commit-Position: refs/heads/master@{#395106} |
