Ensure that connection can be closed in response to a data message.

Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.

Committed: https://crrev.com/aed3bb654ae12f8670d17656fdbc6781d82ecfbb
Cr-Commit-Position: refs/heads/master@{#407635}

[Sergey Ulanov, 2016-07-22 23:29:52]

Patchset #1 (id:1) has been deleted

[Sergey Ulanov, 2016-07-22 23:30:02]

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

[Jamie, 2016-07-23 00:03:48]

lgtm

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
File remoting/protocol/connection_unittest.cc (right):

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
remoting/protocol/connection_unittest.cc:363:
.WillOnce(/*DoAll(InvokeWithoutArgs(this, &ConnectionTest::DestroyHost),*/
Remove the comment?

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/webrt...
File remoting/protocol/webrtc_transport.cc (right):

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_transport.cc:154: // on the stack and so it will be
destroyed later. We need to ensures that
s/ensures/ensure/

[Sergey Ulanov, 2016-07-23 06:30:04]

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
File remoting/protocol/connection_unittest.cc (right):

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
remoting/protocol/connection_unittest.cc:363:
.WillOnce(/*DoAll(InvokeWithoutArgs(this, &ConnectionTest::DestroyHost),*/
On 2016/07/23 00:03:48, Jamie wrote:
> Remove the comment?

Duh, this line is not supposed to be commented, so the test wasn't testing what
it's supposed to test. Uncommented now and also fixed two other issues that were
affected by this bug.

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/webrt...
File remoting/protocol/webrtc_transport.cc (right):

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/webrt...
remoting/protocol/webrtc_transport.cc:154: // on the stack and so it will be
destroyed later. We need to ensures that
On 2016/07/23 00:03:48, Jamie wrote:
> s/ensures/ensure/

reworded this comment

[Sergey Ulanov, 2016-07-23 06:30:36]

Description was changed from

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed an issue that was
causing a crash in that scenario.
==========

to

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing a crash in that scenario.
==========

[Sergey Ulanov, 2016-07-23 06:30:47]

Description was changed from

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing a crash in that scenario.
==========

to

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario.
==========

[Sergey Ulanov, 2016-07-23 06:31:33]

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
File remoting/protocol/connection_unittest.cc (right):

https://codereview.chromium.org/2172153005/diff/20001/remoting/protocol/conne...
remoting/protocol/connection_unittest.cc:363:
.WillOnce(/*DoAll(InvokeWithoutArgs(this, &ConnectionTest::DestroyHost),*/
On 2016/07/23 06:30:03, Sergey Ulanov wrote:
> On 2016/07/23 00:03:48, Jamie wrote:
> > Remove the comment?
> 
> Duh, this line is not supposed to be commented, so the test wasn't testing
what
> it's supposed to test. Uncommented now and also fixed two other issues that
were
> exposed by this test.

[Sergey Ulanov, 2016-07-23 06:32:06]

The CQ bit was checked by sergeyu@chromium.org

[Sergey Ulanov, 2016-07-23 06:32:07]

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/2172153005/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-07-23 06:32:16]

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, 2016-07-23 06:47:25]

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

[commit-bot: I haz the power, 2016-07-23 06:47:26]

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_...)

[Sergey Ulanov, 2016-07-25 21:29:28]

The CQ bit was checked by sergeyu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 21:30:17]

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

[Sergey Ulanov, 2016-07-25 21:31:55]

Jamie, PTAL,
There were more changes required in WebrtcTransport to make the new test pass
without breaking other tests.

[Sergey Ulanov, 2016-07-25 21:32:49]

Description was changed from

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario.
==========

to

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.
==========

[Jamie, 2016-07-25 21:42:37]

lgtm

[commit-bot: I haz the power, 2016-07-25 22:03:30]

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

[commit-bot: I haz the power, 2016-07-25 22:03:31]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2016-07-25 23:40:52]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2016-07-25 23:41:38]

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, 2016-07-25 23:45:58]

Description was changed from

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.
==========

to

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.
==========

[commit-bot: I haz the power, 2016-07-25 23:45:59]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-07-25 23:47:38]

Description was changed from

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.
==========

to

==========
Ensure that connection can be closed in response to a data message.

Added a unittest that verifies that it's safe to tear down connection
synchronously in response to a message. Also fixed issues that were
causing crashes in that scenario. Particularly added
PeerConnectionWrapper that wraps PeerConnection for WebrtcTransport
and allows closing it asynchronously.

Committed: https://crrev.com/aed3bb654ae12f8670d17656fdbc6781d82ecfbb
Cr-Commit-Position: refs/heads/master@{#407635}
==========

[commit-bot: I haz the power, 2016-07-25 23:47:40]

Patchset 3 (id:??) landed as
https://crrev.com/aed3bb654ae12f8670d17656fdbc6781d82ecfbb
Cr-Commit-Position: refs/heads/master@{#407635}