diff options
author | stevenjb <stevenjb@chromium.org> | 2015-01-08 17:46:56 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-09 01:47:49 +0000 |
commit | aaa1265b918ee6195ffc84caf970d509bec18b32 (patch) | |
tree | ba89de149afd69f971edecb07ed5878c2d5cf583 /ash | |
parent | cfb751267474d602441c22f8136ba77f28549368 (diff) | |
download | chromium_src-aaa1265b918ee6195ffc84caf970d509bec18b32.zip chromium_src-aaa1265b918ee6195ffc84caf970d509bec18b32.tar.gz chromium_src-aaa1265b918ee6195ffc84caf970d509bec18b32.tar.bz2 |
Add logging for slow device events, limit network UI update rate
This CL does the following:
* Cleans up some redundant code in the network UI.
* Disassociates Network UI updates from dbus calls and limits the
frequency of UI updates by using a timer to trigger updates.
* Adds NET_LOG_IF_SLOW to device_event_log.h to log slow network events.
Note: I also did virtual/override cleanup while in there and ran clang format on the changed files.
Original CL (reverted):
https://codereview.chromium.org/811623002
BUG=441650
Review URL: https://codereview.chromium.org/800893003
Cr-Commit-Position: refs/heads/master@{#310659}
Diffstat (limited to 'ash')
-rw-r--r-- | ash/system/chromeos/network/network_detailed_view.h | 13 | ||||
-rw-r--r-- | ash/system/chromeos/network/network_state_list_detailed_view.cc | 138 | ||||
-rw-r--r-- | ash/system/chromeos/network/network_state_list_detailed_view.h | 36 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_network.cc | 120 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_network.h | 28 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_network_state_observer.cc | 56 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_network_state_observer.h | 40 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_vpn.cc | 28 | ||||
-rw-r--r-- | ash/system/chromeos/network/tray_vpn.h | 23 |
9 files changed, 209 insertions, 273 deletions
diff --git a/ash/system/chromeos/network/network_detailed_view.h b/ash/system/chromeos/network/network_detailed_view.h index 772f239..0d17c7a 100644 --- a/ash/system/chromeos/network/network_detailed_view.h +++ b/ash/system/chromeos/network/network_detailed_view.h @@ -29,17 +29,10 @@ class NetworkDetailedView : public TrayDetailsView { virtual DetailedViewType GetViewType() const = 0; - // Called when network manager state has changed. - // (Generic update for NetworkTray <> AshSystemTrayDelegate interface). - virtual void ManagerChanged() = 0; - - // Called when the contents of the network list have changed. - // (Called only from TrayNetworkStateObserver). - virtual void NetworkListChanged() = 0; - - // Called when a network service property has changed. + // Called when the contents of the network list have changed or when any + // Manager properties (e.g. technology state) have changed. // (Called only from TrayNetworkStateObserver). - virtual void NetworkServiceChanged(const chromeos::NetworkState* network) = 0; + virtual void Update() = 0; protected: virtual ~NetworkDetailedView() {} diff --git a/ash/system/chromeos/network/network_state_list_detailed_view.cc b/ash/system/chromeos/network/network_state_list_detailed_view.cc index 3f334b5..63d801a 100644 --- a/ash/system/chromeos/network/network_state_list_detailed_view.cc +++ b/ash/system/chromeos/network/network_state_list_detailed_view.cc @@ -24,6 +24,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "chromeos/chromeos_switches.h" +#include "chromeos/device_event_log.h" #include "chromeos/network/device_state.h" #include "chromeos/network/network_configuration_handler.h" #include "chromeos/network/network_state.h" @@ -82,7 +83,6 @@ views::View* CreateInfoBubbleLine(const base::string16& text_label, } // namespace - //------------------------------------------------------------------------------ // A bubble which displays network info. @@ -102,9 +102,7 @@ class NetworkStateListDetailedView::InfoBubble AddChildView(content); } - virtual ~InfoBubble() { - detailed_view_->OnInfoBubbleDestroyed(); - } + ~InfoBubble() override { detailed_view_->OnInfoBubbleDestroyed(); } private: // Not owned. @@ -141,26 +139,13 @@ NetworkStateListDetailedView::~NetworkStateListDetailedView() { info_bubble_->GetWidget()->CloseNow(); } -void NetworkStateListDetailedView::ManagerChanged() { +void NetworkStateListDetailedView::Update() { UpdateNetworkList(); UpdateHeaderButtons(); UpdateNetworkExtra(); Layout(); } -void NetworkStateListDetailedView::NetworkListChanged() { - UpdateNetworkList(); - UpdateHeaderButtons(); - UpdateNetworkExtra(); - Layout(); -} - -void NetworkStateListDetailedView::NetworkServiceChanged( - const NetworkState* network) { - UpdateNetworkList(); - Layout(); -} - // Overridden from NetworkDetailedView: void NetworkStateListDetailedView::Init() { @@ -181,7 +166,7 @@ void NetworkStateListDetailedView::Init() { CreateHeaderButtons(); network_list_view_.set_content_view(scroll_content()); - NetworkListChanged(); + Update(); CallRequestScan(); } @@ -207,22 +192,19 @@ void NetworkStateListDetailedView::ButtonPressed(views::Button* sender, ash::SystemTrayDelegate* delegate = ash::Shell::GetInstance()->system_tray_delegate(); if (sender == button_wifi_) { - bool enabled = handler->IsTechnologyEnabled( - NetworkTypePattern::WiFi()); - handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), - !enabled, + bool enabled = handler->IsTechnologyEnabled(NetworkTypePattern::WiFi()); + handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), !enabled, chromeos::network_handler::ErrorCallback()); } else if (sender == turn_on_wifi_) { - handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), - true, + handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), true, chromeos::network_handler::ErrorCallback()); } else if (sender == button_mobile_) { ToggleMobile(); } else if (sender == settings_) { Shell::GetInstance()->metrics()->RecordUserMetricsAction( - list_type_ == LIST_TYPE_VPN ? - ash::UMA_STATUS_AREA_VPN_SETTINGS_CLICKED : - ash::UMA_STATUS_AREA_NETWORK_SETTINGS_CLICKED); + list_type_ == LIST_TYPE_VPN + ? ash::UMA_STATUS_AREA_VPN_SETTINGS_CLICKED + : ash::UMA_STATUS_AREA_NETWORK_SETTINGS_CLICKED); delegate->ShowNetworkSettings(""); } else if (sender == proxy_settings_) { delegate->ChangeProxySettings(); @@ -257,20 +239,21 @@ void NetworkStateListDetailedView::OnViewClicked(views::View* sender) { if (!network_list_view_.IsViewInList(sender, &service_path)) return; - const NetworkState* network = NetworkHandler::Get()->network_state_handler()-> - GetNetworkState(service_path); + const NetworkState* network = + NetworkHandler::Get()->network_state_handler()->GetNetworkState( + service_path); if (!network || network->IsConnectedState() || network->IsConnectingState()) { Shell::GetInstance()->metrics()->RecordUserMetricsAction( - list_type_ == LIST_TYPE_VPN ? - ash::UMA_STATUS_AREA_SHOW_NETWORK_CONNECTION_DETAILS : - ash::UMA_STATUS_AREA_SHOW_VPN_CONNECTION_DETAILS); + list_type_ == LIST_TYPE_VPN + ? ash::UMA_STATUS_AREA_SHOW_NETWORK_CONNECTION_DETAILS + : ash::UMA_STATUS_AREA_SHOW_VPN_CONNECTION_DETAILS); Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings( service_path); } else { Shell::GetInstance()->metrics()->RecordUserMetricsAction( - list_type_ == LIST_TYPE_VPN ? - ash::UMA_STATUS_AREA_CONNECT_TO_VPN : - ash::UMA_STATUS_AREA_CONNECT_TO_CONFIGURED_NETWORK); + list_type_ == LIST_TYPE_VPN + ? ash::UMA_STATUS_AREA_CONNECT_TO_VPN + : ash::UMA_STATUS_AREA_CONNECT_TO_CONFIGURED_NETWORK); ui::NetworkConnect::Get()->ConnectToNetwork(service_path); } } @@ -284,25 +267,21 @@ void NetworkStateListDetailedView::CreateHeaderEntry() { void NetworkStateListDetailedView::CreateHeaderButtons() { if (list_type_ != LIST_TYPE_VPN) { button_wifi_ = new TrayPopupHeaderButton( - this, - IDR_AURA_UBER_TRAY_WIFI_ENABLED, - IDR_AURA_UBER_TRAY_WIFI_DISABLED, + this, IDR_AURA_UBER_TRAY_WIFI_ENABLED, IDR_AURA_UBER_TRAY_WIFI_DISABLED, IDR_AURA_UBER_TRAY_WIFI_ENABLED_HOVER, - IDR_AURA_UBER_TRAY_WIFI_DISABLED_HOVER, - IDS_ASH_STATUS_TRAY_WIFI); + IDR_AURA_UBER_TRAY_WIFI_DISABLED_HOVER, IDS_ASH_STATUS_TRAY_WIFI); button_wifi_->SetTooltipText( l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_DISABLE_WIFI)); button_wifi_->SetToggledTooltipText( l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_ENABLE_WIFI)); footer()->AddButton(button_wifi_); - button_mobile_ = new TrayPopupHeaderButton( - this, - IDR_AURA_UBER_TRAY_CELLULAR_ENABLED, - IDR_AURA_UBER_TRAY_CELLULAR_DISABLED, - IDR_AURA_UBER_TRAY_CELLULAR_ENABLED_HOVER, - IDR_AURA_UBER_TRAY_CELLULAR_DISABLED_HOVER, - IDS_ASH_STATUS_TRAY_CELLULAR); + button_mobile_ = + new TrayPopupHeaderButton(this, IDR_AURA_UBER_TRAY_CELLULAR_ENABLED, + IDR_AURA_UBER_TRAY_CELLULAR_DISABLED, + IDR_AURA_UBER_TRAY_CELLULAR_ENABLED_HOVER, + IDR_AURA_UBER_TRAY_CELLULAR_DISABLED_HOVER, + IDS_ASH_STATUS_TRAY_CELLULAR); button_mobile_->SetTooltipText( l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_DISABLE_MOBILE)); button_mobile_->SetToggledTooltipText( @@ -311,12 +290,9 @@ void NetworkStateListDetailedView::CreateHeaderButtons() { } info_icon_ = new TrayPopupHeaderButton( - this, - IDR_AURA_UBER_TRAY_NETWORK_INFO, - IDR_AURA_UBER_TRAY_NETWORK_INFO, - IDR_AURA_UBER_TRAY_NETWORK_INFO_HOVER, + this, IDR_AURA_UBER_TRAY_NETWORK_INFO, IDR_AURA_UBER_TRAY_NETWORK_INFO, IDR_AURA_UBER_TRAY_NETWORK_INFO_HOVER, - IDS_ASH_STATUS_TRAY_NETWORK_INFO); + IDR_AURA_UBER_TRAY_NETWORK_INFO_HOVER, IDS_ASH_STATUS_TRAY_NETWORK_INFO); info_icon_->SetTooltipText( l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_NETWORK_INFO)); footer()->AddButton(info_icon_); @@ -330,10 +306,8 @@ void NetworkStateListDetailedView::CreateNetworkExtra() { views::View* bottom_row = new views::View(); views::BoxLayout* layout = new views::BoxLayout( - views::BoxLayout::kHorizontal, - kTrayMenuBottomRowPadding, - kTrayMenuBottomRowPadding, - kTrayMenuBottomRowPaddingBetweenItems); + views::BoxLayout::kHorizontal, kTrayMenuBottomRowPadding, + kTrayMenuBottomRowPadding, kTrayMenuBottomRowPaddingBetweenItems); layout->SetDefaultFlex(1); bottom_row->SetLayoutManager(layout); @@ -351,9 +325,8 @@ void NetworkStateListDetailedView::CreateNetworkExtra() { bottom_row->AddChildView(other_mobile_); } else { other_vpn_ = new TrayPopupLabelButton( - this, - ui::ResourceBundle::GetSharedInstance().GetLocalizedString( - IDS_ASH_STATUS_TRAY_OTHER_VPN)); + this, ui::ResourceBundle::GetSharedInstance().GetLocalizedString( + IDS_ASH_STATUS_TRAY_OTHER_VPN)); bottom_row->AddChildView(other_vpn_); } @@ -466,8 +439,8 @@ void NetworkStateListDetailedView::UpdateNetworkExtra() { } if (show_other_mobile) { other_mobile_->SetVisible(true); - other_mobile_->SetEnabled( - state == NetworkStateHandler::TECHNOLOGY_ENABLED); + other_mobile_->SetEnabled(state == + NetworkStateHandler::TECHNOLOGY_ENABLED); } else { other_mobile_->SetVisible(false); } @@ -481,8 +454,8 @@ void NetworkStateListDetailedView::UpdateNetworkExtra() { void NetworkStateListDetailedView::CreateSettingsEntry() { ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - bool show_settings = ash::Shell::GetInstance()-> - system_tray_delegate()->ShouldShowSettings(); + bool show_settings = + ash::Shell::GetInstance()->system_tray_delegate()->ShouldShowSettings(); if (login_ != user::LOGGED_IN_NONE) { // Allow user access settings only if user is logged in // and showing settings is allowed. There're situations (supervised user @@ -492,7 +465,7 @@ void NetworkStateListDetailedView::CreateSettingsEntry() { settings_ = new TrayPopupLabelButton( this, rb.GetLocalizedString(IDS_ASH_STATUS_TRAY_NETWORK_SETTINGS)); } - } else { + } else { // Allow users to change proxy settings only when not logged in. proxy_settings_ = new TrayPopupLabelButton( this, @@ -504,8 +477,7 @@ void NetworkStateListDetailedView::ToggleInfoBubble() { if (ResetInfoBubble()) return; - info_bubble_ = new InfoBubble( - info_icon_, CreateNetworkInfoView(), this); + info_bubble_ = new InfoBubble(info_icon_, CreateNetworkInfoView(), this); views::BubbleDelegateView::CreateBubble(info_bubble_)->Show(); } @@ -547,27 +519,28 @@ views::View* NetworkStateListDetailedView::CreateNetworkInfoView() { } if (!ip_address.empty()) { - container->AddChildView(CreateInfoBubbleLine(bundle.GetLocalizedString( - IDS_ASH_STATUS_TRAY_IP), ip_address)); + container->AddChildView(CreateInfoBubbleLine( + bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_IP), ip_address)); } if (!ethernet_address.empty()) { - container->AddChildView(CreateInfoBubbleLine(bundle.GetLocalizedString( - IDS_ASH_STATUS_TRAY_ETHERNET), ethernet_address)); + container->AddChildView(CreateInfoBubbleLine( + bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_ETHERNET), + ethernet_address)); } if (!wifi_address.empty()) { - container->AddChildView(CreateInfoBubbleLine(bundle.GetLocalizedString( - IDS_ASH_STATUS_TRAY_WIFI), wifi_address)); + container->AddChildView(CreateInfoBubbleLine( + bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_WIFI), wifi_address)); } if (!vpn_address.empty()) { - container->AddChildView(CreateInfoBubbleLine(bundle.GetLocalizedString( - IDS_ASH_STATUS_TRAY_VPN), vpn_address)); + container->AddChildView(CreateInfoBubbleLine( + bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_VPN), vpn_address)); } // Avoid an empty bubble in the unlikely event that there is no network // information at all. if (!container->has_children()) { - container->AddChildView(CreateInfoBubbleLabel(bundle.GetLocalizedString( - IDS_ASH_STATUS_TRAY_NO_NETWORKS))); + container->AddChildView(CreateInfoBubbleLabel( + bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_NO_NETWORKS))); } return container; @@ -585,8 +558,7 @@ void NetworkStateListDetailedView::CallRequestScan() { void NetworkStateListDetailedView::ToggleMobile() { NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler(); - bool enabled = - handler->IsTechnologyEnabled(NetworkTypePattern::Mobile()); + bool enabled = handler->IsTechnologyEnabled(NetworkTypePattern::Mobile()); ui::NetworkConnect::Get()->SetTechnologyEnabled(NetworkTypePattern::Mobile(), !enabled); } @@ -622,11 +594,9 @@ void NetworkStateListDetailedView::UpdateViewForNetwork( views::Label* NetworkStateListDetailedView::CreateInfoLabel() { views::Label* label = new views::Label(); - label->SetBorder( - views::Border::CreateEmptyBorder(ash::kTrayPopupPaddingBetweenItems, - ash::kTrayPopupPaddingHorizontal, - ash::kTrayPopupPaddingBetweenItems, - 0)); + label->SetBorder(views::Border::CreateEmptyBorder( + ash::kTrayPopupPaddingBetweenItems, ash::kTrayPopupPaddingHorizontal, + ash::kTrayPopupPaddingBetweenItems, 0)); label->SetHorizontalAlignment(gfx::ALIGN_LEFT); label->SetEnabledColor(SkColorSetARGB(192, 0, 0, 0)); return label; diff --git a/ash/system/chromeos/network/network_state_list_detailed_view.h b/ash/system/chromeos/network/network_state_list_detailed_view.h index 4400bec..b00f19af 100644 --- a/ash/system/chromeos/network/network_state_list_detailed_view.h +++ b/ash/system/chromeos/network/network_state_list_detailed_view.h @@ -38,31 +38,24 @@ class NetworkStateListDetailedView public ui::NetworkListDelegate, public base::SupportsWeakPtr<NetworkStateListDetailedView> { public: - enum ListType { - LIST_TYPE_NETWORK, - LIST_TYPE_VPN - }; + enum ListType { LIST_TYPE_NETWORK, LIST_TYPE_VPN }; NetworkStateListDetailedView(SystemTrayItem* owner, ListType list_type, user::LoginStatus login); - virtual ~NetworkStateListDetailedView(); + ~NetworkStateListDetailedView() override; // Overridden from NetworkDetailedView: - virtual void Init() override; - virtual DetailedViewType GetViewType() const override; - virtual void ManagerChanged() override; - virtual void NetworkListChanged() override; - virtual void NetworkServiceChanged( - const chromeos::NetworkState* network) override; + void Init() override; + DetailedViewType GetViewType() const override; + void Update() override; protected: // Overridden from ButtonListener. - virtual void ButtonPressed(views::Button* sender, - const ui::Event& event) override; + void ButtonPressed(views::Button* sender, const ui::Event& event) override; // Overridden from ViewClickListener. - virtual void OnViewClicked(views::View* sender) override; + void OnViewClicked(views::View* sender) override; private: class InfoBubble; @@ -100,14 +93,13 @@ class NetworkStateListDetailedView void ToggleMobile(); // ui::NetworkListDelegate: - virtual views::View* CreateViewForNetwork( - const ui::NetworkInfo& info) override; - virtual bool IsViewHovered(views::View* view) override; - virtual chromeos::NetworkTypePattern GetNetworkTypePattern() const override; - virtual void UpdateViewForNetwork(views::View* view, - const ui::NetworkInfo& info) override; - virtual views::Label* CreateInfoLabel() override; - virtual void RelayoutScrollList() override; + views::View* CreateViewForNetwork(const ui::NetworkInfo& info) override; + bool IsViewHovered(views::View* view) override; + chromeos::NetworkTypePattern GetNetworkTypePattern() const override; + void UpdateViewForNetwork(views::View* view, + const ui::NetworkInfo& info) override; + views::Label* CreateInfoLabel() override; + void RelayoutScrollList() override; // Type of list (all networks or vpn) ListType list_type_; diff --git a/ash/system/chromeos/network/tray_network.cc b/ash/system/chromeos/network/tray_network.cc index a5c5570..b00e220 100644 --- a/ash/system/chromeos/network/tray_network.cc +++ b/ash/system/chromeos/network/tray_network.cc @@ -47,8 +47,7 @@ class NetworkTrayView : public TrayItemView, public ui::network_icon::AnimationObserver { public: explicit NetworkTrayView(TrayNetwork* network_tray) - : TrayItemView(network_tray), - network_tray_(network_tray) { + : TrayItemView(network_tray), network_tray_(network_tray) { SetLayoutManager( new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0)); @@ -58,13 +57,11 @@ class NetworkTrayView : public TrayItemView, UpdateNetworkStateHandlerIcon(); } - virtual ~NetworkTrayView() { + ~NetworkTrayView() override { ui::network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this); } - virtual const char* GetClassName() const override { - return "NetworkTrayView"; - } + const char* GetClassName() const override { return "NetworkTrayView"; } void UpdateNetworkStateHandlerIcon() { NetworkStateHandler* handler = @@ -85,31 +82,29 @@ class NetworkTrayView : public TrayItemView, const NetworkState* connected_network = handler->ConnectedNetworkByType(NetworkTypePattern::NonVirtual()); if (connected_network) { - UpdateConnectionStatus( - base::UTF8ToUTF16(connected_network->name()), true); + UpdateConnectionStatus(base::UTF8ToUTF16(connected_network->name()), + true); } else { UpdateConnectionStatus(base::string16(), false); } } void UpdateAlignment(ShelfAlignment alignment) { - SetLayoutManager(new views::BoxLayout( - alignment == SHELF_ALIGNMENT_BOTTOM ? - views::BoxLayout::kHorizontal : views::BoxLayout::kVertical, - 0, 0, 0)); + SetLayoutManager(new views::BoxLayout(alignment == SHELF_ALIGNMENT_BOTTOM + ? views::BoxLayout::kHorizontal + : views::BoxLayout::kVertical, + 0, 0, 0)); Layout(); } // views::View override. - virtual void GetAccessibleState(ui::AXViewState* state) override { + void GetAccessibleState(ui::AXViewState* state) override { state->name = connection_status_string_; state->role = ui::AX_ROLE_BUTTON; } // ui::network_icon::AnimationObserver - virtual void NetworkIconChanged() override { - UpdateNetworkStateHandlerIcon(); - } + void NetworkIconChanged() override { UpdateNetworkStateHandlerIcon(); } private: // Updates connection status and notifies accessibility event when necessary. @@ -122,7 +117,7 @@ class NetworkTrayView : public TrayItemView, } if (new_connection_status_string != connection_status_string_) { connection_status_string_ = new_connection_status_string; - if(!connection_status_string_.empty()) + if (!connection_status_string_.empty()) NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, true); } } @@ -144,12 +139,11 @@ class NetworkDefaultView : public TrayItemMore, public ui::network_icon::AnimationObserver { public: NetworkDefaultView(TrayNetwork* network_tray, bool show_more) - : TrayItemMore(network_tray, show_more), - network_tray_(network_tray) { + : TrayItemMore(network_tray, show_more), network_tray_(network_tray) { Update(); } - virtual ~NetworkDefaultView() { + ~NetworkDefaultView() override { ui::network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this); } @@ -170,9 +164,7 @@ class NetworkDefaultView : public TrayItemMore, } // ui::network_icon::AnimationObserver - virtual void NetworkIconChanged() override { - Update(); - } + void NetworkIconChanged() override { Update(); } private: TrayNetwork* network_tray_; @@ -185,8 +177,7 @@ class NetworkWifiDetailedView : public NetworkDetailedView { explicit NetworkWifiDetailedView(SystemTrayItem* owner) : NetworkDetailedView(owner) { SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal, - kTrayPopupPaddingHorizontal, - 10, + kTrayPopupPaddingHorizontal, 10, kTrayPopupPaddingBetweenItems)); image_view_ = new views::ImageView; AddChildView(image_view_); @@ -199,58 +190,44 @@ class NetworkWifiDetailedView : public NetworkDetailedView { Update(); } - virtual ~NetworkWifiDetailedView() { - } + ~NetworkWifiDetailedView() override {} // Overridden from NetworkDetailedView: - virtual void Init() override { - } + void Init() override {} - virtual NetworkDetailedView::DetailedViewType GetViewType() const override { + NetworkDetailedView::DetailedViewType GetViewType() const override { return NetworkDetailedView::WIFI_VIEW; } - virtual void ManagerChanged() override { - Update(); - } - - virtual void NetworkListChanged() override { - Update(); - } - - virtual void NetworkServiceChanged( - const chromeos::NetworkState* network) override { - } - - private: - virtual void Layout() override { + void Layout() override { // Center both views vertically. views::View::Layout(); - image_view_->SetY( - (height() - image_view_->GetPreferredSize().height()) / 2); - label_view_->SetY( - (height() - label_view_->GetPreferredSize().height()) / 2); + image_view_->SetY((height() - image_view_->GetPreferredSize().height()) / + 2); + label_view_->SetY((height() - label_view_->GetPreferredSize().height()) / + 2); } - void Update() { + void Update() override { bool wifi_enabled = NetworkHandler::Get()->network_state_handler()->IsTechnologyEnabled( NetworkTypePattern::WiFi()); - const int image_id = wifi_enabled ? - IDR_AURA_UBER_TRAY_WIFI_ENABLED : IDR_AURA_UBER_TRAY_WIFI_DISABLED; + const int image_id = wifi_enabled ? IDR_AURA_UBER_TRAY_WIFI_ENABLED + : IDR_AURA_UBER_TRAY_WIFI_DISABLED; ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance(); image_view_->SetImage(bundle.GetImageNamed(image_id).ToImageSkia()); - const int string_id = wifi_enabled ? - IDS_ASH_STATUS_TRAY_NETWORK_WIFI_ENABLED : - IDS_ASH_STATUS_TRAY_NETWORK_WIFI_DISABLED; + const int string_id = wifi_enabled + ? IDS_ASH_STATUS_TRAY_NETWORK_WIFI_ENABLED + : IDS_ASH_STATUS_TRAY_NETWORK_WIFI_DISABLED; label_view_->SetText(bundle.GetLocalizedString(string_id)); - label_view_->SizeToFit(kTrayPopupMinWidth - - kTrayPopupPaddingHorizontal * 2 - kTrayPopupPaddingBetweenItems - - kTrayPopupDetailsIconWidth); + label_view_->SizeToFit( + kTrayPopupMinWidth - kTrayPopupPaddingHorizontal * 2 - + kTrayPopupPaddingBetweenItems - kTrayPopupDetailsIconWidth); } + private: views::ImageView* image_view_; views::Label* label_view_; @@ -290,15 +267,15 @@ views::View* TrayNetwork::CreateDefaultView(user::LoginStatus status) { if (!chromeos::NetworkHandler::IsInitialized()) return NULL; CHECK(tray_ != NULL); - default_ = new tray::NetworkDefaultView( - this, status != user::LOGGED_IN_LOCKED); + default_ = + new tray::NetworkDefaultView(this, status != user::LOGGED_IN_LOCKED); return default_; } views::View* TrayNetwork::CreateDetailedView(user::LoginStatus status) { CHECK(detailed_ == NULL); Shell::GetInstance()->metrics()->RecordUserMetricsAction( - ash::UMA_STATUS_AREA_DETAILED_NETWORK_VIEW); + ash::UMA_STATUS_AREA_DETAILED_NETWORK_VIEW); if (!chromeos::NetworkHandler::IsInitialized()) return NULL; if (request_wifi_view_) { @@ -344,35 +321,24 @@ void TrayNetwork::RequestToggleWifi() { NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler(); bool enabled = handler->IsTechnologyEnabled(NetworkTypePattern::WiFi()); Shell::GetInstance()->metrics()->RecordUserMetricsAction( - enabled ? - ash::UMA_STATUS_AREA_DISABLE_WIFI : - ash::UMA_STATUS_AREA_ENABLE_WIFI); - handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), - !enabled, + enabled ? ash::UMA_STATUS_AREA_DISABLE_WIFI + : ash::UMA_STATUS_AREA_ENABLE_WIFI); + handler->SetTechnologyEnabled(NetworkTypePattern::WiFi(), !enabled, chromeos::network_handler::ErrorCallback()); } void TrayNetwork::OnCaptivePortalDetected( const std::string& /* service_path */) { - NetworkStateChanged(false); + NetworkStateChanged(); } -void TrayNetwork::NetworkStateChanged(bool list_changed) { +void TrayNetwork::NetworkStateChanged() { if (tray_) tray_->UpdateNetworkStateHandlerIcon(); if (default_) default_->Update(); - if (detailed_) { - if (list_changed) - detailed_->NetworkListChanged(); - else - detailed_->ManagerChanged(); - } -} - -void TrayNetwork::NetworkServiceChanged(const chromeos::NetworkState* network) { if (detailed_) - detailed_->NetworkServiceChanged(network); + detailed_->Update(); } } // namespace ash diff --git a/ash/system/chromeos/network/tray_network.h b/ash/system/chromeos/network/tray_network.h index 407b64e..72c0a09 100644 --- a/ash/system/chromeos/network/tray_network.h +++ b/ash/system/chromeos/network/tray_network.h @@ -31,32 +31,28 @@ class TrayNetwork : public SystemTrayItem, public TrayNetworkStateObserver::Delegate { public: explicit TrayNetwork(SystemTray* system_tray); - virtual ~TrayNetwork(); + ~TrayNetwork() override; tray::NetworkDetailedView* detailed() { return detailed_; } // SystemTrayItem - virtual views::View* CreateTrayView(user::LoginStatus status) override; - virtual views::View* CreateDefaultView(user::LoginStatus status) override; - virtual views::View* CreateDetailedView(user::LoginStatus status) override; - virtual void DestroyTrayView() override; - virtual void DestroyDefaultView() override; - virtual void DestroyDetailedView() override; - virtual void UpdateAfterLoginStatusChange(user::LoginStatus status) override; - virtual void UpdateAfterShelfAlignmentChange( - ShelfAlignment alignment) override; + views::View* CreateTrayView(user::LoginStatus status) override; + views::View* CreateDefaultView(user::LoginStatus status) override; + views::View* CreateDetailedView(user::LoginStatus status) override; + void DestroyTrayView() override; + void DestroyDefaultView() override; + void DestroyDetailedView() override; + void UpdateAfterLoginStatusChange(user::LoginStatus status) override; + void UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) override; // NetworkObserver - virtual void RequestToggleWifi() override; + void RequestToggleWifi() override; // NetworkPortalDetectorObserver - virtual void OnCaptivePortalDetected( - const std::string& service_path) override; + void OnCaptivePortalDetected(const std::string& service_path) override; // TrayNetworkStateObserver::Delegate - virtual void NetworkStateChanged(bool list_changed) override; - virtual void NetworkServiceChanged( - const chromeos::NetworkState* network) override; + void NetworkStateChanged() override; private: tray::NetworkTrayView* tray_; diff --git a/ash/system/chromeos/network/tray_network_state_observer.cc b/ash/system/chromeos/network/tray_network_state_observer.cc index 4239be2..c23279e 100644 --- a/ash/system/chromeos/network/tray_network_state_observer.cc +++ b/ash/system/chromeos/network/tray_network_state_observer.cc @@ -7,45 +7,60 @@ #include <set> #include <string> +#include "ash/system/tray/system_tray.h" #include "base/location.h" +#include "chromeos/device_event_log.h" #include "chromeos/network/network_state.h" #include "chromeos/network/network_state_handler.h" #include "third_party/cros_system_api/dbus/service_constants.h" #include "ui/chromeos/network/network_icon.h" +#include "ui/compositor/scoped_animation_duration_scale_mode.h" using chromeos::NetworkHandler; +namespace { + +const int kUpdateFrequencyMs = 1000; + +} // namespace + namespace ash { TrayNetworkStateObserver::TrayNetworkStateObserver(Delegate* delegate) - : delegate_(delegate) { + : delegate_(delegate), + purge_icons_(false), + update_frequency_(kUpdateFrequencyMs) { + if (ui::ScopedAnimationDurationScaleMode::duration_scale_mode() != + ui::ScopedAnimationDurationScaleMode::NORMAL_DURATION) { + update_frequency_ = 0; // Send updates immediately for tests. + } if (NetworkHandler::IsInitialized()) { - NetworkHandler::Get()->network_state_handler()->AddObserver( - this, FROM_HERE); + NetworkHandler::Get()->network_state_handler()->AddObserver(this, + FROM_HERE); } } TrayNetworkStateObserver::~TrayNetworkStateObserver() { if (NetworkHandler::IsInitialized()) { - NetworkHandler::Get()->network_state_handler()->RemoveObserver( - this, FROM_HERE); + NetworkHandler::Get()->network_state_handler()->RemoveObserver(this, + FROM_HERE); } } void TrayNetworkStateObserver::NetworkListChanged() { - delegate_->NetworkStateChanged(true); - ui::network_icon::PurgeNetworkIconCache(); + purge_icons_ = true; + SignalUpdate(); } void TrayNetworkStateObserver::DeviceListChanged() { - delegate_->NetworkStateChanged(false); + SignalUpdate(); } // Any change to the Default (primary connected) network, including Strength // changes, should trigger a NetworkStateChanged update. void TrayNetworkStateObserver::DefaultNetworkChanged( const chromeos::NetworkState* network) { - delegate_->NetworkStateChanged(true); + SignalUpdate(); } // Any change to the Connection State should trigger a NetworkStateChanged @@ -53,7 +68,7 @@ void TrayNetworkStateObserver::DefaultNetworkChanged( // connected. void TrayNetworkStateObserver::NetworkConnectionStateChanged( const chromeos::NetworkState* network) { - delegate_->NetworkStateChanged(true); + SignalUpdate(); } // This tracks Strength and other property changes for all networks. It will @@ -61,13 +76,22 @@ void TrayNetworkStateObserver::NetworkConnectionStateChanged( // changes. void TrayNetworkStateObserver::NetworkPropertiesUpdated( const chromeos::NetworkState* network) { - if (network == - NetworkHandler::Get()->network_state_handler()->DefaultNetwork()) { - // Trigger NetworkStateChanged in case the Strength property of the - // Default network changed. - delegate_->NetworkStateChanged(true); + SignalUpdate(); +} + +void TrayNetworkStateObserver::SignalUpdate() { + if (timer_.IsRunning()) + return; + timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(update_frequency_), + this, &TrayNetworkStateObserver::SendNetworkStateChanged); +} + +void TrayNetworkStateObserver::SendNetworkStateChanged() { + delegate_->NetworkStateChanged(); + if (purge_icons_) { + ui::network_icon::PurgeNetworkIconCache(); + purge_icons_ = false; } - delegate_->NetworkServiceChanged(network); } } // namespace ash diff --git a/ash/system/chromeos/network/tray_network_state_observer.h b/ash/system/chromeos/network/tray_network_state_observer.h index f062a6b..af9bed6 100644 --- a/ash/system/chromeos/network/tray_network_state_observer.h +++ b/ash/system/chromeos/network/tray_network_state_observer.h @@ -8,6 +8,7 @@ #include <string> #include "base/compiler_specific.h" +#include "base/timer/timer.h" #include "chromeos/network/network_state_handler_observer.h" namespace ash { @@ -16,13 +17,9 @@ class TrayNetworkStateObserver : public chromeos::NetworkStateHandlerObserver { public: class Delegate { public: - // Called when the network state may have changed. If |list_changed| is - // true then the list of networks may have changed. - virtual void NetworkStateChanged(bool list_changed) = 0; - - // Called when the properties for |network| may have been updated. - virtual void NetworkServiceChanged( - const chromeos::NetworkState* network) = 0; + // Called when any interesting network changes occur. The frequency of this + // event is limited to kUpdateFrequencyMs. + virtual void NetworkStateChanged() = 0; protected: virtual ~Delegate() {} @@ -30,21 +27,34 @@ class TrayNetworkStateObserver : public chromeos::NetworkStateHandlerObserver { explicit TrayNetworkStateObserver(Delegate* delegate); - virtual ~TrayNetworkStateObserver(); + ~TrayNetworkStateObserver() override; // NetworkStateHandlerObserver overrides. - virtual void NetworkListChanged() override; - virtual void DeviceListChanged() override; - virtual void DefaultNetworkChanged( - const chromeos::NetworkState* network) override; - virtual void NetworkConnectionStateChanged( - const chromeos::NetworkState* network) override; - virtual void NetworkPropertiesUpdated( + void NetworkListChanged() override; + void DeviceListChanged() override; + void DefaultNetworkChanged(const chromeos::NetworkState* network) override; + void NetworkConnectionStateChanged( const chromeos::NetworkState* network) override; + void NetworkPropertiesUpdated(const chromeos::NetworkState* network) override; private: + void SignalUpdate(); + void SendNetworkStateChanged(); + + // Unowned Delegate pointer (must outlive this instance). Delegate* delegate_; + // Set to true when we should purge stale icons in the cache. + bool purge_icons_; + + // Frequency at which to push NetworkStateChanged updates. This avoids + // unnecessarily frequent UI updates (which can be expensive). We set this + // to 0 for tests to eliminate timing variance. + int update_frequency_; + + // Timer used to limit the frequency of NetworkStateChanged updates. + base::OneShotTimer<TrayNetworkStateObserver> timer_; + DISALLOW_COPY_AND_ASSIGN(TrayNetworkStateObserver); }; diff --git a/ash/system/chromeos/network/tray_vpn.cc b/ash/system/chromeos/network/tray_vpn.cc index 545a5ba..0d4cbea 100644 --- a/ash/system/chromeos/network/tray_vpn.cc +++ b/ash/system/chromeos/network/tray_vpn.cc @@ -13,6 +13,7 @@ #include "ash/system/tray/tray_constants.h" #include "ash/system/tray/tray_item_more.h" #include "ash/system/tray/tray_popup_label_button.h" +#include "chromeos/device_event_log.h" #include "chromeos/network/network_state.h" #include "chromeos/network/network_state_handler.h" #include "grit/ash_strings.h" @@ -38,7 +39,7 @@ class VpnDefaultView : public TrayItemMore, Update(); } - virtual ~VpnDefaultView() { + ~VpnDefaultView() override { ui::network_icon::NetworkIconAnimation::GetInstance()->RemoveObserver(this); } @@ -67,9 +68,7 @@ class VpnDefaultView : public TrayItemMore, } // ui::network_icon::AnimationObserver - virtual void NetworkIconChanged() override { - Update(); - } + void NetworkIconChanged() override { Update(); } private: void GetNetworkStateHandlerImageAndLabel(gfx::ImageSkia* image, @@ -83,8 +82,8 @@ class VpnDefaultView : public TrayItemMore, *image = ui::network_icon::GetImageForDisconnectedNetwork( ui::network_icon::ICON_TYPE_DEFAULT_VIEW, shill::kTypeVPN); if (label) { - *label = l10n_util::GetStringUTF16( - IDS_ASH_STATUS_TRAY_VPN_DISCONNECTED); + *label = + l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_VPN_DISCONNECTED); } *animating = false; return; @@ -104,9 +103,7 @@ class VpnDefaultView : public TrayItemMore, } // namespace tray TrayVPN::TrayVPN(SystemTray* system_tray) - : SystemTrayItem(system_tray), - default_(NULL), - detailed_(NULL) { + : SystemTrayItem(system_tray), default_(NULL), detailed_(NULL) { network_state_observer_.reset(new TrayNetworkStateObserver(this)); } @@ -166,20 +163,11 @@ void TrayVPN::UpdateAfterLoginStatusChange(user::LoginStatus status) { void TrayVPN::UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) { } -void TrayVPN::NetworkStateChanged(bool list_changed) { +void TrayVPN::NetworkStateChanged() { if (default_) default_->Update(); - if (detailed_) { - if (list_changed) - detailed_->NetworkListChanged(); - else - detailed_->ManagerChanged(); - } -} - -void TrayVPN::NetworkServiceChanged(const chromeos::NetworkState* network) { if (detailed_) - detailed_->NetworkServiceChanged(network); + detailed_->Update(); } } // namespace ash diff --git a/ash/system/chromeos/network/tray_vpn.h b/ash/system/chromeos/network/tray_vpn.h index 74d7aaa..9b56c82 100644 --- a/ash/system/chromeos/network/tray_vpn.h +++ b/ash/system/chromeos/network/tray_vpn.h @@ -22,23 +22,20 @@ class TrayVPN : public SystemTrayItem, public TrayNetworkStateObserver::Delegate { public: explicit TrayVPN(SystemTray* system_tray); - virtual ~TrayVPN(); + ~TrayVPN() override; // SystemTrayItem - virtual views::View* CreateTrayView(user::LoginStatus status) override; - virtual views::View* CreateDefaultView(user::LoginStatus status) override; - virtual views::View* CreateDetailedView(user::LoginStatus status) override; - virtual void DestroyTrayView() override; - virtual void DestroyDefaultView() override; - virtual void DestroyDetailedView() override; - virtual void UpdateAfterLoginStatusChange(user::LoginStatus status) override; - virtual void UpdateAfterShelfAlignmentChange( - ShelfAlignment alignment) override; + views::View* CreateTrayView(user::LoginStatus status) override; + views::View* CreateDefaultView(user::LoginStatus status) override; + views::View* CreateDetailedView(user::LoginStatus status) override; + void DestroyTrayView() override; + void DestroyDefaultView() override; + void DestroyDetailedView() override; + void UpdateAfterLoginStatusChange(user::LoginStatus status) override; + void UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) override; // TrayNetworkStateObserver::Delegate - virtual void NetworkStateChanged(bool list_changed) override; - virtual void NetworkServiceChanged( - const chromeos::NetworkState* network) override; + void NetworkStateChanged() override; private: tray::VpnDefaultView* default_; |