Fix crash when user disconnects It2Me host

Fix crash when user disconnects It2Me host

Cause:
The delegating_signal_strategy_ pointer is destroyed on the network
thread while It2meNativeMessagingHost keeps a raw pointer to it.

Fix:
DelegatingSignalStrategy will expose a callback that internally binds
weak pointer.  This will allow the callback to be invoked safely from
any thread even after the instance is destroyed.

BUG=700152
Review-Url: https://codereview.chromium.org/2747743004
Cr-Commit-Position: refs/heads/master@{#457288}
Committed: https://chromium.googlesource.com/chromium/src/+/4429e92d02a964039db7ef88c6676cb8ce20fc56

[kelvinp, 2017-03-14 00:56:31]

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

[kelvinp, 2017-03-14 00:56:32]

PTAL

[Sergey Ulanov, 2017-03-14 19:20:59]

LGTM when my style comments are addressed.

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
File remoting/host/it2me/it2me_native_messaging_host.cc (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
remoting/host/it2me/it2me_native_messaging_host.cc:299:
DelegatingSignalStrategy* delegating_signal_strategy =
Use unique_ptr<> please:

  auto delegating_signal_strategy =
      base::MakeUnique<DelegatingSignalStrategy>(..);

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
remoting/host/it2me/it2me_native_messaging_host.cc:306:
signal_strategy.reset(delegating_signal_strategy);
signal_strategy = std::move(delegating_signal_strategy);

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
File remoting/signaling/delegating_signal_strategy.cc (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.cc:30: return
base::Bind(&OnIncomingMessageFromDelegate, weak_factory_.GetWeakPtr(),
use base::BindRepeating() please

(base::Bind is being replaced with BindOnce and BindRepeating)

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.cc:41: FROM_HERE,
base::Bind(&DelegatingSignalStrategy::OnIncomingMessage,
BindOnce

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
File remoting/signaling/delegating_signal_strategy.h (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.h:34: typedef
base::Callback<void(const std::string&)> IqCallback;
Please use base::RepeatingCallback instead of base::Callback

(Callback is being replaced with OnceCallback and RepeatingCallback. There are
some details
onhttps://chromium.googlesource.com/chromium/src/+/master/docs/callback.md)

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.h:42: IqCallback
MakeIncomingMessageCallback();
s/Make/Get/

(potentially this function could just return a callback it already has, it
doesn't need to "make" a new callback every time)

[kelvinp, 2017-03-15 21:56:46]

FYI

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
File remoting/host/it2me/it2me_native_messaging_host.cc (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
remoting/host/it2me/it2me_native_messaging_host.cc:299:
DelegatingSignalStrategy* delegating_signal_strategy =
On 2017/03/14 19:20:58, Sergey Ulanov wrote:
> Use unique_ptr<> please:
> 
>   auto delegating_signal_strategy =
>       base::MakeUnique<DelegatingSignalStrategy>(..);
> 

Done.

https://codereview.chromium.org/2747743004/diff/1/remoting/host/it2me/it2me_n...
remoting/host/it2me/it2me_native_messaging_host.cc:306:
signal_strategy.reset(delegating_signal_strategy);
On 2017/03/14 19:20:58, Sergey Ulanov wrote:
> signal_strategy = std::move(delegating_signal_strategy);

Done.

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
File remoting/signaling/delegating_signal_strategy.cc (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.cc:30: return
base::Bind(&OnIncomingMessageFromDelegate, weak_factory_.GetWeakPtr(),
On 2017/03/14 19:20:59, Sergey Ulanov wrote:
> use base::BindRepeating() please
> 
> (base::Bind is being replaced with BindOnce and BindRepeating)

Done.

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.cc:41: FROM_HERE,
base::Bind(&DelegatingSignalStrategy::OnIncomingMessage,
On 2017/03/14 19:20:59, Sergey Ulanov wrote:
> BindOnce

We can't migrate yet as PostTask() expects a Closure which is still typedef to
RepeatingCallback.

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
File remoting/signaling/delegating_signal_strategy.h (right):

https://codereview.chromium.org/2747743004/diff/1/remoting/signaling/delegati...
remoting/signaling/delegating_signal_strategy.h:34: typedef
base::Callback<void(const std::string&)> IqCallback;
On 2017/03/14 19:20:59, Sergey Ulanov wrote:
> Please use base::RepeatingCallback instead of base::Callback
> 
> (Callback is being replaced with OnceCallback and RepeatingCallback. There are
> some details
> onhttps://chromium.googlesource.com/chromium/src/+/master/docs/callback.md)

Done.

[kelvinp, 2017-03-15 21:56:47]

FYI

[kelvinp, 2017-03-15 21:56:53]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2017-03-15 21:56:53]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2747743004/#ps20001
(title: "Reviewer's feedback")

[commit-bot: I haz the power, 2017-03-15 21:57:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 22:12:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 22:12:02]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kelvinp, 2017-03-16 00:08:53]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2017-03-16 00:08:54]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2747743004/#ps40001
(title: "Fix Client breakage")

[commit-bot: I haz the power, 2017-03-16 00:09:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 00:43:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489622933807330,
"parent_rev": "704aef7d3e590faa7af12dc6e9a20fbf947906d8", "commit_rev":
"4429e92d02a964039db7ef88c6676cb8ce20fc56"}

[commit-bot: I haz the power, 2017-03-16 00:44:22]

Description was changed from

==========
Fix crash when user disconnects It2Me host

Cause:
The delegating_signal_strategy_ pointer is destroyed on the network
thread while It2meNativeMessagingHost keeps a raw pointer to it.

Fix:
DelegatingSignalStrategy will expose a callback that internally binds
weak pointer.  This will allow the callback to be invoked safely from
any thread even after the instance is destroyed.

BUG=700152
==========

to

==========
Fix crash when user disconnects It2Me host

Cause:
The delegating_signal_strategy_ pointer is destroyed on the network
thread while It2meNativeMessagingHost keeps a raw pointer to it.

Fix:
DelegatingSignalStrategy will expose a callback that internally binds
weak pointer.  This will allow the callback to be invoked safely from
any thread even after the instance is destroyed.

BUG=700152

Review-Url: https://codereview.chromium.org/2747743004
Cr-Commit-Position: refs/heads/master@{#457288}
Committed:
https://chromium.googlesource.com/chromium/src/+/4429e92d02a964039db7ef88c667...
==========

[commit-bot: I haz the power, 2017-03-16 00:44:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/4429e92d02a964039db7ef88c667...