Fix OnChannelError() in StreamTextureHost

Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162
Committed: https://crrev.com/3717d9ae21d551d001edce8d6c269149676541a2
Cr-Commit-Position: refs/heads/master@{#434715}

[tguilbert, 2016-11-23 01:22:58]

Description was changed from

==========
Update guards in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to make
ownership clearer.

BUG=667162
==========

to

==========
Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162
==========

[tguilbert, 2016-11-23 01:25:29]

Patchset #1 (id:1) has been deleted

[tguilbert, 2016-11-23 01:28:12]

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

[tguilbert, 2016-11-23 01:28:13]

I don't think this will fix the bug completely (there seem to be two crashes
bundled in there), but I think it should help with stability in general (the
|route_id| checks didn't actually do anything ever).

PTAL :)

[watk, 2016-11-23 20:26:07]

nice, lgtm

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
File content/renderer/gpu/stream_texture_host_android.cc (right):

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:18: listener_(NULL),
nit: can you make this nullptr for consistency with below while you're at it.

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:34: if (!channel_.get())
All the if (!channel_.get())s can be replace with just if (!channel_)

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:59: return;
Re your question: think it's a personal preference thing whether you do this for
one-liners. I kinda like the old ones because they're shorter. But I'm fine with
this too.

[tguilbert, 2016-11-23 20:43:13]

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

[tguilbert, 2016-11-23 20:43:13]

jbauman, can you OWNERS review
content/renderer/gpu/stream_texture_host_android.cc?

Thank you!
Thomas

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
File content/renderer/gpu/stream_texture_host_android.cc (right):

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:18: listener_(NULL),
On 2016/11/23 20:26:07, watk wrote:
> nit: can you make this nullptr for consistency with below while you're at it.

Done.

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:34: if (!channel_.get())
On 2016/11/23 20:26:07, watk wrote:
> All the if (!channel_.get())s can be replace with just if (!channel_)

Woops yes.

https://codereview.chromium.org/2530443002/diff/20001/content/renderer/gpu/st...
content/renderer/gpu/stream_texture_host_android.cc:59: return;
On 2016/11/23 20:26:07, watk wrote:
> Re your question: think it's a personal preference thing whether you do this
for
> one-liners. I kinda like the old ones because they're shorter. But I'm fine
with
> this too.

Ok. I also preferred the old way. Changing it back.

[jbauman, 2016-11-23 22:50:17]

lgtm

[tguilbert, 2016-11-23 23:45:42]

The CQ bit was checked by tguilbert@chromium.org

[tguilbert, 2016-11-23 23:45:43]

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/2530443002/#ps40001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-11-23 23:46:24]

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-11-24 01:49:17]

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

[commit-bot: I haz the power, 2016-11-24 01:49:18]

Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[tguilbert, 2016-11-28 19:47:02]

The CQ bit was checked by tguilbert@chromium.org

[commit-bot: I haz the power, 2016-11-28 19:48:12]

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-11-28 20:53:13]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480362422979700,
"parent_rev": "30f5ca74b92ae81b7586a3bcd3bd3e3c71aacd5b", "commit_rev":
"6b2098d30d10b82418b96be526fcb594d34b4e88"}

[commit-bot: I haz the power, 2016-11-28 20:53:37]

Description was changed from

==========
Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162
==========

to

==========
Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162
==========

[commit-bot: I haz the power, 2016-11-28 20:53:38]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-11-28 20:57:11]

Description was changed from

==========
Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162
==========

to

==========
Fix OnChannelError() in StreamTextureHost

StreamTextureHost is always created with a valid |route_id|, and it also
does not handle OnChannelError().

This change removes the useless |route_id| checks, sets |channel_| to
nullptr when OnChannelError() is called, and adds appropriate |channel_|
null checks.

This change also removes raw pointers from StreamTextureProxy, to fix
style conformance and StreamTextureHost ownership clearer.

BUG=667162

Committed: https://crrev.com/3717d9ae21d551d001edce8d6c269149676541a2
Cr-Commit-Position: refs/heads/master@{#434715}
==========

[commit-bot: I haz the power, 2016-11-28 20:57:13]

Patchset 2 (id:??) landed as
https://crrev.com/3717d9ae21d551d001edce8d6c269149676541a2
Cr-Commit-Position: refs/heads/master@{#434715}