Don't background pause suspend until we have future data.

Don't background pause suspend until we have future data.

Otherwise the pause state is not reliable and we may incorrectly
prevent background playback attempts.

We can't always force background pause suspend since background
playback may trigger a play() but we don't know the correct paused
state yet.

BUG=658680
TEST=new test.

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

[DaleCurtis, 2016-10-26 22:30:38]

Patchset #1 (id:1) has been deleted

[DaleCurtis, 2016-10-26 22:31:28]

Description was changed from

==========
Don't background pause suspend until we have future data.

Otherwise the pause state is not reliable and we may incorrectly
prevent background playback attempts.

BUG=
==========

to

==========
Don't background pause suspend until we have future data.

Otherwise the pause state is not reliable and we may incorrectly
prevent background playback attempts.

We can't always force background pause suspend since background
playback may trigger a play() but we don't know the correct paused
state yet.

BUG=658680
TEST=new test.
==========

[DaleCurtis, 2016-10-26 22:31:29]

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

[sandersd (OOO until July 31), 2016-10-26 23:07:46]

lgtm % nits.

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
media/blink/webmediaplayer_impl.cc:1760: // we may miss out on legitimate
background playback attempts.
The comment is slightly confusing since we can still background-suspend such
players.

If anything, the comments should be on all the other places we use |paused_|, to
explain why we can do that when the value has little meaning before
|have_future_data|.

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
File media/blink/webmediaplayer_impl_unittest.cc (right):

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
media/blink/webmediaplayer_impl_unittest.cc:310:
SetReadyState(blink::WebMediaPlayer::ReadyStateHaveFutureData);
This doesn't look like a construction test.

[DaleCurtis, 2016-10-27 00:36:37]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-10-27 00:36:40]

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
File media/blink/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
media/blink/webmediaplayer_impl.cc:1760: // we may miss out on legitimate
background playback attempts.
On 2016/10/26 at 23:07:46, sandersd wrote:
> The comment is slightly confusing since we can still background-suspend such
players.
> 
> If anything, the comments should be on all the other places we use |paused_|,
to explain why we can do that when the value has little meaning before
|have_future_data|.

Reworded.

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
File media/blink/webmediaplayer_impl_unittest.cc (right):

https://codereview.chromium.org/2449873006/diff/20001/media/blink/webmediapla...
media/blink/webmediaplayer_impl_unittest.cc:310:
SetReadyState(blink::WebMediaPlayer::ReadyStateHaveFutureData);
On 2016/10/26 at 23:07:46, sandersd wrote:
> This doesn't look like a construction test.

Hmm, good point already covered in a background case below.

[DaleCurtis, 2016-10-27 00:36:40]

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/2449873006/#ps40001
(title: "Reword comments, remove test.")

[commit-bot: I haz the power, 2016-10-27 00:37:08]

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-10-27 01:50:07]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-10-27 01:52:11]

Description was changed from

==========
Don't background pause suspend until we have future data.

Otherwise the pause state is not reliable and we may incorrectly
prevent background playback attempts.

We can't always force background pause suspend since background
playback may trigger a play() but we don't know the correct paused
state yet.

BUG=658680
TEST=new test.
==========

to

==========
Don't background pause suspend until we have future data.

Otherwise the pause state is not reliable and we may incorrectly
prevent background playback attempts.

We can't always force background pause suspend since background
playback may trigger a play() but we don't know the correct paused
state yet.

BUG=658680
TEST=new test.

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

[commit-bot: I haz the power, 2016-10-27 01:52:12]

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