The accessibility, audio, and Bluetooth detailed views in the Ash MD system menu each implement their own (non-virtual) versions of AddScrollListItem(); there is more than likely a good refactoring opportunity here.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac commit 0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac Author: mohsen <mohsen@chromium.org> Date: Tue Apr 25 22:43:59 2017 Refactor AddScrollListItem() in system menu detailed views Most of system menu detailed views have a scrollable list of items with very similar layouts. This CL adds a few functions in TrayDetailsView to create such items. Among different detailed views, network and VPN are not using these common functions, yet, as they need to be cleaned up first. That will be done in follow-up CLs. BUG=686343 TEST=TrayDetailsViewTest.ScrollContentsTest and *TrayAccessibilityTest.CheckMarksOnDetailMenu* in ash_unittests Review-Url: https://codereview.chromium.org/2831023003 Cr-Commit-Position: refs/heads/master@{#467147} [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/audio/audio_detailed_view.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/audio/audio_detailed_view.h [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/bluetooth/tray_bluetooth.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/cast/tray_cast.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/ime_menu/ime_list_view.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/ime_menu/ime_list_view.h [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/network/network_list.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/network/vpn_list_view.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/hover_highlight_view.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/hover_highlight_view.h [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/tray_details_view.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/tray_details_view.h [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/tray_details_view_unittest.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/tray_popup_utils.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray/tray_popup_utils.h [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray_accessibility.cc [modify] https://crrev.com/0b6712fcc4a9d0cd07624db4b3fac5c67edb2bac/ash/system/tray_accessibility.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6f5e538e663781caf96718cdd72c5154404179c commit c6f5e538e663781caf96718cdd72c5154404179c Author: mohsen <mohsen@chromium.org> Date: Sat Apr 29 05:50:54 2017 Remove NetworkListDelegate NetworkListDelegate was introduced when parts of network UI were living in a different component. Now that everything is in //ash/system/network there is no need for the delegate. NetworkListView and VPNListView can access its methods directly from NetworkStateListDetailedView. Also, some of the functionality was only used in NetworkListView and moved to that class. This is the first step to clean up code for network and VPN detailed views necessary to share code between all detailed views. BUG=686343 TEST=none Review-Url: https://codereview.chromium.org/2843163003 Cr-Commit-Position: refs/heads/master@{#468227} [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/BUILD.gn [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_list.cc [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_list.h [delete] https://crrev.com/00f09058b11938a3dfc685e1ae41f4019241b215/ash/system/network/network_list_delegate.h [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_list_view_base.cc [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_list_view_base.h [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_state_list_detailed_view.cc [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/network_state_list_detailed_view.h [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/vpn_list_view.cc [modify] https://crrev.com/c6f5e538e663781caf96718cdd72c5154404179c/ash/system/network/vpn_list_view.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a0c83a2260a7d321e1495153a892db1f0a4bc44 commit 2a0c83a2260a7d321e1495153a892db1f0a4bc44 Author: mohsen <mohsen@chromium.org> Date: Wed May 03 19:59:43 2017 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} [modify] https://crrev.com/2a0c83a2260a7d321e1495153a892db1f0a4bc44/ash/system/network/network_state_list_detailed_view.cc [modify] https://crrev.com/2a0c83a2260a7d321e1495153a892db1f0a4bc44/ash/system/network/network_state_list_detailed_view.h [modify] https://crrev.com/2a0c83a2260a7d321e1495153a892db1f0a4bc44/ash/system/network/vpn_list_view.cc [modify] https://crrev.com/2a0c83a2260a7d321e1495153a892db1f0a4bc44/ash/system/network/vpn_list_view.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/710a8606de93326c2121a1c6970af7a9e800c258 commit 710a8606de93326c2121a1c6970af7a9e800c258 Author: mohsen <mohsen@chromium.org> Date: Wed May 10 13:45:27 2017 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} [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/BUILD.gn [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/network_list.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/network_list.h [delete] https://crrev.com/b7599ca4415f3dd4325fa31cb4c37704d68e3cca/ash/system/network/network_list_view_base.cc [delete] https://crrev.com/b7599ca4415f3dd4325fa31cb4c37704d68e3cca/ash/system/network/network_list_view_base.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/network_state_list_detailed_view.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/network_state_list_detailed_view.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/tray_network.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/tray_network.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/tray_network_unittest.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/tray_vpn.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/tray_vpn.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/vpn_list_view.cc [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/network/vpn_list_view.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/ash/system/tray/hover_highlight_view.h [modify] https://crrev.com/710a8606de93326c2121a1c6970af7a9e800c258/chrome/browser/ui/ash/networking_config_delegate_chromeos_browsertest.cc
Comment 1 by tdander...@chromium.org
, Jan 28 2017