Fix browser crash when calling MediaPlayerBridge::PrepareAsync

Fix browser crash when calling MediaPlayerBridge::PrepareAsync

BUG=319956
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235663

[qinmin, 2013-11-15 19:36:04]

PTAL.

downstream change: https://gerrit-int.chromium.org/#/c/46026/

[scherkus (not reviewing), 2013-11-15 23:14:04]

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
File media/base/android/media_player_bridge.cc (right):

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.cc:148: if
(Java_MediaPlayerBridge_setDataSource(
I'd flip this around to use early returns

if (!Java_MediaPlayerBridge_setDataSource(...)) {
  OnMediaError(MEDIA_ERROR_FORMAT);
  return;
}

manager()->RequestMediaResources(...);

if (!Java_MediaPlayerBridge_prepareAsync(...)) {
  OnMediaError(MEDIA_ERROR_FORMAT);
  return;
}



it's a bit more verborse but clearer imo

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.cc:151:
manager()->RequestMediaResources(player_id());
do we need to call ReleaseMediaResources() if prepareAsync() fails?

[qinmin, 2013-11-16 00:00:19]

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
File media/base/android/media_player_bridge.cc (right):

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.cc:148: if
(Java_MediaPlayerBridge_setDataSource(
On 2013/11/15 23:14:05, scherkus wrote:
> I'd flip this around to use early returns
> 
> if (!Java_MediaPlayerBridge_setDataSource(...)) {
>   OnMediaError(MEDIA_ERROR_FORMAT);
>   return;
> }
> 
> manager()->RequestMediaResources(...);
> 
> if (!Java_MediaPlayerBridge_prepareAsync(...)) {
>   OnMediaError(MEDIA_ERROR_FORMAT);
>   return;
> }
> 
> 
> 
> it's a bit more verborse but clearer imo

Done.

https://codereview.chromium.org/74293002/diff/1/media/base/android/media_play...
media/base/android/media_player_bridge.cc:151:
manager()->RequestMediaResources(player_id());
No, we shouldn't. 
The java MediaPlayer instance still lives if we simply call
ReleaseMediaResources(). So we need the manager to still take this player into
account when allocating resources.

Calling Release() will also work, but it feels wierd setDataSource() can delete
the player instance 

On 2013/11/15 23:14:05, scherkus wrote:
> do we need to call ReleaseMediaResources() if prepareAsync() fails?

[scherkus (not reviewing), 2013-11-16 00:20:14]

lgtm

[commit-bot: I haz the power, 2013-11-16 02:01:51]

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

[commit-bot: I haz the power, 2013-11-18 08:20:37]

Change committed as 235663