Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(509)

Issue 2869053002: Merge NetworkListViewBase hierarchy and NetworkStateListDetailedView (Closed)

Created:
3 years, 7 months ago by mohsen
Modified:
3 years, 7 months ago
Reviewers:
tdanderson, stevenjb
CC:
chromium-reviews, kalyank, sadrul, tdanderson
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Merge NetworkListViewBase hierarchy and NetworkStateListDetailedView Currently, NetworkListViewBase has two subclasses: NetworkListView for network detailed view and VPNListView for vpn detailed view. The actual detailed view is NetworkStateListDetailedView which delegates most of its functionalities to NetworkListViewBase sublcass instance it owns. This delegation seems unnecessary and prevents some code sharing with other detailed views. This CL removes NetworkListViewBase class and changes its two subclasses to inherit from NetworkStateListDetailedView. So, common functionality of these two detailed views will reside in NetworkStateListDetailedView and its two sublcasses will implement specific functionalities of each detailed view. BUG=686343 TEST=none Review-Url: https://codereview.chromium.org/2869053002 Cr-Commit-Position: refs/heads/master@{#470554} Committed: https://chromium.googlesource.com/chromium/src/+/710a8606de93326c2121a1c6970af7a9e800c258

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -235 lines) Patch
M ash/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M ash/system/network/network_list.h View 3 chunks +8 lines, -7 lines 0 comments Download
M ash/system/network/network_list.cc View 13 chunks +20 lines, -26 lines 0 comments Download
D ash/system/network/network_list_view_base.h View 1 chunk +0 lines, -58 lines 0 comments Download
D ash/system/network/network_list_view_base.cc View 1 chunk +0 lines, -19 lines 0 comments Download
M ash/system/network/network_state_list_detailed_view.h View 5 chunks +16 lines, -17 lines 0 comments Download
M ash/system/network/network_state_list_detailed_view.cc View 8 chunks +10 lines, -62 lines 0 comments Download
M ash/system/network/tray_network.h View 3 chunks +3 lines, -3 lines 0 comments Download
M ash/system/network/tray_network.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M ash/system/network/tray_network_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/system/network/tray_vpn.h View 2 chunks +2 lines, -2 lines 0 comments Download
M ash/system/network/tray_vpn.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M ash/system/network/vpn_list_view.h View 4 chunks +7 lines, -7 lines 0 comments Download
M ash/system/network/vpn_list_view.cc View 11 chunks +18 lines, -24 lines 0 comments Download
M ash/system/tray/hover_highlight_view.h View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/networking_config_delegate_chromeos_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 26 (17 generated)
mohsen
Please take a look...
3 years, 7 months ago (2017-05-09 04:11:53 UTC) #4
tdanderson
stevenjb@, can you please look at this instead? I will be OOO for the next ...
3 years, 7 months ago (2017-05-09 21:49:25 UTC) #8
stevenjb
Aaaand we're back where we started (more or less) :) This lgtm. I won't pretend ...
3 years, 7 months ago (2017-05-09 23:25:15 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2869053002/1
3 years, 7 months ago (2017-05-10 01:00:28 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; ...
3 years, 7 months ago (2017-05-10 03:04:54 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2869053002/1
3 years, 7 months ago (2017-05-10 03:23:46 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/422533)
3 years, 7 months ago (2017-05-10 04:58:53 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2869053002/1
3 years, 7 months ago (2017-05-10 13:00:19 UTC) #23
commit-bot: I haz the power
3 years, 7 months ago (2017-05-10 13:45:42 UTC) #26
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/710a8606de93326c2121a1c6970a...

Powered by Google App Engine
This is Rietveld 408576698