Keep playing audio even when opening another tab, going to another app etc.

Keep playing audio even when opening another tab, going to another app etc.

Test: run both content shell and chrome for android, gone to a few html5 serving pages.

http://romaindillet.github.com/That-Radio-Player/#
and
http://grooveshark.com/#!/playlist/ac-dc/28046570

Then verified that audio keeps playing if I switch tabs or leave the app.

Also added some logging to verify that no matter what the internal player is released when you leave either when you leave the page if nothing is playing or upon closing the tab or navigating away if it is.




BUG=121898
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167906

[Miguel Garcia, 2012-11-13 15:49:58]

Min, if you are ok, I am going to wait till you have a look first and assuming
you are ok I will then include someone OWNERS file.

[qinmin, 2012-11-14 18:14:29]

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
content/renderer/render_view_impl.cc:5683: DCHECK(media_player_manager_);
this DCHECK is useless, if media_player_manager_ is null, it will crash in code
below.

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
content/renderer/render_view_impl.cc:5686: // unless some audio is playing.
some media is playing

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
File webkit/media/android/webmediaplayer_manager_android.cc (right):

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.cc:34: DCHECK(player);
useless DCHECK

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.cc:37: if (player &&
(!player->hasAudio() || player->paused()))
hasAudio() always return true on android. Even on desktop, it only checks the
current frame. You can remove it here.

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
File webkit/media/android/webmediaplayer_manager_android.h (right):

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.h:32: // an audio play is in
progress.
not "audio", but ”media“, the logic should also apply to video.

[Miguel Garcia, 2012-11-14 18:50:31]

Thanks for the quick review!

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
content/renderer/render_view_impl.cc:5683: DCHECK(media_player_manager_);
On 2012/11/14 18:14:29, qinmin wrote:
> this DCHECK is useless, if media_player_manager_ is null, it will crash in
code
> below.

Done.

http://codereview.chromium.org/11361239/diff/3005/content/renderer/render_vie...
content/renderer/render_view_impl.cc:5686: // unless some audio is playing.
On 2012/11/14 18:14:29, qinmin wrote:
> some media is playing

Done.

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
File webkit/media/android/webmediaplayer_manager_android.cc (right):

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.cc:34: DCHECK(player);
On 2012/11/14 18:14:29, qinmin wrote:
> useless DCHECK

removed

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.cc:37: if (player &&
(!player->hasAudio() || player->paused()))
Removed

On 2012/11/14 18:14:29, qinmin wrote:
> hasAudio() always return true on android. Even on desktop, it only checks the
> current frame. You can remove it here.

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
File webkit/media/android/webmediaplayer_manager_android.h (right):

http://codereview.chromium.org/11361239/diff/3005/webkit/media/android/webmed...
webkit/media/android/webmediaplayer_manager_android.h:32: // an audio play is in
progress.
Well I thought that if the media happened to be video without audio it'd be nice
to stop it but ok.

On 2012/11/14 18:14:29, qinmin wrote:
> not "audio", but ”media“, the logic should also apply to video.

[qinmin, 2012-11-14 19:01:40]

lgtm.

[Miguel Garcia, 2012-11-14 19:05:57]

Kent, James can you have a look for OWNERs approval of the different
directories?

[commit-bot: I haz the power, 2012-11-14 19:26:37]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[jamesr, 2012-11-14 22:09:20]

lgtm

[commit-bot: I haz the power, 2012-11-15 08:51:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miguelg@chromium.org/11361239/7002

[commit-bot: I haz the power, 2012-11-15 13:01:57]

Change committed as 167906