Simplify code for VPN entries in system menu

Simplify code for VPN entries in system menu

VPNListEntryBase, which is not needed anymore, is removed.

Also, VPN providers are directly added in VPNListProviderEntry instead
of delegating to VPNListView. This way VPNListView doesn't need to be a
ViewClickListener anymore which makes merging NetworkListViewBase
hierarchy into NetworkStateListDetailedView easier in follow-up CLs.

BUG=686343
TEST=none

Review-Url: https://codereview.chromium.org/2853143002
Cr-Commit-Position: refs/heads/master@{#469089}
Committed: https://chromium.googlesource.com/chromium/src/+/2a0c83a2260a7d321e1495153a892db1f0a4bc44

[mohsen, 2017-05-01 22:25:37]

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

[commit-bot: I haz the power, 2017-05-01 22:26:42]

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-05-01 23:03:44]

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

[commit-bot: I haz the power, 2017-05-01 23:03:48]

Dry run: This issue passed the CQ dry run.

[mohsen, 2017-05-02 01:58:03]

mohsen@chromium.org changed reviewers:
+ tdanderson@chromium.org

[mohsen, 2017-05-02 01:58:04]

Please take a look...

[tdanderson, 2017-05-03 18:31:40]

LGTM

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
File ash/system/network/network_state_list_detailed_view.h (right):

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
ash/system/network/network_state_list_detailed_view.h:4: 
nit: I think the number after BUG= is incorrect in the CL description.

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
ash/system/network/network_state_list_detailed_view.h:8: #include <memory>
nit: 'VPNListEntryBase' instead of 'VPNEntryListBase' in your CL description.

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
File ash/system/network/vpn_list_view.cc (right):

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
ash/system/network/vpn_list_view.cc:118: return;
Looks like an unnecessary return

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
ash/system/network/vpn_list_view.cc:122: const VPNProvider vpn_provider_;
& ?

[mohsen, 2017-05-03 18:58:08]

Description was changed from

==========
Simplify code for VPN entries in system menu

VPNEntryListBase, which is not needed anymore, is removed.

Also, VPN providers are directly added in VPNListProviderEntry instead
of delegating to VPNListView. This way VPNListView doesn't need to be a
ViewClickListener anymore which makes merging NetworkListViewBase
hierarchy into NetworkStateListDetailedView easier in follow-up CLs.

BUG=686434
TEST=none
==========

to

==========
Simplify code for VPN entries in system menu

VPNListEntryBase, which is not needed anymore, is removed.

Also, VPN providers are directly added in VPNListProviderEntry instead
of delegating to VPNListView. This way VPNListView doesn't need to be a
ViewClickListener anymore which makes merging NetworkListViewBase
hierarchy into NetworkStateListDetailedView easier in follow-up CLs.

BUG=686343
TEST=none
==========

[mohsen, 2017-05-03 19:14:04]

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

[mohsen, 2017-05-03 19:14:28]

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
File ash/system/network/network_state_list_detailed_view.h (right):

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
ash/system/network/network_state_list_detailed_view.h:4: 
On 2017/05/03 at 18:31:39, tdanderson wrote:
> nit: I think the number after BUG= is incorrect in the CL description.

Thanks for catching! Fixed.

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/network_...
ash/system/network/network_state_list_detailed_view.h:8: #include <memory>
On 2017/05/03 at 18:31:39, tdanderson wrote:
> nit: 'VPNListEntryBase' instead of 'VPNEntryListBase' in your CL description.

Thanks for catching! Fixed.

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
File ash/system/network/vpn_list_view.cc (right):

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
ash/system/network/vpn_list_view.cc:118: return;
On 2017/05/03 at 18:31:39, tdanderson wrote:
> Looks like an unnecessary return

Removed.

https://codereview.chromium.org/2853143002/diff/1/ash/system/network/vpn_list...
ash/system/network/vpn_list_view.cc:122: const VPNProvider vpn_provider_;
On 2017/05/03 at 18:31:39, tdanderson wrote:
> & ?

The original provider struct is destructed after the scroll list is populated
(it's a local variable in VPNListView::AddProvidersAndNetworks()). We need to
keep a copy (similar to |provider_view_map_| which keeps a copy).

[commit-bot: I haz the power, 2017-05-03 19:14:48]

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-05-03 19:51:45]

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

[commit-bot: I haz the power, 2017-05-03 19:51:46]

Dry run: This issue passed the CQ dry run.

[mohsen, 2017-05-03 19:53:38]

The CQ bit was checked by mohsen@chromium.org

[mohsen, 2017-05-03 19:53:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org
Link to the patchset: https://codereview.chromium.org/2853143002/#ps20001
(title: "Removed unnecessary return")

[commit-bot: I haz the power, 2017-05-03 19:54:13]

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-05-03 19:59:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493841218229210,
"parent_rev": "673dad144d4488b424fd08587a4142a6f6f6bcee", "commit_rev":
"2a0c83a2260a7d321e1495153a892db1f0a4bc44"}

[commit-bot: I haz the power, 2017-05-03 19:59:54]

Description was changed from

==========
Simplify code for VPN entries in system menu

VPNListEntryBase, which is not needed anymore, is removed.

Also, VPN providers are directly added in VPNListProviderEntry instead
of delegating to VPNListView. This way VPNListView doesn't need to be a
ViewClickListener anymore which makes merging NetworkListViewBase
hierarchy into NetworkStateListDetailedView easier in follow-up CLs.

BUG=686343
TEST=none
==========

to

==========
Simplify code for VPN entries in system menu

VPNListEntryBase, which is not needed anymore, is removed.

Also, VPN providers are directly added in VPNListProviderEntry instead
of delegating to VPNListView. This way VPNListView doesn't need to be a
ViewClickListener anymore which makes merging NetworkListViewBase
hierarchy into NetworkStateListDetailedView easier in follow-up CLs.

BUG=686343
TEST=none

Review-Url: https://codereview.chromium.org/2853143002
Cr-Commit-Position: refs/heads/master@{#469089}
Committed:
https://chromium.googlesource.com/chromium/src/+/2a0c83a2260a7d321e1495153a89...
==========

[commit-bot: I haz the power, 2017-05-03 19:59:55]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2a0c83a2260a7d321e1495153a89...