media: Make sure we transition back to a non-loading state

media: Make sure we transition back to a non-loading state

An earilier bug-workaround kept the media in a loading state when
a read operation was pending, but didn't always transition back to
idle when the read operation was done. This CL fixes the problem
by making sure that we check the loading state after read callbacks.

These extra calls to check the loading state can in some cases cause
us to miss progress callbacks. We fix this by allowing in-flight
progress callbacks to complete even if reader_ has been destroyed.

BUG=662615
Committed: https://crrev.com/a89cc8bcb5d204e3f42eb56d2db24837ba0d3a8a
Cr-Commit-Position: refs/heads/master@{#430860}

[hubbe, 2016-11-07 23:05:50]

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

[hubbe, 2016-11-07 23:05:56]

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

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

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-11-07 23:38:55]

lgtm.

[DaleCurtis, 2016-11-07 23:42:40]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-11-07 23:42:40]

Leaks should have a test added.

[commit-bot: I haz the power, 2016-11-08 01:39:56]

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

[commit-bot: I haz the power, 2016-11-08 01:39:57]

Dry run: This issue passed the CQ dry run.

[hubbe, 2016-11-08 21:49:57]

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

[commit-bot: I haz the power, 2016-11-08 21:50:31]

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

[hubbe, 2016-11-08 21:50:45]

Test added. PTAL.

[DaleCurtis, 2016-11-08 21:53:24]

lgtm to land this as is, but it seems like something we should have a layout
test for too.

[commit-bot: I haz the power, 2016-11-08 21:54:15]

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

[commit-bot: I haz the power, 2016-11-08 21:54:15]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hubbe, 2016-11-08 21:58:23]

On 2016/11/08 21:53:24, DaleCurtis wrote:
> lgtm to land this as is, but it seems like something we should have a layout
> test for too.

The layout test would be flaky though, as read operations comes from a separate
thread.

[DaleCurtis, 2016-11-08 21:59:22]

Ah, I thought the test case was reliable.

[hubbe, 2016-11-08 22:09:30]

On 2016/11/08 21:59:22, DaleCurtis wrote:
> Ah, I thought the test case was reliable.

The test would have false positives, not sure at what rate.

[hubbe, 2016-11-08 22:41:00]

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

[commit-bot: I haz the power, 2016-11-08 22:41:27]

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, 2016-11-09 00:06:15]

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

[commit-bot: I haz the power, 2016-11-09 00:06:16]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[hubbe, 2016-11-09 00:18:17]

The CQ bit was checked by hubbe@chromium.org

[hubbe, 2016-11-09 00:18:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org, dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2481673004/#ps40001
(title: "merged")

[commit-bot: I haz the power, 2016-11-09 00:19:15]

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-11-09 04:28:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-09 04:33:00]

Description was changed from

==========
media: Make sure we transition back to a non-loading state

An earilier bug-workaround kept the media in a loading state when
a read operation was pending, but didn't always transition back to
idle when the read operation was done. This CL fixes the problem
by making sure that we check the loading state after read callbacks.

These extra calls to check the loading state can in some cases cause
us to miss progress callbacks. We fix this by allowing in-flight
progress callbacks to complete even if reader_ has been destroyed.

BUG=662615
==========

to

==========
media: Make sure we transition back to a non-loading state

An earilier bug-workaround kept the media in a loading state when
a read operation was pending, but didn't always transition back to
idle when the read operation was done. This CL fixes the problem
by making sure that we check the loading state after read callbacks.

These extra calls to check the loading state can in some cases cause
us to miss progress callbacks. We fix this by allowing in-flight
progress callbacks to complete even if reader_ has been destroyed.

BUG=662615

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

[commit-bot: I haz the power, 2016-11-09 04:33:01]

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