Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(539)

Issue 1258323003: Fix XmppSignalStrategy to return correct result from SendStanza(). (Closed)

Created:
5 years, 4 months ago by Sergey Ulanov
Modified:
5 years, 4 months ago
Reviewers:
dcaiafa
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@move_config
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix XmppSignalStrategy to return correct result from SendStanza(). Previously XmppSignalStrategy::SendStanza() was returning true even after write has failed on the socket. Failed socket write causes the XMPP connection to be closed. When sending host offline reason that results in HeartbeatSender being destroyed together with the IqSender that was used to send the message, which was causing the crash in the linked bug. Now IqSender::SendIq() can handle properly the case when SignalStrategy::SendStanza() fails. BUG=513951 Committed: https://crrev.com/f2382d91b38e9ea7469e3a9da289bb4e6e25cad8 Cr-Commit-Position: refs/heads/master@{#341243}

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -8 lines) Patch
M remoting/signaling/signal_strategy.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/signaling/xmpp_signal_strategy.cc View 2 chunks +6 lines, -3 lines 0 comments Download
M remoting/signaling/xmpp_signal_strategy_unittest.cc View 6 chunks +30 lines, -4 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
Sergey Ulanov
5 years, 4 months ago (2015-07-28 23:54:09 UTC) #3
dcaiafa
lgtm
5 years, 4 months ago (2015-07-29 23:09:04 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1258323003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1258323003/20001
5 years, 4 months ago (2015-07-30 01:37:24 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/33760)
5 years, 4 months ago (2015-07-30 02:36:45 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1258323003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1258323003/20001
5 years, 4 months ago (2015-07-31 00:00:55 UTC) #10
commit-bot: I haz the power
Committed patchset #1 (id:20001)
5 years, 4 months ago (2015-07-31 00:35:38 UTC) #11
commit-bot: I haz the power
5 years, 4 months ago (2015-07-31 00:36:19 UTC) #12
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/f2382d91b38e9ea7469e3a9da289bb4e6e25cad8
Cr-Commit-Position: refs/heads/master@{#341243}

Powered by Google App Engine
This is Rietveld 408576698