Remove OnMediaResourcesReleased callback passed to MediaPlayerAndroid

Remove OnMediaResourcesReleased callback passed to MediaPlayerAndroid

This callback has the reentrance issue as calling MediaPlayerBridge::Release() will call into function.
The worst case happens when BrowserMediaPlayerManager dtor is called. In that case, this function will be called on each player, while BMPM is in a partially destroyed state.
Since BMPM controls the lifetime of the players, it can call OnMediaResourcesReleased() when it calls Release() on a player.
For MSE case, we also simplified the situation here since we can check MSP::IsSurfaceInUse(). This CL also fixes that call.

BUG=410096
Committed: https://crrev.com/0b08967cc3c1a8e6bc17921201553bb6d8a7bb90
Cr-Commit-Position: refs/heads/master@{#293207}

[qinmin, 2014-09-03 00:56:16]

qinmin@chromium.org changed reviewers:
+ xhwang@chromium.org

[qinmin, 2014-09-03 00:56:16]

PTAL

[xhwang, 2014-09-03 16:36:58]

lgtm % nits

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:578:
OnMediaResourcesReleased(player_id);
This pattern appeared 3 times:

player->Release();
OnMediaResourcesReleased(player->player_id());

How about add a helper function like ReleasePlayer()?

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.h (right):

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.h:162: virtual void
OnMediaResourcesReleased(int player_id);
nit: This is not a callback anymore. How about
"ReleaseMediaResources(player_id)"?

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.h:162: virtual void
OnMediaResourcesReleased(int player_id);
Please update the CL description/title:

"Remove BrowserMediaPlayerManager::OnMediaResourcesReleased()"

You are not really removing this call :) Instead, you are removing the
ReleaseMediaResourcesCB in MediaPlayerAndroid ctor.

[qinmin, 2014-09-03 18:27:52]

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.cc (right):

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.cc:578:
OnMediaResourcesReleased(player_id);
On 2014/09/03 16:36:58, xhwang wrote:
> This pattern appeared 3 times:
> 
> player->Release();
> OnMediaResourcesReleased(player->player_id());
> 
> How about add a helper function like ReleasePlayer()?

Done.

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
File content/browser/media/android/browser_media_player_manager.h (right):

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.h:162: virtual void
OnMediaResourcesReleased(int player_id);
On 2014/09/03 16:36:58, xhwang wrote:
> nit: This is not a callback anymore. How about
> "ReleaseMediaResources(player_id)"?

Done.

https://codereview.chromium.org/534063002/diff/1/content/browser/media/androi...
content/browser/media/android/browser_media_player_manager.h:162: virtual void
OnMediaResourcesReleased(int player_id);
On 2014/09/03 16:36:58, xhwang wrote:
> Please update the CL description/title:
> 
> "Remove BrowserMediaPlayerManager::OnMediaResourcesReleased()"
> 
> You are not really removing this call :) Instead, you are removing the
> ReleaseMediaResourcesCB in MediaPlayerAndroid ctor.

Done.

[qinmin, 2014-09-03 18:27:56]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2014-09-03 18:34:56]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2014-09-03 18:38:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qinmin@chromium.org/534063002/40001

[commit-bot: I haz the power, 2014-09-03 21:52:00]

Committed patchset #3 (id:40001) as bc9e95e89a6fa3f131fc48a4ba034f037d4643c4

[commit-bot: I haz the power, 2014-09-10 03:28:07]

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