Remove DCHECK from HeartbeatSender::OnSignallingDisconnected()

Remove DCHECK from HeartbeatSender::OnSignallingDisconnected()

Allow that method to be called without a corresponding call to
OnSignalConnected() - the linked bug-report explains how this can happen.

This is only a partial bug-fix; still need to address the problem of this
method being called on the wrong thread from ChromotingHost::Shutdown().

BUG=83788
TEST=Manual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91737

[Wez, 2011-07-06 22:45:15]

Drive-by...

http://codereview.chromium.org/7307031/diff/1/remoting/host/heartbeat_sender.cc
File remoting/host/heartbeat_sender.cc (right):

http://codereview.chromium.org/7307031/diff/1/remoting/host/heartbeat_sender....
remoting/host/heartbeat_sender.cc:86: request_.reset(NULL);
This change means that the heart-beat request only gets torn-down/cancelled if
the sender ever reached the STARTED state; why not tear it down regardless? 
Similarly, we're only setting the state to STOPPED iff it was previously
STARTED.

Why not just change the DCHECK on |state_| to test for INITIALIZED or STARTED
rather than just STARTED?

[Lambros, 2011-07-06 23:13:00]

On 2011/07/06 22:45:15, Wez wrote:
> Drive-by...
> 
>
http://codereview.chromium.org/7307031/diff/1/remoting/host/heartbeat_sender.cc
> File remoting/host/heartbeat_sender.cc (right):
> 
>
http://codereview.chromium.org/7307031/diff/1/remoting/host/heartbeat_sender....
> remoting/host/heartbeat_sender.cc:86: request_.reset(NULL);
> This change means that the heart-beat request only gets torn-down/cancelled if
> the sender ever reached the STARTED state; why not tear it down regardless? 
> Similarly, we're only setting the state to STOPPED iff it was previously
> STARTED.
> 
> Why not just change the DCHECK on |state_| to test for INITIALIZED or STARTED
> rather than just STARTED?
I still favor removing the DCHECK completely.  In my test scenario, I think the
OnDisconnect() handler is being called twice - once from when the XMPP
connection closes, and then from ChromotingHost::Shutdown().

[Sergey Ulanov, 2011-07-06 23:17:28]

LGTM

[Wez, 2011-07-06 23:43:15]

On 2011/07/06 23:13:00, Lambros wrote:
> I still favor removing the DCHECK completely.  In my test scenario, I think
the
> OnDisconnect() handler is being called twice - once from when the XMPP
> connection closes, and then from ChromotingHost::Shutdown().

That sounds like faulty calling-code. :(

LGTM for this change, nonetheless, though.

[commit-bot: I haz the power, 2011-07-07 19:00:28]

Change committed as 91737