[Chromoting] Drop unordered messages instead of crashing

[Chromoting] Drop unordered messages instead of crashing

This issue happens on my laptop as well as a virtual machine, which indicates a
broader impact. Though the reason of receiving unordered messages is still
unknown, fixing it from the host side is still useful.
Loggically speaking this change does not impact the behavior in release build:
instead of wasting some memory, we won't be able to retrieve the unordered
messages anyway.

BUG=694083

[Hzj_jie, 2017-05-26 19:15:55]

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

[commit-bot: I haz the power, 2017-05-26 19:16:49]

Dry run: CQ is trying da patch.

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

[Hzj_jie, 2017-05-26 19:17:01]

Description was changed from

==========
[Chromoting] Drop unordered message instead of crashing

This issue happens on my laptop instead of a virtual machine, which indicates a
broader impact. Though the reason of receiving unordered messages is still
unknown, fixing it from the host side is still useful.

BUG=694083
==========

to

==========
[Chromoting] Drop unordered messages instead of crashing

This issue happens on my laptop as well as a virtual machine, which indicates a
broader impact. Though the reason of receiving unordered messages is still
unknown, fixing it from the host side is still useful.

BUG=694083
==========

[Hzj_jie, 2017-05-26 19:17:01]

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

[commit-bot: I haz the power, 2017-05-26 19:24:50]

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

[commit-bot: I haz the power, 2017-05-26 19:24:51]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Hzj_jie, 2017-05-26 19:25:38]

Description was changed from

==========
[Chromoting] Drop unordered messages instead of crashing

This issue happens on my laptop as well as a virtual machine, which indicates a
broader impact. Though the reason of receiving unordered messages is still
unknown, fixing it from the host side is still useful.

BUG=694083
==========

to

==========
[Chromoting] Drop unordered messages instead of crashing

This issue happens on my laptop as well as a virtual machine, which indicates a
broader impact. Though the reason of receiving unordered messages is still
unknown, fixing it from the host side is still useful.
Loggically speaking this change does not impact the behavior in release build:
instead of wasting some memory, we won't be able to retrieve the unordered
messages anyway.

BUG=694083
==========

[Hzj_jie, 2017-05-26 19:26:04]

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

[commit-bot: I haz the power, 2017-05-26 19:26:46]

Dry run: 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-05-26 19:33:54]

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

[commit-bot: I haz the power, 2017-05-26 19:33:56]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Hzj_jie, 2017-05-26 19:38:10]

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

[commit-bot: I haz the power, 2017-05-26 19:38:55]

Dry run: 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-05-26 19:47:15]

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

[commit-bot: I haz the power, 2017-05-26 19:47:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Hzj_jie, 2017-05-26 20:12:41]

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

[commit-bot: I haz the power, 2017-05-26 20:13:13]

Dry run: 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-05-26 20:51:34]

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

[commit-bot: I haz the power, 2017-05-26 20:51:35]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2017-05-30 19:03:09]

I think we should understand the root cause. Signaling service may deliver
messages out-of-order. OrderedMessageQueue is designed to reorder messages to
match the order on the sender side. It's implemented with an assumption that the
client will wait response to the first message before sending another one.
If you can reproduce this issue can you the capture the messages that the host
receives? (debug build of the host logs it in the console) 
This patch looks good to me, but I'd like to understand why we need it.

[Hzj_jie, 2017-05-30 23:22:51]

On 2017/05/30 19:03:09, Sergey Ulanov wrote:
> I think we should understand the root cause. Signaling service may deliver
> messages out-of-order. OrderedMessageQueue is designed to reorder messages to
> match the order on the sender side. It's implemented with an assumption that
the
> client will wait response to the first message before sending another one.
> If you can reproduce this issue can you the capture the messages that the host
> receives? (debug build of the host logs it in the console) 
> This patch looks good to me, but I'd like to understand why we need it.

Sure. But it's not a very consistent behavior, I will attach log to the bug once
I have got a reproduce.

[Hzj_jie, 2017-06-13 22:17:32]

On 2017/05/30 23:22:51, Hzj_jie wrote:
> On 2017/05/30 19:03:09, Sergey Ulanov wrote:
> > I think we should understand the root cause. Signaling service may deliver
> > messages out-of-order. OrderedMessageQueue is designed to reorder messages
to
> > match the order on the sender side. It's implemented with an assumption that
> the
> > client will wait response to the first message before sending another one.
> > If you can reproduce this issue can you the capture the messages that the
host
> > receives? (debug build of the host logs it in the console) 
> > This patch looks good to me, but I'd like to understand why we need it.
> 
> Sure. But it's not a very consistent behavior, I will attach log to the bug
once
> I have got a reproduce.

A reproduce has been caught and the bug is updated.

[Sergey Ulanov, 2017-06-14 23:35:12]

On 2017/06/13 22:17:32, Hzj_jie wrote:
> On 2017/05/30 23:22:51, Hzj_jie wrote:
> > On 2017/05/30 19:03:09, Sergey Ulanov wrote:
> > > I think we should understand the root cause. Signaling service may deliver
> > > messages out-of-order. OrderedMessageQueue is designed to reorder messages
> to
> > > match the order on the sender side. It's implemented with an assumption
that
> > the
> > > client will wait response to the first message before sending another one.
> > > If you can reproduce this issue can you the capture the messages that the
> host
> > > receives? (debug build of the host logs it in the console) 
> > > This patch looks good to me, but I'd like to understand why we need it.
> > 
> > Sure. But it's not a very consistent behavior, I will attach log to the bug
> once
> > I have got a reproduce.
> 
> A reproduce has been caught and the bug is updated.

Please see my comment in that bug. I think this DCHECK is still valid. Problem
is that the queue is not intialized properly after the first session-initate
message

[Hzj_jie, 2017-06-14 23:36:50]

On 2017/06/14 23:35:12, Sergey Ulanov wrote:
> On 2017/06/13 22:17:32, Hzj_jie wrote:
> > On 2017/05/30 23:22:51, Hzj_jie wrote:
> > > On 2017/05/30 19:03:09, Sergey Ulanov wrote:
> > > > I think we should understand the root cause. Signaling service may
deliver
> > > > messages out-of-order. OrderedMessageQueue is designed to reorder
messages
> > to
> > > > match the order on the sender side. It's implemented with an assumption
> that
> > > the
> > > > client will wait response to the first message before sending another
one.
> > > > If you can reproduce this issue can you the capture the messages that
the
> > host
> > > > receives? (debug build of the host logs it in the console) 
> > > > This patch looks good to me, but I'd like to understand why we need it.
> > > 
> > > Sure. But it's not a very consistent behavior, I will attach log to the
bug
> > once
> > > I have got a reproduce.
> > 
> > A reproduce has been caught and the bug is updated.
> 
> Please see my comment in that bug. I think this DCHECK is still valid. Problem
> is that the queue is not intialized properly after the first session-initate
> message

Kelvin is fixing the issue, so this change should be deprecated. Sorry about
forgetting closing this change.

[Hzj_jie, 2017-06-14 23:36:58]

zijiehe@chromium.org changed reviewers:
- sergeyu@chromium.org