Fix a renderer crash when media elements get deleted

Fix a renderer crash when media elements get deleted

When media elements got deleted, stream_texture_factory_ will get deleted before video_frame_.
This causes the callback function to execute with a null pointer.
Move the DestroyStreamTexture call in to WMPA dtor.

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

[Ami GONE FROM CHROMIUM, 2012-07-30 23:09:35]

http://codereview.chromium.org/10828079/diff/1/webkit/media/android/webmediap...
File webkit/media/android/webmediaplayer_android.cc (right):

http://codereview.chromium.org/10828079/diff/1/webkit/media/android/webmediap...
webkit/media/android/webmediaplayer_android.cc:95: if (stream_id_)
Huh; I didn't realize there's only one VideoFrame in the android port (in the
media/ pipeline there are 4 in flight during playback).  Is there any reason
DestroyStreamTexture shouldn't be inlined into the dtor?
Further is there any reason the CreateStreamTexture call needs to be lazy
instead of inlining it into the ctor?  It seems like that'd be much clearer to
read...

[qinmin, 2012-07-31 22:07:52]

PTAL

https://chromiumcodereview.appspot.com/10828079/diff/1/webkit/media/android/w...
File webkit/media/android/webmediaplayer_android.cc (right):

https://chromiumcodereview.appspot.com/10828079/diff/1/webkit/media/android/w...
webkit/media/android/webmediaplayer_android.cc:95: if (stream_id_)
We had some concerns previously on memory usage of surface texture. The android
team has recently confirmed with us that no buffer will be allocated if surface
texture is not being used. So moving CreateStreamTexture to ctor makes sense
now. 
Modified the code to make stream_id_ and texture_id_ initialized in ctor, and
deletion happens in dtor. video_frame_ will be constructed once its size is
known.
On 2012/07/30 23:09:36, Ami Fischman wrote:
> Huh; I didn't realize there's only one VideoFrame in the android port (in the
> media/ pipeline there are 4 in flight during playback).  Is there any reason
> DestroyStreamTexture shouldn't be inlined into the dtor?
> Further is there any reason the CreateStreamTexture call needs to be lazy
> instead of inlining it into the ctor?  It seems like that'd be much clearer to
> read...

[Ami GONE FROM CHROMIUM, 2012-07-31 22:27:40]

lgtm

https://chromiumcodereview.appspot.com/10828079/diff/7001/webkit/media/androi...
File webkit/media/android/webmediaplayer_android.cc (right):

https://chromiumcodereview.appspot.com/10828079/diff/7001/webkit/media/androi...
webkit/media/android/webmediaplayer_android.cc:87: if
(stream_texture_factory_.get()) {
What use is this class if there is no stream_texture_factory_ set?
(can this be promoted to a DCHECK, instead?)

[qinmin, 2012-07-31 22:52:33]

https://chromiumcodereview.appspot.com/10828079/diff/7001/webkit/media/androi...
File webkit/media/android/webmediaplayer_android.cc (right):

https://chromiumcodereview.appspot.com/10828079/diff/7001/webkit/media/androi...
webkit/media/android/webmediaplayer_android.cc:87: if
(stream_texture_factory_.get()) {
This is currently being used by DRT, so I will just keep it for now

On 2012/07/31 22:27:40, Ami Fischman wrote:
> What use is this class if there is no stream_texture_factory_ set?
> (can this be promoted to a DCHECK, instead?)

[commit-bot: I haz the power, 2012-07-31 22:52:55]

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

[commit-bot: I haz the power, 2012-07-31 22:52:56]

Failed to apply patch for webkit/media/android/webmediaplayer_android.cc:
While running patch -p1 --forward --force;
patching file webkit/media/android/webmediaplayer_android.cc
Hunk #4 FAILED at 541.
1 out of 4 hunks FAILED -- saving rejects to file
webkit/media/android/webmediaplayer_android.cc.rej

[commit-bot: I haz the power, 2012-08-01 01:38:53]

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

[commit-bot: I haz the power, 2012-08-01 03:47:43]

Try job failure for 10828079-7005 (retry) on win_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-08-01 04:20:51]

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

[commit-bot: I haz the power, 2012-08-01 06:21:45]

Change committed as 149379