Skip to content

GitLab

T
tangled
Commit 9f7c8a88 authored by Steven Bennetts's avatar Steven Bennetts Committed by Commit Bot
Browse files
Options

Cleanup NetworkStateListDetailedView 

Multiple refactorings made this class and its inheritors a bit tricky
to follow. This CL removes some duplication and moves observers closer
to where they are used.

Bug: 862420

Change-Id: Ibfe1424b621a478f3ec5020b9f3d453cff79b94e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752773
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686832}
parent 2c1f2716
Hide whitespace changes
Inline Side-by-side
Showing with 59 additions and 89 deletions
ash/system/network/network_list_view.cc
View file @ 9f7c8a88
......@@ -12,7 +12,6 @@
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/network_icon.h"
#include "ash/system/network/network_icon_animation.h"
#include "ash/system/network/network_info.h"
......@@ -75,8 +74,7 @@ bool IsSecondaryUser() {
NetworkListView::NetworkListView(DetailedViewDelegate* delegate,
LoginStatus login)
: NetworkStateListDetailedView(delegate, LIST_TYPE_NETWORK, login),
model_(Shell::Get()->system_tray_model()->network_state_model()) {}
: NetworkStateListDetailedView(delegate, LIST_TYPE_NETWORK, login) {}
NetworkListView::~NetworkListView() {
network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this);
......@@ -84,7 +82,7 @@ NetworkListView::~NetworkListView() {
void NetworkListView::UpdateNetworkList() {
CHECK(scroll_content());
model_->cros_network_config()->GetNetworkStateList(
model()->cros_network_config()->GetNetworkStateList(
NetworkFilter::New(FilterType::kVisible, NetworkType::kAll,
chromeos::network_config::mojom::kNoLimit),
base::BindOnce(&NetworkListView::OnGetNetworkStateList,
......@@ -131,7 +129,7 @@ void NetworkListView::OnGetNetworkStateList(
network->cellular ? network->cellular->activation_state
: ActivationStateType::kUnknown;
if (network->type == NetworkType::kCellular &&
model_->GetDeviceState(NetworkType::kCellular) !=
model()->GetDeviceState(NetworkType::kCellular) !=
DeviceStateType::kEnabled &&
activation_state == ActivationStateType::kNoService) {
continue;
......@@ -223,7 +221,7 @@ NetworkListView::UpdateNetworkListEntries() {
// Keep an index where the next child should be inserted.
int index = 0;
const NetworkStateProperties* default_network = model_->default_network();
const NetworkStateProperties* default_network = model()->default_network();
bool using_proxy = default_network &&
default_network->proxy_mode == ProxyMode::kFixedServers;
// Show a warning that the connection might be monitored if connected to a VPN
......@@ -270,7 +268,7 @@ NetworkListView::UpdateNetworkListEntries() {
wifi_header_view_ = new WifiSectionHeaderView();
bool wifi_enabled =
model_->GetDeviceState(NetworkType::kWiFi) == DeviceStateType::kEnabled;
model()->GetDeviceState(NetworkType::kWiFi) == DeviceStateType::kEnabled;
index = UpdateNetworkSectionHeader(NetworkType::kWiFi, wifi_enabled, index,
wifi_header_view_, &wifi_separator_view_);
......@@ -308,12 +306,12 @@ NetworkListView::UpdateNetworkListEntries() {
bool NetworkListView::ShouldMobileDataSectionBeShown() {
// The section should always be shown if Cellular networks are available.
if (model_->GetDeviceState(NetworkType::kCellular) !=
if (model()->GetDeviceState(NetworkType::kCellular) !=
DeviceStateType::kUnavailable) {
return true;
}
DeviceStateType tether_state = model_->GetDeviceState(NetworkType::kTether);
DeviceStateType tether_state = model()->GetDeviceState(NetworkType::kTether);
// Hide the section if both Cellular and Tether are UNAVAILABLE.
if (tether_state == DeviceStateType::kUnavailable)
return false;
......
ash/system/network/network_list_view.h
View file @ 9f7c8a88
......@@ -28,7 +28,6 @@ namespace ash {
class HoverHighlightView;
class TrayInfoLabel;
class TriView;
class TrayNetworkStateModel;
namespace tray {
class NetworkSectionHeaderView;
......@@ -129,8 +128,6 @@ class NetworkListView : public NetworkStateListDetailedView,
// otherwise false.
bool NeedUpdateViewForNetwork(const NetworkInfo& info) const;
TrayNetworkStateModel* model_;
bool needs_relayout_ = false;
// Owned by the views heirarchy.
......
ash/system/network/network_state_list_detailed_view.cc
View file @ 9f7c8a88
......@@ -12,7 +12,6 @@
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/tray_network_state_model.h"
#include "ash/system/tray/system_menu_button.h"
#include "ash/system/tray/tri_view.h"
#include "base/strings/utf_string_conversions.h"
......@@ -202,18 +201,12 @@ NetworkStateListDetailedView::NetworkStateListDetailedView(
info_bubble_(nullptr) {}
NetworkStateListDetailedView::~NetworkStateListDetailedView() {
model_->RemoveObserver(this);
if (info_bubble_)
info_bubble_->OnNetworkStateListDetailedViewIsDeleting();
ResetInfoBubble();
}
void NetworkStateListDetailedView::Update() {
UpdateNetworkList();
UpdateHeaderButtons();
UpdateScanningBar();
Layout();
}
void NetworkStateListDetailedView::ToggleInfoBubbleForTesting() {
ToggleInfoBubble();
}
......@@ -228,12 +221,28 @@ void NetworkStateListDetailedView::Init() {
? IDS_ASH_STATUS_TRAY_NETWORK
: IDS_ASH_STATUS_TRAY_VPN);
model_->AddObserver(this);
Update();
if (list_type_ == LIST_TYPE_NETWORK && IsWifiEnabled())
ScanAndStartTimer();
}
void NetworkStateListDetailedView::Update() {
UpdateNetworkList();
UpdateHeaderButtons();
UpdateScanningBar();
Layout();
}
void NetworkStateListDetailedView::ActiveNetworkStateChanged() {
Update();
}
void NetworkStateListDetailedView::NetworkListChanged() {
Update();
}
void NetworkStateListDetailedView::HandleButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == info_button_) {
......
ash/system/network/network_state_list_detailed_view.h
View file @ 9f7c8a88
......@@ -8,6 +8,7 @@
#include <string>
#include "ash/login_status.h"
#include "ash/system/network/tray_network_state_model.h"
#include "ash/system/tray/tray_detailed_view.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
......@@ -19,22 +20,17 @@ class Button;
}
namespace ash {
class TrayNetworkStateModel;
namespace tray {
// Exported for tests.
class ASH_EXPORT NetworkStateListDetailedView : public TrayDetailedView {
class ASH_EXPORT NetworkStateListDetailedView
: public TrayDetailedView,
public TrayNetworkStateModel::Observer {
public:
~NetworkStateListDetailedView() override;
void Init();
// Called when the contents of the network list have changed or when any
// Manager properties (e.g. technology state) have changed.
void Update();
void ToggleInfoBubbleForTesting();
// views::View:
......@@ -55,9 +51,18 @@ class ASH_EXPORT NetworkStateListDetailedView : public TrayDetailedView {
// leaves |guid| unchanged and returns |false|.
virtual bool IsNetworkEntry(views::View* view, std::string* guid) const = 0;
// Called when the network model changes or when a network icon changes.
void Update();
TrayNetworkStateModel* model() { return model_; }
private:
class InfoBubble;
// TrayNetworkStateModel::Observer:
void ActiveNetworkStateChanged() override;
void NetworkListChanged() override;
// TrayDetailedView:
void HandleViewClicked(views::View* view) override;
void HandleButtonPressed(views::Button* sender,
......
ash/system/network/unified_network_detailed_view_controller.cc
View file @ 9f7c8a88
......@@ -6,7 +6,6 @@
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/network_list_view.h"
#include "ash/system/tray/detailed_view_delegate.h"
......@@ -16,13 +15,10 @@ UnifiedNetworkDetailedViewController::UnifiedNetworkDetailedViewController(
UnifiedSystemTrayController* tray_controller)
: detailed_view_delegate_(
std::make_unique<DetailedViewDelegate>(tray_controller)) {
Shell::Get()->system_tray_model()->network_state_model()->AddObserver(this);
}
UnifiedNetworkDetailedViewController::~UnifiedNetworkDetailedViewController() {
Shell::Get()->system_tray_model()->network_state_model()->RemoveObserver(
this);
}
UnifiedNetworkDetailedViewController::~UnifiedNetworkDetailedViewController() =
default;
views::View* UnifiedNetworkDetailedViewController::CreateView() {
DCHECK(!view_);
......@@ -33,14 +29,4 @@ views::View* UnifiedNetworkDetailedViewController::CreateView() {
return view_;
}
void UnifiedNetworkDetailedViewController::ActiveNetworkStateChanged() {
if (view_)
view_->Update();
}
void UnifiedNetworkDetailedViewController::NetworkListChanged() {
if (view_)
view_->Update();
}
} // namespace ash
ash/system/network/unified_network_detailed_view_controller.h
View file @ 9f7c8a88
......@@ -5,8 +5,10 @@
#ifndef ASH_SYSTEM_NETWORK_UNIFIED_NETWORK_DETAILED_VIEW_CONTROLLER_H_
#define ASH_SYSTEM_NETWORK_UNIFIED_NETWORK_DETAILED_VIEW_CONTROLLER_H_
#include "ash/system/network/tray_network_state_model.h"
#include <memory>
#include "ash/system/unified/detailed_view_controller.h"
#include "base/macros.h"
namespace ash {
......@@ -18,9 +20,7 @@ class DetailedViewDelegate;
class UnifiedSystemTrayController;
// Controller of Network detailed view in UnifiedSystemTray.
class UnifiedNetworkDetailedViewController
: public DetailedViewController,
public TrayNetworkStateModel::Observer {
class UnifiedNetworkDetailedViewController : public DetailedViewController {
public:
explicit UnifiedNetworkDetailedViewController(
UnifiedSystemTrayController* tray_controller);
......@@ -29,10 +29,6 @@ class UnifiedNetworkDetailedViewController
// DetailedViewControllerBase:
views::View* CreateView() override;
// TrayNetworkStateModel::Observer:
void ActiveNetworkStateChanged() override;
void NetworkListChanged() override;
private:
const std::unique_ptr<DetailedViewDelegate> detailed_view_delegate_;
......
ash/system/network/unified_vpn_detailed_view_controller.cc
View file @ 9f7c8a88
......@@ -6,7 +6,6 @@
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/system/model/system_tray_model.h"
#include "ash/system/network/vpn_list_view.h"
#include "ash/system/tray/detailed_view_delegate.h"
......@@ -16,13 +15,9 @@ UnifiedVPNDetailedViewController::UnifiedVPNDetailedViewController(
UnifiedSystemTrayController* tray_controller)
: detailed_view_delegate_(
std::make_unique<DetailedViewDelegate>(tray_controller)) {
Shell::Get()->system_tray_model()->network_state_model()->AddObserver(this);
}
UnifiedVPNDetailedViewController::~UnifiedVPNDetailedViewController() {
Shell::Get()->system_tray_model()->network_state_model()->RemoveObserver(
this);
}
UnifiedVPNDetailedViewController::~UnifiedVPNDetailedViewController() = default;
views::View* UnifiedVPNDetailedViewController::CreateView() {
DCHECK(!view_);
......@@ -33,14 +28,4 @@ views::View* UnifiedVPNDetailedViewController::CreateView() {
return view_;
}
void UnifiedVPNDetailedViewController::ActiveNetworkStateChanged() {
if (view_)
view_->Update();
}
void UnifiedVPNDetailedViewController::NetworkListChanged() {
if (view_)
view_->Update();
}
} // namespace ash
ash/system/network/unified_vpn_detailed_view_controller.h
View file @ 9f7c8a88
......@@ -5,8 +5,10 @@
#ifndef ASH_SYSTEM_NETWORK_UNIFIED_VPN_DETAILED_VIEW_CONTROLLER_H_
#define ASH_SYSTEM_NETWORK_UNIFIED_VPN_DETAILED_VIEW_CONTROLLER_H_
#include "ash/system/network/tray_network_state_model.h"
#include <memory>
#include "ash/system/unified/detailed_view_controller.h"
#include "base/macros.h"
namespace ash {
......@@ -18,9 +20,7 @@ class DetailedViewDelegate;
class UnifiedSystemTrayController;
// Controller of VPN detailed view in UnifiedSystemTray.
class UnifiedVPNDetailedViewController
: public DetailedViewController,
public TrayNetworkStateModel::Observer {
class UnifiedVPNDetailedViewController : public DetailedViewController {
public:
explicit UnifiedVPNDetailedViewController(
UnifiedSystemTrayController* tray_controller);
......@@ -29,10 +29,6 @@ class UnifiedVPNDetailedViewController
// DetailedViewControllerBase:
views::View* CreateView() override;
// TrayNetworkStateModel::Observer:
void ActiveNetworkStateChanged() override;
void NetworkListChanged() override;
private:
const std::unique_ptr<DetailedViewDelegate> detailed_view_delegate_;
......
ash/system/network/vpn_list_view.cc
View file @ 9f7c8a88
......@@ -173,6 +173,7 @@ class VPNListNetworkEntry : public HoverHighlightView,
public network_icon::AnimationObserver {
public:
VPNListNetworkEntry(VPNListView* vpn_list_view,
TrayNetworkStateModel* model,
const NetworkStateProperties* network);
~VPNListNetworkEntry() override;
......@@ -190,6 +191,7 @@ class VPNListNetworkEntry : public HoverHighlightView,
void UpdateFromNetworkState(const NetworkStateProperties* network);
VPNListView* const owner_;
TrayNetworkStateModel* model_;
const std::string guid_;
views::LabelButton* disconnect_button_ = nullptr;
......@@ -200,8 +202,12 @@ class VPNListNetworkEntry : public HoverHighlightView,
};
VPNListNetworkEntry::VPNListNetworkEntry(VPNListView* owner,
TrayNetworkStateModel* model,
const NetworkStateProperties* network)
: HoverHighlightView(owner), owner_(owner), guid_(network->guid) {
: HoverHighlightView(owner),
owner_(owner),
model_(model),
guid_(network->guid) {
UpdateFromNetworkState(network);
}
......@@ -210,7 +216,7 @@ VPNListNetworkEntry::~VPNListNetworkEntry() {
}
void VPNListNetworkEntry::NetworkIconChanged() {
owner_->model()->cros_network_config()->GetNetworkState(
model_->cros_network_config()->GetNetworkState(
guid_, base::BindOnce(&VPNListNetworkEntry::OnGetNetworkState,
weak_ptr_factory_.GetWeakPtr()));
}
......@@ -271,8 +277,7 @@ void VPNListNetworkEntry::UpdateFromNetworkState(
} // namespace
VPNListView::VPNListView(DetailedViewDelegate* delegate, LoginStatus login)
: NetworkStateListDetailedView(delegate, LIST_TYPE_VPN, login),
model_(Shell::Get()->system_tray_model()->network_state_model()) {
: NetworkStateListDetailedView(delegate, LIST_TYPE_VPN, login) {
Shell::Get()->vpn_list()->AddObserver(this);
}
......@@ -281,7 +286,7 @@ VPNListView::~VPNListView() {
}
void VPNListView::UpdateNetworkList() {
model_->cros_network_config()->GetNetworkStateList(
model()->cros_network_config()->GetNetworkStateList(
NetworkFilter::New(FilterType::kVisible, NetworkType::kVPN,
chromeos::network_config::mojom::kNoLimit),
base::BindOnce(&VPNListView::OnGetNetworkStateList,
......@@ -373,7 +378,7 @@ const char* VPNListView::GetClassName() const {
}
void VPNListView::AddNetwork(const NetworkStateProperties* network) {
views::View* entry(new VPNListNetworkEntry(this, network));
views::View* entry(new VPNListNetworkEntry(this, model(), network));
scroll_content()->AddChildView(entry);
network_view_guid_map_[entry] = network->guid;
list_empty_ = false;
......
ash/system/network/vpn_list_view.h
View file @ 9f7c8a88
......@@ -21,9 +21,6 @@ class View;
}
namespace ash {
class TrayNetworkStateModel;
namespace tray {
// A list of VPN providers and networks that shows VPN providers and networks in
......@@ -56,8 +53,6 @@ class VPNListView : public NetworkStateListDetailedView,
// VpnList::Observer:
void OnVPNProvidersChanged() override;
TrayNetworkStateModel* model() { return model_; }
// See Shell::RegisterProfilePrefs().
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
......@@ -95,8 +90,6 @@ class VPNListView : public NetworkStateListDetailedView,
// Adds all available VPN providers and networks to the list.
void AddProvidersAndNetworks(const NetworkStateList& networks);
TrayNetworkStateModel* model_;
// A mapping from each VPN provider's list entry to the provider.
std::map<const views::View* const, VPNProvider> provider_view_map_;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment