Fix a timing bug in multibuffer.

Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
Committed: https://crrev.com/3b2a5629da155db59d3449a6d640c107e91aa14d
Cr-Commit-Position: refs/heads/master@{#419995}

[hubbe, 2016-09-21 00:21:27]

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

[hubbe, 2016-09-21 00:21:46]

hubbe@chromium.org changed reviewers:
+ sandersd@chromium.org

[hubbe, 2016-09-21 00:22:16]

Description was changed from

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
==========

to

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
==========

[commit-bot: I haz the power, 2016-09-21 00:22:22]

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

[sandersd (OOO until July 31), 2016-09-21 01:13:39]

lgtm

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
File media/blink/multibuffer_data_source.cc (right):

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
media/blink/multibuffer_data_source.cc:534: void
MultibufferDataSource::UpdateLoadingState(bool force_loading) {
Nit: Add _Locked suffix?

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
media/blink/multibuffer_data_source.cc:542: is_loading = true;
This means nothing to me, but I'll believe you.

[commit-bot: I haz the power, 2016-09-21 01:34:19]

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

[commit-bot: I haz the power, 2016-09-21 01:34:19]

Dry run: This issue passed the CQ dry run.

[hubbe, 2016-09-21 04:11:48]

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

[commit-bot: I haz the power, 2016-09-21 04:12:23]

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

[hubbe, 2016-09-21 04:19:22]

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
File media/blink/multibuffer_data_source.cc (right):

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
media/blink/multibuffer_data_source.cc:534: void
MultibufferDataSource::UpdateLoadingState(bool force_loading) {
On 2016/09/21 01:13:39, sandersd wrote:
> Nit: Add _Locked suffix?

Done

https://codereview.chromium.org/2357773003/diff/1/media/blink/multibuffer_dat...
media/blink/multibuffer_data_source.cc:542: is_loading = true;
On 2016/09/21 01:13:39, sandersd wrote:
> This means nothing to me, but I'll believe you.

Well, basically we can't destroy reader_ if there is a read operation pending.
There are a few ways to do this:

1) pretend like we're loading if a read operation is pending. This sort of makes
sense as reading will move the read pointer and cause additional buffering.

2) just do "if(read_op_) return", which just prevents us from going to/from a
loading state while a read is pending. Should also work, but just doesn't feel
as correct to me.

3) Do the state to not loading now, but destroy reader_ later. (Seems a bit more
complicated to me, as the state might change again between now and then...)

[commit-bot: I haz the power, 2016-09-21 05:31:48]

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

[commit-bot: I haz the power, 2016-09-21 05:31:49]

Dry run: This issue passed the CQ dry run.

[hubbe, 2016-09-21 07:10:28]

The CQ bit was checked by hubbe@chromium.org

[hubbe, 2016-09-21 07:10:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/2357773003/#ps20001
(title: "comments addressed")

[commit-bot: I haz the power, 2016-09-21 07:10:51]

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-09-21 07:18:07]

Description was changed from

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
==========

to

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
==========

[commit-bot: I haz the power, 2016-09-21 07:18:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-21 07:20:39]

Description was changed from

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434
==========

to

==========
Fix a timing bug in multibuffer.

When "cancel on defer is on", the multibuffer reader can sometimes get
delete before it is allowed to deliver a pending read callback. This basically
hangs the multibuffer data source as it waits for the read callback forever.

BUG=648395, 647867, 646434

Committed: https://crrev.com/3b2a5629da155db59d3449a6d640c107e91aa14d
Cr-Commit-Position: refs/heads/master@{#419995}
==========

[commit-bot: I haz the power, 2016-09-21 07:20:40]

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