[cast_channel] Clean up CastSocketImpl ctor parameters

[cast_channel] Clean up CastSocketImpl ctor parameters and ChannelAuthType usage

- Remove |owner_extension_id| (not used anymore) and |channel_auth| (should always be SSL_VERIFIED) from CastSocketImpl's ctor
- Remove ChannelAuthType from cast_channel_enum.h and clean up its usage thoughout the code

BUG=732669
Review-Url: https://codereview.chromium.org/2942743002
Cr-Commit-Position: refs/heads/master@{#479807}
Committed: https://chromium.googlesource.com/chromium/src/+/ebd6f8fd664d851ccc571bbd80ec0767d4bc40a8

[zhaobin, 2017-06-14 20:35:30]

zhaobin@chromium.org changed reviewers:
+ imcheng@chromium.org, mfoltz@chromium.org

[zhaobin, 2017-06-14 20:35:38]

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

[commit-bot: I haz the power, 2017-06-14 20:35:55]

Dry run: 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, 2017-06-14 21:49:11]

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

[commit-bot: I haz the power, 2017-06-14 21:49:13]

Dry run: This issue passed the CQ dry run.

[imcheng, 2017-06-14 22:25:34]

lgtm

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
File components/cast_channel/cast_socket.cc (right):

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.cc:115:
channel_auth_(ChannelAuthType::SSL_VERIFIED),
Can we remove this field and just hardcode SSL_VERIFIED wherever channel_auth_ /
(channel_auth()) is used?


It seems like we can remove ChannelAuthType entirely; but we may have to do it
in stages since it's also defined in the Cast Channel API IDL.

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
File components/cast_channel/cast_socket.h (right):

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.h:141: const base::TimeDelta&
connect_timeout,
Optional: It's fine to pass base::TimeDelta by value, as it is compact enough
and immutable.

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
File extensions/browser/api/cast_channel/cast_channel_api.cc (right):

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
extensions/browser/api/cast_channel/cast_channel_api.cc:281: channel_auth_ =
channel_auth_ isn't used anymore and can be removed.

[mark a. foltz, 2017-06-14 22:33:21]

It should be possible to remove the channel_auth_type_ field and ChannelAuthType
internal enum as well, but that is a bigger change (may require fixing up
numerous tests).  Feel free to leave that for another day.

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
File components/cast_channel/cast_socket.h (right):

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.h:291: ChannelAuthType channel_auth_;
It should be possible to remove this (it can only take one value).

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
File extensions/browser/api/cast_channel/cast_channel_api.cc (right):

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
extensions/browser/api/cast_channel/cast_channel_api.cc:340:
SetResultFromSocket(*socket);
In FillChannelInfo, you can hard code SSL_VERIFIED from the extension API.  Then
it should be possible to remove the internal ChannelAuthType enum as well.

[mark a. foltz, 2017-06-14 22:33:30]

lgtm

[zhaobin, 2017-06-15 02:22:05]

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

[commit-bot: I haz the power, 2017-06-15 02:22:14]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zhaobin, 2017-06-15 02:22:22]

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
File components/cast_channel/cast_socket.cc (right):

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.cc:115:
channel_auth_(ChannelAuthType::SSL_VERIFIED),
On 2017/06/14 22:25:34, imcheng wrote:
> Can we remove this field and just hardcode SSL_VERIFIED wherever channel_auth_
/
> (channel_auth()) is used?
> 
> 
> It seems like we can remove ChannelAuthType entirely; but we may have to do it
> in stages since it's also defined in the Cast Channel API IDL. 

Done.

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
File components/cast_channel/cast_socket.h (right):

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.h:141: const base::TimeDelta&
connect_timeout,
On 2017/06/14 22:25:34, imcheng wrote:
> Optional: It's fine to pass base::TimeDelta by value, as it is compact enough
> and immutable.

Fixed in another patch (https://codereview.chromium.org/2937153002/).

https://codereview.chromium.org/2942743002/diff/1/components/cast_channel/cas...
components/cast_channel/cast_socket.h:291: ChannelAuthType channel_auth_;
On 2017/06/14 22:33:21, mark a. foltz wrote:
> It should be possible to remove this (it can only take one value).

Done.

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
File extensions/browser/api/cast_channel/cast_channel_api.cc (right):

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
extensions/browser/api/cast_channel/cast_channel_api.cc:281: channel_auth_ =
On 2017/06/14 22:25:34, imcheng wrote:
> channel_auth_ isn't used anymore and can be removed.

Done.

https://codereview.chromium.org/2942743002/diff/1/extensions/browser/api/cast...
extensions/browser/api/cast_channel/cast_channel_api.cc:340:
SetResultFromSocket(*socket);
On 2017/06/14 22:33:21, mark a. foltz wrote:
> In FillChannelInfo, you can hard code SSL_VERIFIED from the extension API. 
Then
> it should be possible to remove the internal ChannelAuthType enum as well.

Done.

[commit-bot: I haz the power, 2017-06-15 02:24:26]

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

[commit-bot: I haz the power, 2017-06-15 02:24:26]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mark a. foltz, 2017-06-15 17:19:01]

lgtm

Looks Great To Me.  Thanks for deleting more code!

It looks like you'll have to rebase, I saw a merge conflict in
cast_channel_enum.h in the bot output.

[mark a. foltz, 2017-06-15 17:19:45]

On 2017/06/15 at 17:19:01, mark a. foltz wrote:
> lgtm
> 
> Looks Great To Me.  Thanks for deleting more code!
> 
> It looks like you'll have to rebase, I saw a merge conflict in
cast_channel_enum.h in the bot output.

Also please update the patch description to mention removing channel_auth
everywhere (not just the ctor).

[zhaobin, 2017-06-15 17:25:02]

Description was changed from

==========
[cast_channel] Clean up CastSocketImpl ctor parameters

Remove |owner_extension_id| (not used anymore) and |channel_auth| (should always
be ChannelAuthType::SSL_VERIFIED) from CastSocketImpl's ctor

BUG=732669
==========

to

==========
[cast_channel] Clean up CastSocketImpl ctor parameters and ChannelAuthType usage

- Remove |owner_extension_id| (not used anymore) and |channel_auth| (should
always be SSL_VERIFIED) from CastSocketImpl's ctor
- Remove ChannelAuthType from cast_channel_enum.h and clean up its usage
thoughout the code

BUG=732669
==========

[zhaobin, 2017-06-15 17:55:11]

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

[commit-bot: I haz the power, 2017-06-15 17:55:43]

Dry run: 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, 2017-06-15 18:06:47]

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

[commit-bot: I haz the power, 2017-06-15 18:06:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zhaobin, 2017-06-15 18:56:11]

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

[commit-bot: I haz the power, 2017-06-15 18:56:52]

Dry run: 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, 2017-06-15 19:57:24]

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

[commit-bot: I haz the power, 2017-06-15 19:57:25]

Dry run: This issue passed the CQ dry run.

[zhaobin, 2017-06-15 20:12:16]

The CQ bit was checked by zhaobin@chromium.org

[zhaobin, 2017-06-15 20:12:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
imcheng@chromium.org, mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/2942743002/#ps60001
(title: "fix compile error")

[commit-bot: I haz the power, 2017-06-15 20:12:36]

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, 2017-06-15 20:19:25]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497557536500570,
"parent_rev": "8283e2a125eac90604b4dd2f67fd34d9c9d50115", "commit_rev":
"ebd6f8fd664d851ccc571bbd80ec0767d4bc40a8"}

[commit-bot: I haz the power, 2017-06-15 20:19:38]

Description was changed from

==========
[cast_channel] Clean up CastSocketImpl ctor parameters and ChannelAuthType usage

- Remove |owner_extension_id| (not used anymore) and |channel_auth| (should
always be SSL_VERIFIED) from CastSocketImpl's ctor
- Remove ChannelAuthType from cast_channel_enum.h and clean up its usage
thoughout the code

BUG=732669
==========

to

==========
[cast_channel] Clean up CastSocketImpl ctor parameters and ChannelAuthType usage

- Remove |owner_extension_id| (not used anymore) and |channel_auth| (should
always be SSL_VERIFIED) from CastSocketImpl's ctor
- Remove ChannelAuthType from cast_channel_enum.h and clean up its usage
thoughout the code

BUG=732669

Review-Url: https://codereview.chromium.org/2942743002
Cr-Commit-Position: refs/heads/master@{#479807}
Committed:
https://chromium.googlesource.com/chromium/src/+/ebd6f8fd664d851ccc571bbd80ec...
==========

[commit-bot: I haz the power, 2017-06-15 20:19:42]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ebd6f8fd664d851ccc571bbd80ec...