Fix SetStreamTextureSize crash

Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.

Committed: https://crrev.com/08543d080630ba84936851881fecd2dd37e889be
Cr-Commit-Position: refs/heads/master@{#437060}

[tguilbert, 2016-12-07 00:37:03]

Description was changed from

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safe guard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified HLS stil works.
==========

to

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safe guard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

[tguilbert, 2016-12-07 00:37:31]

Description was changed from

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safe guard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

to

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

[tguilbert, 2016-12-07 00:38:04]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 00:39:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tguilbert, 2016-12-07 00:40:01]

tguilbert@chromium.org changed reviewers:
+ watk@chromium.org

[tguilbert, 2016-12-07 00:40:02]

PTAL :)

Thanks!
Thomas

[tguilbert, 2016-12-07 01:15:02]

The CQ bit was checked by tguilbert@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 01:15:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[watk, 2016-12-07 01:48:11]

lgtm

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
File content/renderer/media/android/stream_texture_wrapper_impl.cc (right):

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.cc:132: //
InitializeOnMainThread() hasn't completed, or failed.
nit: "hasn't completed" makes it sounds like it runs concurrently. Maybe
"InitializeOnMainThread() hasn't run or failed".

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.cc:179: // |init_cb|
is bound to the thread that originally called Initialize().
nit: comment feels out of place now that you're calling it above as well. Could
probably safely be deleted.

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
File content/renderer/media/android/stream_texture_wrapper_impl.h (right):

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.h:58: //   -
|init_cb| will be run on the calling thread, and will passed a bool
"will be"

[commit-bot: I haz the power, 2016-12-07 02:58:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 02:58:54]

Dry run: This issue passed the CQ dry run.

[tguilbert, 2016-12-07 19:23:56]

Thank you! Submitting.

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
File content/renderer/media/android/stream_texture_wrapper_impl.cc (right):

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.cc:132: //
InitializeOnMainThread() hasn't completed, or failed.
On 2016/12/07 01:48:11, watk wrote:
> nit: "hasn't completed" makes it sounds like it runs concurrently. Maybe
> "InitializeOnMainThread() hasn't run or failed".

Done.

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.cc:179: // |init_cb|
is bound to the thread that originally called Initialize().
On 2016/12/07 01:48:11, watk wrote:
> nit: comment feels out of place now that you're calling it above as well.
Could
> probably safely be deleted.

Done.

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
File content/renderer/media/android/stream_texture_wrapper_impl.h (right):

https://codereview.chromium.org/2556983002/diff/20001/content/renderer/media/...
content/renderer/media/android/stream_texture_wrapper_impl.h:58: //   -
|init_cb| will be run on the calling thread, and will passed a bool
On 2016/12/07 01:48:11, watk wrote:
> "will be"

Done.

[tguilbert, 2016-12-07 19:23:59]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-12-07 19:23:59]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org
Link to the patchset: https://codereview.chromium.org/2556983002/#ps40001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-12-07 19:24:47]

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-12-07 21:16:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481138639294140,
"parent_rev": "d4c34084a229cede91590a0e8d8e7903957819e1", "commit_rev":
"cbe616667caf91cd43ff233888869e316a818389"}

[commit-bot: I haz the power, 2016-12-07 21:17:06]

Description was changed from

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

to

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

[commit-bot: I haz the power, 2016-12-07 21:17:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-07 21:20:37]

Description was changed from

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.
==========

to

==========
Fix SetStreamTextureSize crash

We currently do not properly report initialization failures in the
StreamTextureWrapperImpl. This means that we can be left in a state
where the |stream_texture_proxy_| is null after initialization.

This change modifies the MediaPlayerRendererClient's init sequence to
properly error out early and abort initialization when we are not able
to create a StreamTextureProxy. It also adds a safeguard to avoid a
null dereference in StreamTextureWrapperImpl::UpdateTextureSize().

BUG=671358
TEST=Manualy verified playing HLS still works.

Committed: https://crrev.com/08543d080630ba84936851881fecd2dd37e889be
Cr-Commit-Position: refs/heads/master@{#437060}
==========

[commit-bot: I haz the power, 2016-12-07 21:20:38]

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