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}

ash/BUILD.gn

@@ -380,7 +380,6 @@ component("ash") {
     "system/network/network_info.h",
     "system/network/network_list.cc",
     "system/network/network_list.h",
-    "system/network/network_list_delegate.h",
     "system/network/network_list_view_base.cc",
     "system/network/network_list_view_base.h",
     "system/network/network_observer.h",

ash/system/network/network_list.h

@@ -26,9 +26,8 @@ class View;
 }
 
 namespace ash {
-
+class HoverHighlightView;
 struct NetworkInfo;
-class NetworkListDelegate;
 class TriView;
 
 // A list of available networks of a given type. This class is used for all
@@ -38,7 +37,7 @@ class NetworkListView : public NetworkListViewBase,
  public:
   class SectionHeaderRowView;
 
-  explicit NetworkListView(NetworkListDelegate* delegate);
+  explicit NetworkListView(tray::NetworkStateListDetailedView* detailed_view);
   ~NetworkListView() override;
 
   // NetworkListViewBase:
@@ -72,6 +71,18 @@ class NetworkListView : public NetworkListViewBase,
   // being used.
   TriView* CreateConnectionWarning();
 
+  // Creates and returns a View with the information in |info|.
+  HoverHighlightView* CreateViewForNetwork(const NetworkInfo& info);
+
+  // Updates |view| with the information in |info|. Note that |view| is
+  // guaranteed to be a View returned from |CreateViewForNetwork()|.
+  void UpdateViewForNetwork(HoverHighlightView* view, const NetworkInfo& info);
+
+  // Creates the view of an extra icon appearing next to the network name
+  // indicating that the network is controlled by an extension. If no extension
+  // is registered for this network, returns |nullptr|.
+  views::View* CreateControlledByExtensionView(const NetworkInfo& info);
+
   // Adds or updates child views representing the network connections when
   // |is_wifi| is matching the attribute of a network connection starting at
   // |child_index|. Returns a set of guids for the added network
@@ -112,7 +123,6 @@ class NetworkListView : public NetworkListViewBase,
   bool NeedUpdateViewForNetwork(const NetworkInfo& info) const;
 
   bool needs_relayout_;
-  NetworkListDelegate* delegate_;
 
   views::Label* no_wifi_networks_view_;
   views::Label* no_cellular_networks_view_;
@@ -131,7 +141,7 @@ class NetworkListView : public NetworkListViewBase,
   NetworkMap network_map_;
 
   // A map of network guids to their view.
-  using NetworkGuidMap = std::map<std::string, views::View*>;
+  using NetworkGuidMap = std::map<std::string, HoverHighlightView*>;
   NetworkGuidMap network_guid_map_;
 
   // Save a map of network guids to their infos against current |network_list_|.

ash/system/network/network_list_delegate.h

-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef ASH_SYSTEM_NETWORK_NETWORK_LIST_DELEGATE_H_
-#define ASH_SYSTEM_NETWORK_NETWORK_LIST_DELEGATE_H_
-
-namespace chromeos {
-class NetworkTypePattern;
-}
-
-namespace views {
-class Label;
-class View;
-}
-
-namespace ash {
-
-struct NetworkInfo;
-
-class NetworkListDelegate {
- public:
-  virtual ~NetworkListDelegate() {}
-
-  // Creates and returns a View with the information in |info|.
-  virtual views::View* CreateViewForNetwork(const NetworkInfo& info) = 0;
-
-  // Returns the type of network this list should use.
-  virtual chromeos::NetworkTypePattern GetNetworkTypePattern() const = 0;
-
-  // Updates |view| with the information in |info|. Note that |view| is
-  // guaranteed to be a View returned from |CreateViewForNetwork()|.
-  virtual void UpdateViewForNetwork(views::View* view,
-                                    const NetworkInfo& info) = 0;
-
-  // Creates a Label to be displayed in the list to present some information
-  // (e.g. unavailability of network etc.).
-  virtual views::Label* CreateInfoLabel() = 0;
-
-  // Called when the user clicks on an entry representing a network in the list.
-  virtual void OnNetworkEntryClicked(views::View* sender) = 0;
-
-  // Called when the user clicks on a "Join Other" button.
-  virtual void OnOtherWifiClicked() = 0;
-
-  virtual void RelayoutScrollList() = 0;
-};
-
-}  // namespace ash
-
-#endif  // ASH_SYSTEM_NETWORK_NETWORK_LIST_DELEGATE_H_

ash/system/network/network_list_view_base.cc

@@ -4,9 +4,15 @@
 
 #include "ash/system/network/network_list_view_base.h"
 
+#include "base/logging.h"
+
 namespace ash {
 
-NetworkListViewBase::NetworkListViewBase() {}
+NetworkListViewBase::NetworkListViewBase(
+    tray::NetworkStateListDetailedView* detailed_view)
+    : detailed_view_(detailed_view) {
+  DCHECK(detailed_view_);
+}
 
 NetworkListViewBase::~NetworkListViewBase() {}
 

ash/system/network/network_list_view_base.h

@@ -14,12 +14,14 @@ class View;
 }
 
 namespace ash {
+namespace tray {
+class NetworkStateListDetailedView;
+}
 
 // Base class for a list of available networks (and, in the case of VPNs, the
 // list of available VPN providers).
 class NetworkListViewBase {
  public:
-  NetworkListViewBase();
   virtual ~NetworkListViewBase();
 
   void set_container(views::View* container) { container_ = container; }
@@ -33,9 +35,18 @@ class NetworkListViewBase {
   virtual bool IsNetworkEntry(views::View* view, std::string* guid) const = 0;
 
  protected:
-  views::View* container() { return container_; }
+  explicit NetworkListViewBase(
+      tray::NetworkStateListDetailedView* detailed_view);
+
+  tray::NetworkStateListDetailedView* detailed_view() const {
+    return detailed_view_;
+  }
+
+  views::View* container() const { return container_; }
 
  private:
+  tray::NetworkStateListDetailedView* const detailed_view_;
+
   // The container that holds the actual list entries.
   views::View* container_ = nullptr;
 

ash/system/network/network_state_list_detailed_view.cc

@@ -17,19 +17,13 @@
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_icon_animation.h"
-#include "ash/system/network/network_info.h"
 #include "ash/system/network/network_list.h"
 #include "ash/system/network/network_list_view_base.h"
 #include "ash/system/network/tray_network_state_observer.h"
 #include "ash/system/network/vpn_list_view.h"
-#include "ash/system/networking_config_delegate.h"
-#include "ash/system/tray/fixed_sized_image_view.h"
-#include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/system_menu_button.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_controller.h"
-#include "ash/system/tray/system_tray_delegate.h"
-#include "ash/system/tray/throbber_view.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_details_view.h"
 #include "ash/system/tray/tray_popup_header_button.h"
@@ -79,30 +73,6 @@ namespace {
 // Delay between scan requests.
 const int kRequestScanDelaySeconds = 10;
 
-// TODO(varkha): Consolidate with a similar method in tray_bluetooth.cc.
-void SetupConnectedItem(HoverHighlightView* container,
-                        const base::string16& text,
-                        const gfx::ImageSkia& image) {
-  container->AddIconAndLabels(
-      image, text,
-      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED));
-  TrayPopupItemStyle style(TrayPopupItemStyle::FontStyle::CAPTION);
-  style.set_color_style(TrayPopupItemStyle::ColorStyle::CONNECTED);
-  style.SetupLabel(container->sub_text_label());
-}
-
-// TODO(varkha): Consolidate with a similar method in tray_bluetooth.cc.
-void SetupConnectingItem(HoverHighlightView* container,
-                         const base::string16& text,
-                         const gfx::ImageSkia& image) {
-  container->AddIconAndLabels(
-      image, text,
-      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING));
-  ThrobberView* throbber = new ThrobberView;
-  throbber->Start();
-  container->AddRightView(throbber);
-}
-
 }  // namespace
 
 //------------------------------------------------------------------------------
@@ -445,32 +415,6 @@ views::View* NetworkStateListDetailedView::CreateNetworkInfoView() {
   return label;
 }
 
-views::View* NetworkStateListDetailedView::CreateControlledByExtensionView(
-    const NetworkInfo& info) {
-  NetworkingConfigDelegate* networking_config_delegate =
-      Shell::Get()->system_tray_delegate()->GetNetworkingConfigDelegate();
-  if (!networking_config_delegate)
-    return nullptr;
-  std::unique_ptr<const NetworkingConfigDelegate::ExtensionInfo>
-      extension_info =
-          networking_config_delegate->LookUpExtensionForNetwork(info.guid);
-  if (!extension_info)
-    return nullptr;
-
-  // Get the tooltip text.
-  base::string16 tooltip_text = l10n_util::GetStringFUTF16(
-      IDS_ASH_STATUS_TRAY_EXTENSION_CONTROLLED_WIFI,
-      base::UTF8ToUTF16(extension_info->extension_name));
-
-  views::ImageView* controlled_icon =
-      new FixedSizedImageView(kTrayPopupDetailsIconWidth, 0);
-
-  controlled_icon->SetImage(
-      gfx::CreateVectorIcon(kCaptivePortalIcon, kMenuIconColor));
-  controlled_icon->SetTooltipText(tooltip_text);
-  return controlled_icon;
-}
-
 void NetworkStateListDetailedView::CallRequestScan() {
   VLOG(1) << "Requesting Network Scan.";
   NetworkHandler::Get()->network_state_handler()->RequestScan();
@@ -481,64 +425,10 @@ void NetworkStateListDetailedView::CallRequestScan() {
       base::TimeDelta::FromSeconds(kRequestScanDelaySeconds));
 }
 
-views::View* NetworkStateListDetailedView::CreateViewForNetwork(
-    const NetworkInfo& info) {
-  HoverHighlightView* container = new HoverHighlightView(this);
-  if (info.connected)
-    SetupConnectedItem(container, info.label, info.image);
-  else if (info.connecting)
-    SetupConnectingItem(container, info.label, info.image);
-  else
-    container->AddIconAndLabel(info.image, info.label);
-  container->SetTooltipText(info.tooltip);
-  views::View* controlled_icon = CreateControlledByExtensionView(info);
-  if (controlled_icon)
-    container->AddChildView(controlled_icon);
-  return container;
-}
-
-NetworkTypePattern NetworkStateListDetailedView::GetNetworkTypePattern() const {
-  return list_type_ == LIST_TYPE_VPN ? NetworkTypePattern::VPN()
-                                     : NetworkTypePattern::NonVirtual();
-}
-
-void NetworkStateListDetailedView::UpdateViewForNetwork(
-    views::View* view,
-    const NetworkInfo& info) {
-  HoverHighlightView* container = static_cast<HoverHighlightView*>(view);
-  DCHECK(!container->has_children());
-  if (info.connected)
-    SetupConnectedItem(container, info.label, info.image);
-  else if (info.connecting)
-    SetupConnectingItem(container, info.label, info.image);
-  else
-    container->AddIconAndLabel(info.image, info.label);
-  views::View* controlled_icon = CreateControlledByExtensionView(info);
-  container->SetTooltipText(info.tooltip);
-  if (controlled_icon)
-    view->AddChildView(controlled_icon);
-}
-
-views::Label* NetworkStateListDetailedView::CreateInfoLabel() {
-  views::Label* label = new views::Label();
-  label->SetBorder(views::CreateEmptyBorder(kTrayPopupPaddingBetweenItems,
-                                            kTrayPopupPaddingHorizontal,
-                                            kTrayPopupPaddingBetweenItems, 0));
-  label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  label->SetEnabledColor(SkColorSetARGB(192, 0, 0, 0));
-  return label;
-}
-
 void NetworkStateListDetailedView::OnNetworkEntryClicked(views::View* sender) {
   HandleViewClicked(sender);
 }
 
-void NetworkStateListDetailedView::OnOtherWifiClicked() {
-  ShellPort::Get()->RecordUserMetricsAction(
-      UMA_STATUS_AREA_NETWORK_JOIN_OTHER_CLICKED);
-  Shell::Get()->system_tray_controller()->ShowNetworkCreate(shill::kTypeWifi);
-}
-
 void NetworkStateListDetailedView::RelayoutScrollList() {
   scroller()->Layout();
 }

ash/system/network/network_state_list_detailed_view.h

@@ -9,7 +9,6 @@
 #include <string>
 
 #include "ash/login_status.h"
-#include "ash/system/network/network_list_delegate.h"
 #include "ash/system/tray/tray_details_view.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -17,10 +16,6 @@
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/custom_button.h"
 
-namespace chromeos {
-class NetworkTypePattern;
-}
-
 namespace ash {
 class NetworkListViewBase;
 }
@@ -36,7 +31,6 @@ namespace tray {
 
 class NetworkStateListDetailedView
     : public TrayDetailsView,
-      public NetworkListDelegate,
       public base::SupportsWeakPtr<NetworkStateListDetailedView> {
  public:
   enum ListType { LIST_TYPE_NETWORK, LIST_TYPE_VPN };
@@ -52,6 +46,11 @@ class NetworkStateListDetailedView
   // Manager properties (e.g. technology state) have changed.
   void Update();
 
+  // Called when the user clicks on an entry representing a network in the list.
+  void OnNetworkEntryClicked(views::View* sender);
+
+  void RelayoutScrollList();
+
  private:
   class InfoBubble;
 
@@ -74,24 +73,9 @@ class NetworkStateListDetailedView
   void OnInfoBubbleDestroyed();
   views::View* CreateNetworkInfoView();
 
-  // Creates the view of an extra icon appearing next to the network name
-  // indicating that the network is controlled by an extension. If no extension
-  // is registered for this network, returns |nullptr|.
-  views::View* CreateControlledByExtensionView(const NetworkInfo& info);
-
   // Periodically request a network scan.
   void CallRequestScan();
 
-  // NetworkListDelegate:
-  views::View* CreateViewForNetwork(const NetworkInfo& info) override;
-  chromeos::NetworkTypePattern GetNetworkTypePattern() const override;
-  void UpdateViewForNetwork(views::View* view,
-                            const NetworkInfo& info) override;
-  views::Label* CreateInfoLabel() override;
-  void OnNetworkEntryClicked(views::View* sender) override;
-  void OnOtherWifiClicked() override;
-  void RelayoutScrollList() override;
-
   // Type of list (all networks or vpn)
   ListType list_type_;
 

ash/system/network/vpn_list_view.cc

@@ -16,7 +16,7 @@
 #include "ash/system/network/network_icon.h"
 #include "ash/system/network/network_icon_animation.h"
 #include "ash/system/network/network_icon_animation_observer.h"
-#include "ash/system/network/network_list_delegate.h"
+#include "ash/system/network/network_state_list_detailed_view.h"
 #include "ash/system/network/vpn_list.h"
 #include "ash/system/tray/hover_highlight_view.h"
 #include "ash/system/tray/system_menu_button.h"
@@ -220,7 +220,8 @@ void VPNListNetworkEntry::UpdateFromNetworkState(
   Layout();
 }
 
-// TODO(varkha): Consolidate with a similar method in tray_bluetooth.cc.
+// TODO(varkha|mohsen): Consolidate with a similar method in
+// BluetoothDetailedView. See https://crbug.com/686924.
 void VPNListNetworkEntry::SetupConnectedItem(const base::string16& text,
                                              const gfx::ImageSkia& image) {
   AddIconAndLabels(
@@ -231,7 +232,8 @@ void VPNListNetworkEntry::SetupConnectedItem(const base::string16& text,
   style.SetupLabel(sub_text_label());
 }
 
-// TODO(varkha): Consolidate with a similar method in tray_bluetooth.cc.
+// TODO(varkha|mohsen): Consolidate with a similar method in
+// BluetoothDetailedView. See https://crbug.com/686924.
 void VPNListNetworkEntry::SetupConnectingItem(const base::string16& text,
                                               const gfx::ImageSkia& image) {
   AddIconAndLabels(
@@ -244,7 +246,8 @@ void VPNListNetworkEntry::SetupConnectingItem(const base::string16& text,
 
 }  // namespace
 
-VPNListView::VPNListView(NetworkListDelegate* delegate) : delegate_(delegate) {
+VPNListView::VPNListView(tray::NetworkStateListDetailedView* detailed_view)
+    : NetworkListViewBase(detailed_view) {
   Shell::Get()->vpn_list()->AddObserver(this);
 }
 
@@ -314,7 +317,7 @@ void VPNListView::Update() {
 
   // Layout the updated list.
   container()->SizeToPreferredSize();
-  delegate_->RelayoutScrollList();
+  detailed_view()->RelayoutScrollList();
 
   if (scroll_to_show_view) {
     // Scroll the list so that |scroll_to_show_view| is in view.
@@ -357,7 +360,7 @@ void VPNListView::OnViewClicked(views::View* sender) {
   // If the user clicked on a network entry, let the |delegate_| trigger a
   // connection attempt (if the network is currently disconnected) or show a
   // configuration dialog (if the network is currently connected or connecting).
-  delegate_->OnNetworkEntryClicked(sender);
+  detailed_view()->OnNetworkEntryClicked(sender);
 }
 
 void VPNListView::AddNetwork(const chromeos::NetworkState* network) {

ash/system/network/vpn_list_view.h

@@ -18,15 +18,14 @@ namespace chromeos {
 class NetworkState;
 }
 
-namespace ash {
-class NetworkListDelegate;
-}
-
 namespace views {
 class View;
 }
 
 namespace ash {
+namespace tray {
+class NetworkStateListDetailedView;
+}
 
 // A list of VPN providers and networks that shows VPN providers and networks in
 // a hierarchical layout, allowing the user to see at a glance which provider a
@@ -45,7 +44,7 @@ class VPNListView : public NetworkListViewBase,
                     public VpnList::Observer,
                     public ViewClickListener {
  public:
-  explicit VPNListView(NetworkListDelegate* delegate);
+  explicit VPNListView(tray::NetworkStateListDetailedView* detailed_view);
   ~VPNListView() override;
 
   // NetworkListViewBase:
@@ -72,8 +71,6 @@ class VPNListView : public NetworkListViewBase,
   void AddProvidersAndNetworks(
       const chromeos::NetworkStateHandler::NetworkStateList& networks);
 
-  NetworkListDelegate* const delegate_;
-
   // A mapping from each VPN provider's list entry to the provider.
   std::map<const views::View* const, VPNProvider> provider_view_map_;