From 66147f68983ac39e559afc9b3776d3da8454c7d5 Mon Sep 17 00:00:00 2001 From: Ahmed Fakhry Date: Thu, 10 Sep 2015 14:12:37 -0700 Subject: [PATCH] Task Manager Should remember the most recently enabled columns. This CL: 1- Makes adding and removing columns to the task manager a lot easier and cleaner. 2- Stores/retrieves the columns visibility to/from the local state prefs. 3- Stores/retrieves the sorted column and the sort order. TBR=thestig@chromium.org BUG=452521 TEST=browser_tests --gtest_filter=NewTaskManagerViewTest.* Committed: https://crrev.com/4e16add803fbb0ed18413496d3fb4424fa4957c6 Cr-Commit-Position: refs/heads/master@{#346524} Review URL: https://codereview.chromium.org/1320563002 Cr-Commit-Position: refs/heads/master@{#346734} (cherry picked from commit 4780b87d1c25f68c39abc623199f3e71a477d32d) Review URL: https://codereview.chromium.org/1332083002 . Cr-Commit-Position: refs/branch-heads/2490@{#228} Cr-Branched-From: 7790a3535f2a81a03685eca31a32cf69ae0c114f-refs/heads/master@{#344925} --- .../providers/child_process_task_provider.cc | 8 +- chrome/browser/task_manager/task_manager.cc | 1 + .../browser/ui/views/new_task_manager_view.cc | 366 +++++++++++------- .../browser/ui/views/new_task_manager_view.h | 57 +++ .../new_task_manager_view_browsertest.cc | 144 +++++++ chrome/chrome_tests.gypi | 1 + chrome/common/pref_names.cc | 4 + chrome/common/pref_names.h | 1 + 8 files changed, 438 insertions(+), 144 deletions(-) create mode 100644 chrome/browser/ui/views/new_task_manager_view_browsertest.cc diff --git a/chrome/browser/task_management/providers/child_process_task_provider.cc b/chrome/browser/task_management/providers/child_process_task_provider.cc index a6f2073a13280..ca00a514b022f 100644 --- a/chrome/browser/task_management/providers/child_process_task_provider.cc +++ b/chrome/browser/task_management/providers/child_process_task_provider.cc @@ -123,12 +123,10 @@ void ChildProcessTaskProvider::ChildProcessDataCollected( void ChildProcessTaskProvider::CreateTask( const content::ChildProcessData& data) { - // The following case should never happen since we start observing - // |BrowserChildProcessObserver| only after we collect all pre-existing child - // processes and are notified (on the UI thread) that the collection is - // completed at |ChildProcessDataCollected()|. if (tasks_by_handle_.find(data.handle) != tasks_by_handle_.end()) { - NOTREACHED(); + // This case can happen when some of the child process data we collect upon + // StartUpdating() might be of BrowserChildProcessHosts whose channels + // hadn't connected yet. So we just return. return; } diff --git a/chrome/browser/task_manager/task_manager.cc b/chrome/browser/task_manager/task_manager.cc index 5107779b4876a..fbe855960be14 100644 --- a/chrome/browser/task_manager/task_manager.cc +++ b/chrome/browser/task_manager/task_manager.cc @@ -1500,6 +1500,7 @@ Resource* TaskManagerModel::GetResource(int index) const { // static void TaskManager::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterDictionaryPref(prefs::kTaskManagerWindowPlacement); + registry->RegisterDictionaryPref(prefs::kTaskManagerColumnVisibility); } bool TaskManager::IsBrowserProcess(int index) const { diff --git a/chrome/browser/ui/views/new_task_manager_view.cc b/chrome/browser/ui/views/new_task_manager_view.cc index 6e07c9c5b8843..8afec24e23c09 100644 --- a/chrome/browser/ui/views/new_task_manager_view.cc +++ b/chrome/browser/ui/views/new_task_manager_view.cc @@ -241,6 +241,106 @@ class TaskManagerValuesStringifier { //////////////////////////////////////////////////////////////////////////////// +// IMPORTANT: Do NOT change the below list without changing the COLUMN_LIST +// macro below. +const TableColumnData kColumns[] = { + { IDS_TASK_MANAGER_TASK_COLUMN, ui::TableColumn::LEFT, -1, 1, true, true, + true }, + { IDS_TASK_MANAGER_PROFILE_NAME_COLUMN, ui::TableColumn::LEFT, -1, 0, true, + true, false }, + { IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, true }, + { IDS_TASK_MANAGER_SHARED_MEM_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, + { IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, + { IDS_TASK_MANAGER_CPU_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, false, + true }, + { IDS_TASK_MANAGER_NET_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, false, + true }, + { IDS_TASK_MANAGER_PROCESS_ID_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + true, true }, + +#if defined(OS_WIN) + { IDS_TASK_MANAGER_GDI_HANDLES_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, + { IDS_TASK_MANAGER_USER_HANDLES_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, +#endif + + { IDS_TASK_MANAGER_WEBCORE_IMAGE_CACHE_COLUMN, ui::TableColumn::RIGHT, -1, 0, + true, false, false }, + { IDS_TASK_MANAGER_WEBCORE_SCRIPTS_CACHE_COLUMN, ui::TableColumn::RIGHT, -1, + 0, true, false, false }, + { IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN, ui::TableColumn::RIGHT, -1, 0, + true, false, false }, + { IDS_TASK_MANAGER_VIDEO_MEMORY_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, + { IDS_TASK_MANAGER_SQLITE_MEMORY_USED_COLUMN, ui::TableColumn::RIGHT, -1, 0, + true, false, false }, + +#if !defined(DISABLE_NACL) + { IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN, ui::TableColumn::RIGHT, -1, 0, + true, true, false }, +#endif // !defined(DISABLE_NACL) + + { IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN, ui::TableColumn::RIGHT, + -1, 0, true, false, false }, + +#if defined(OS_MACOSX) || defined(OS_LINUX) + // TODO(port): Port the idle wakeups per second to platforms other than Linux + // and MacOS (http://crbug.com/120488). + { IDS_TASK_MANAGER_IDLE_WAKEUPS_COLUMN, ui::TableColumn::RIGHT, -1, 0, true, + false, false }, +#endif // defined(OS_MACOSX) || defined(OS_LINUX) +}; + +const size_t kColumnsSize = arraysize(kColumns); + +const char kSortColumnIdKey[] = "sort_column_id"; +const char kSortIsAscendingKey[] = "sort_is_ascending"; + +// We can't use the integer IDs of the columns converted to strings as session +// restore keys. These integer values can change from one build to another as +// they are generated. Instead we use the literal string value of the column +// ID symbol (i.e. for the ID IDS_TASK_MANAGER_TASK_COLUMN, we use the literal +// string "IDS_TASK_MANAGER_TASK_COLUMN". The following macros help us +// efficiently get the literal ID for the integer value. +#define COLUMNS_LITS(def) \ + def(IDS_TASK_MANAGER_TASK_COLUMN) \ + def(IDS_TASK_MANAGER_PROFILE_NAME_COLUMN) \ + def(IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN) \ + def(IDS_TASK_MANAGER_SHARED_MEM_COLUMN) \ + def(IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN) \ + def(IDS_TASK_MANAGER_CPU_COLUMN) \ + def(IDS_TASK_MANAGER_NET_COLUMN) \ + def(IDS_TASK_MANAGER_PROCESS_ID_COLUMN) \ + def(IDS_TASK_MANAGER_GDI_HANDLES_COLUMN) \ + def(IDS_TASK_MANAGER_USER_HANDLES_COLUMN) \ + def(IDS_TASK_MANAGER_WEBCORE_IMAGE_CACHE_COLUMN) \ + def(IDS_TASK_MANAGER_WEBCORE_SCRIPTS_CACHE_COLUMN) \ + def(IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN) \ + def(IDS_TASK_MANAGER_VIDEO_MEMORY_COLUMN) \ + def(IDS_TASK_MANAGER_SQLITE_MEMORY_USED_COLUMN) \ + def(IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN) \ + def(IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN) \ + def(IDS_TASK_MANAGER_IDLE_WAKEUPS_COLUMN) +// Add to the above list in the macro any new IDs added in the future. Also +// remove the removed ones. + +#define COLUMN_ID_AS_STRING(col_id) case col_id: return std::string(#col_id); + +std::string GetColumnIdAsString(int column_id) { + switch (column_id) { + COLUMNS_LITS(COLUMN_ID_AS_STRING) + default: + NOTREACHED(); + return std::string(); + } +} + +//////////////////////////////////////////////////////////////////////////////// + // The table model of the task manager table view that will observe the // task manager backend and adapt its interface to match the requirements of the // TableView. @@ -278,10 +378,13 @@ class NewTaskManagerView::TableModel // Kills the process on which the task at |row_index| is running. void KillTask(int row_index); - // Called when a column visibility is toggled from the context menu of the - // table view. This will result in enabling or disabling some resources - // refresh types in the task manager. - void ToggleColumnVisibility(views::TableView* table, int column_id); + // Based on the given |visibility| and the |column_id|, a particular refresh + // type will be enabled or disabled. Multiple columns can map to the same + // refresh type, for that we need |table| to determine if any is visible. + void UpdateRefreshTypes(views::TableView* table, + int column_id, + bool visibility); + // Checks if the task at |row_index| is running on the browser process. bool IsBrowserProcess(int row_index) const; @@ -618,16 +721,17 @@ void NewTaskManagerView::TableModel::KillTask(int row_index) { process.Terminate(content::RESULT_CODE_KILLED, false); } -void NewTaskManagerView::TableModel::ToggleColumnVisibility( - views::TableView* table, - int column_id) { - DCHECK(table); - - bool new_visibility = !table->IsColumnVisible(column_id); - table->SetColumnVisibility(column_id, new_visibility); - +void NewTaskManagerView::TableModel::UpdateRefreshTypes(views::TableView* table, + int column_id, + bool visibility) { + bool new_visibility = visibility; RefreshType type = REFRESH_TYPE_NONE; switch (column_id) { + case IDS_TASK_MANAGER_PROFILE_NAME_COLUMN: + case IDS_TASK_MANAGER_TASK_COLUMN: + case IDS_TASK_MANAGER_PROCESS_ID_COLUMN: + return; // The data is these columns do not change. + case IDS_TASK_MANAGER_NET_COLUMN: type = REFRESH_TYPE_NETWORK_USAGE; break; @@ -636,12 +740,12 @@ void NewTaskManagerView::TableModel::ToggleColumnVisibility( type = REFRESH_TYPE_CPU; break; + case IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN: case IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN: case IDS_TASK_MANAGER_SHARED_MEM_COLUMN: - case IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN: type = REFRESH_TYPE_MEMORY; - if (table->IsColumnVisible(IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN) || - table->IsColumnVisible(IDS_TASK_MANAGER_SHARED_MEM_COLUMN) || + if (table->IsColumnVisible(IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN) || + table->IsColumnVisible(IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN) || table->IsColumnVisible(IDS_TASK_MANAGER_SHARED_MEM_COLUMN)) { new_visibility = true; } @@ -894,6 +998,7 @@ void NewTaskManagerView::WindowClosing() { // owned by the Views hierarchy. g_task_manager_view = nullptr; } + StoreColumnsSettings(); table_model_->StopUpdating(); } @@ -968,7 +1073,7 @@ bool NewTaskManagerView::GetAcceleratorForCommandId( } void NewTaskManagerView::ExecuteCommand(int id, int event_flags) { - table_model_->ToggleColumnVisibility(tab_table_, id); + ToggleColumnVisibility(id); } NewTaskManagerView::NewTaskManagerView(chrome::HostDesktopType desktop_type) @@ -976,147 +1081,43 @@ NewTaskManagerView::NewTaskManagerView(chrome::HostDesktopType desktop_type) new NewTaskManagerView::TableModel(REFRESH_TYPE_CPU | REFRESH_TYPE_MEMORY | REFRESH_TYPE_NETWORK_USAGE)), - menu_runner_(nullptr), - always_on_top_menu_text_(), kill_button_(nullptr), about_memory_link_(nullptr), tab_table_(nullptr), tab_table_parent_(nullptr), - columns_(), desktop_type_(desktop_type), is_always_on_top_(false) { Init(); } -void NewTaskManagerView::Init() { - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_TASK_COLUMN, - ui::TableColumn::LEFT, -1, 1)); - columns_.back().sortable = true; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_PROFILE_NAME_COLUMN, - ui::TableColumn::LEFT, -1, 0)); - columns_.back().sortable = true; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_SHARED_MEM_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_CPU_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_NET_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_PROCESS_ID_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - -#if defined(OS_WIN) - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_GDI_HANDLES_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_USER_HANDLES_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; -#endif - - columns_.push_back(ui::TableColumn( - IDS_TASK_MANAGER_WEBCORE_IMAGE_CACHE_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn( - IDS_TASK_MANAGER_WEBCORE_SCRIPTS_CACHE_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_VIDEO_MEMORY_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_SQLITE_MEMORY_USED_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; - -#if !defined(DISABLE_NACL) - columns_.push_back(ui::TableColumn( - IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; -#endif // !defined(DISABLE_NACL) - - columns_.push_back( - ui::TableColumn(IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; +// static +NewTaskManagerView* NewTaskManagerView::GetInstanceForTests() { + return g_task_manager_view; +} -#if defined(OS_MACOSX) || defined(OS_LINUX) - // TODO(port): Port the idle wakeups per second to platforms other than Linux - // and MacOS (http://crbug.com/120488). - columns_.push_back(ui::TableColumn(IDS_TASK_MANAGER_IDLE_WAKEUPS_COLUMN, - ui::TableColumn::RIGHT, -1, 0)); - columns_.back().sortable = true; - columns_.back().initial_sort_is_ascending = false; -#endif // defined(OS_MACOSX) || defined(OS_LINUX) +void NewTaskManagerView::Init() { + columns_settings_.reset(new base::DictionaryValue); + + // Create the table columns. + for (size_t i = 0; i < kColumnsSize; ++i) { + const auto& col_data = kColumns[i]; + columns_.push_back(ui::TableColumn(col_data.id, col_data.align, + col_data.width, col_data.percent)); + columns_.back().sortable = col_data.sortable; + columns_.back().initial_sort_is_ascending = + col_data.initial_sort_is_ascending; + } + // Create the table view. tab_table_ = new views::TableView( table_model_.get(), columns_, views::ICON_AND_TEXT, false); tab_table_->SetGrouper(table_model_.get()); - - // Hide some columns by default - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_PROFILE_NAME_COLUMN, false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_SHARED_MEM_COLUMN, false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN, false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_WEBCORE_IMAGE_CACHE_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_WEBCORE_SCRIPTS_CACHE_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_VIDEO_MEMORY_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_SQLITE_MEMORY_USED_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_NACL_DEBUG_STUB_PORT_COLUMN, - false); - tab_table_->SetColumnVisibility( - IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN, - false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_GDI_HANDLES_COLUMN, false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_USER_HANDLES_COLUMN, false); - tab_table_->SetColumnVisibility(IDS_TASK_MANAGER_IDLE_WAKEUPS_COLUMN, false); - tab_table_->SetObserver(this); tab_table_->set_context_menu_controller(this); set_context_menu_controller(this); + RetrieveSavedColumnsSettingsAndUpdateTable(); + kill_button_ = new views::LabelButton(this, l10n_util::GetStringUTF16(IDS_TASK_MANAGER_KILL)); kill_button_->SetStyle(views::Button::STYLE_BUTTON); @@ -1154,5 +1155,92 @@ void NewTaskManagerView::RetriveSavedAlwaysOnTopState() { dictionary->GetBoolean("always_on_top", &is_always_on_top_); } +void NewTaskManagerView::RetrieveSavedColumnsSettingsAndUpdateTable() { + if (!g_browser_process->local_state()) + return; + + const base::DictionaryValue* dictionary = + g_browser_process->local_state()->GetDictionary( + prefs::kTaskManagerColumnVisibility); + if (!dictionary) + return; + + // Do a best effort of retrieving the correct settings from the local state. + // Use the default settings of the value if it fails to be retrieved. + std::string sorted_col_id; + bool sort_is_ascending = true; + dictionary->GetString(kSortColumnIdKey, &sorted_col_id); + dictionary->GetBoolean(kSortIsAscendingKey, &sort_is_ascending); + + int current_visible_column_index = 0; + for (size_t i = 0; i < kColumnsSize; ++i) { + const int col_id = kColumns[i].id; + const std::string col_id_key(GetColumnIdAsString(col_id)); + + if (col_id_key.empty()) + continue; + + bool col_visibility = kColumns[i].default_visibility; + dictionary->GetBoolean(col_id_key, &col_visibility); + + // If the above GetBoolean() fails, the |col_visibility| remains at the + // default visibility. + columns_settings_->SetBoolean(col_id_key, col_visibility); + tab_table_->SetColumnVisibility(col_id, col_visibility); + table_model_->UpdateRefreshTypes(tab_table_, col_id, col_visibility); + + if (col_visibility) { + if (sorted_col_id == col_id_key) { + if (sort_is_ascending == kColumns[i].initial_sort_is_ascending) { + tab_table_->ToggleSortOrder(current_visible_column_index); + } else { + // Unfortunately the API of ui::TableView doesn't provide a clean way + // to sort by a particular column ID and a sort direction. If the + // retrieved sort direction is different than the initial one, we have + // to toggle the sort order twice! + // Note that the function takes the visible_column_index rather than + // a column ID. + tab_table_->ToggleSortOrder(current_visible_column_index); + tab_table_->ToggleSortOrder(current_visible_column_index); + } + } + + ++current_visible_column_index; + } + } +} + +void NewTaskManagerView::StoreColumnsSettings() { + PrefService* local_state = g_browser_process->local_state(); + if (!local_state) + return; + + DictionaryPrefUpdate dict_update(local_state, + prefs::kTaskManagerColumnVisibility); + + base::DictionaryValue::Iterator it(*columns_settings_); + while (!it.IsAtEnd()) { + dict_update->Set(it.key(), it.value().CreateDeepCopy()); + it.Advance(); + } + + // Store the current sort status to be restored again at startup. + if (tab_table_->sort_descriptors().empty()) { + dict_update->SetString(kSortColumnIdKey, ""); + } else { + const auto& sort_descriptor = tab_table_->sort_descriptors().front(); + dict_update->SetString(kSortColumnIdKey, + GetColumnIdAsString(sort_descriptor.column_id)); + dict_update->SetBoolean(kSortIsAscendingKey, sort_descriptor.ascending); + } +} + +void NewTaskManagerView::ToggleColumnVisibility(int column_id) { + bool new_visibility = !tab_table_->IsColumnVisible(column_id); + tab_table_->SetColumnVisibility(column_id, new_visibility); + columns_settings_->SetBoolean(GetColumnIdAsString(column_id), new_visibility); + table_model_->UpdateRefreshTypes(tab_table_, column_id, new_visibility); +} + } // namespace task_management diff --git a/chrome/browser/ui/views/new_task_manager_view.h b/chrome/browser/ui/views/new_task_manager_view.h index 2d4d038840303..2291d8af02e06 100644 --- a/chrome/browser/ui/views/new_task_manager_view.h +++ b/chrome/browser/ui/views/new_task_manager_view.h @@ -26,6 +26,44 @@ class View; namespace task_management { +// A collection of data to be used in the construction of a task manager table +// column. +struct TableColumnData { + // The generated ID of the column. These can change from one build to another. + // Their values are controlled by the generation from generated_resources.grd. + int id; + + // The alignment of the text displayed in this column. + ui::TableColumn::Alignment align; + + // |width| and |percent| used to define the size of the column. See + // ui::TableColumn::width and ui::TableColumn::percent for details. + int width; + float percent; + + // Is the column sortable. + bool sortable; + + // Is the initial sort order ascending? + bool initial_sort_is_ascending; + + // The default visibility of this column at startup of the table if no + // visibility is stored for it in the prefs. + bool default_visibility; +}; + +// The task manager table columns and their properties. +extern const TableColumnData kColumns[]; +extern const size_t kColumnsSize; + +// Session Restore Keys. +extern const char kSortColumnIdKey[]; +extern const char kSortIsAscendingKey[]; + +// Returns the |column_id| as a string value to be used as keys in the user +// preferences. +std::string GetColumnIdAsString(int column_id); + // The new task manager UI container. class NewTaskManagerView : public views::ButtonListener, @@ -85,10 +123,13 @@ class NewTaskManagerView void ExecuteCommand(int id, int event_flags) override; private: + friend class NewTaskManagerViewTest; class TableModel; explicit NewTaskManagerView(chrome::HostDesktopType desktop_type); + static NewTaskManagerView* GetInstanceForTests(); + // Creates the child controls. void Init(); @@ -101,10 +142,26 @@ class NewTaskManagerView // Restores saved "always on top" state from a previous session. void RetriveSavedAlwaysOnTopState(); + // Restores the saved columns settings from a previous session into + // |columns_settings_| and updates the table view. + void RetrieveSavedColumnsSettingsAndUpdateTable(); + + // Stores the current values in |column_settings_| to the user prefs so that + // it can be restored later next time the task manager view is opened. + void StoreColumnsSettings(); + + void ToggleColumnVisibility(int column_id); + scoped_ptr table_model_; scoped_ptr menu_runner_; + // Contains either the column settings retrieved from user preferences if it + // exists, or the default column settings. + // The columns settings are the visible columns and the last sorted column + // and the direction of the sort. + scoped_ptr columns_settings_; + // We need to own the text of the menu, the Windows API does not copy it. base::string16 always_on_top_menu_text_; diff --git a/chrome/browser/ui/views/new_task_manager_view_browsertest.cc b/chrome/browser/ui/views/new_task_manager_view_browsertest.cc new file mode 100644 index 0000000000000..2e27afb5e689e --- /dev/null +++ b/chrome/browser/ui/views/new_task_manager_view_browsertest.cc @@ -0,0 +1,144 @@ +// Copyright 2015 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. + +#include "base/prefs/pref_service.h" +#include "base/prefs/scoped_user_pref_update.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/ui/browser_dialogs.h" +#include "chrome/browser/ui/views/new_task_manager_view.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "content/public/test/test_utils.h" +#include "ui/views/controls/table/table_view.h" + +namespace task_management { + +class NewTaskManagerViewTest : public InProcessBrowserTest { + public: + NewTaskManagerViewTest() {} + ~NewTaskManagerViewTest() override {} + + void TearDownOnMainThread() override { + // Make sure the task manager is closed (if any). + chrome::HideTaskManager(); + content::RunAllPendingInMessageLoop(); + ASSERT_FALSE(GetView()); + + InProcessBrowserTest::TearDownOnMainThread(); + } + + NewTaskManagerView* GetView() const { + return NewTaskManagerView::GetInstanceForTests(); + } + + views::TableView* GetTable() const { + return GetView() ? GetView()->tab_table_ : nullptr; + } + + void ClearStoredColumnSettings() const { + PrefService* local_state = g_browser_process->local_state(); + if (!local_state) + FAIL(); + + DictionaryPrefUpdate dict_update(local_state, + prefs::kTaskManagerColumnVisibility); + dict_update->Clear(); + } + + void ToggleColumnVisibility(NewTaskManagerView* view, int col_id) { + DCHECK(view); + view->ToggleColumnVisibility(col_id); + } + + private: + DISALLOW_COPY_AND_ASSIGN(NewTaskManagerViewTest); +}; + +// Tests that all defined columns have a corresponding string IDs for keying +// into the user preferences dictionary. +IN_PROC_BROWSER_TEST_F(NewTaskManagerViewTest, AllColumnsHaveStringIds) { + for (size_t i = 0; i < kColumnsSize; ++i) + EXPECT_NE("", GetColumnIdAsString(kColumns[i].id)); +} + +// In the case of no settings stored in the user preferences local store, test +// that the task manager table starts with the default columns visibility as +// stored in |kColumns|. +IN_PROC_BROWSER_TEST_F(NewTaskManagerViewTest, TableStartsWithDefaultColumns) { + ASSERT_NO_FATAL_FAILURE(ClearStoredColumnSettings()); + + chrome::ShowTaskManager(browser()); + views::TableView* table = GetTable(); + ASSERT_TRUE(table); + + EXPECT_FALSE(table->is_sorted()); + for (size_t i = 0; i < kColumnsSize; ++i) { + EXPECT_EQ(kColumns[i].default_visibility, + table->IsColumnVisible(kColumns[i].id)); + } +} + +// Tests that changing columns visibility and sort order will be stored upon +// closing the task manager view and restored when re-opened. +IN_PROC_BROWSER_TEST_F(NewTaskManagerViewTest, ColumnsSettingsAreRestored) { + ASSERT_NO_FATAL_FAILURE(ClearStoredColumnSettings()); + + chrome::ShowTaskManager(browser()); + NewTaskManagerView* view = GetView(); + ASSERT_TRUE(view); + views::TableView* table = GetTable(); + ASSERT_TRUE(table); + + // Toggle the visibility of all columns. + EXPECT_FALSE(table->is_sorted()); + for (size_t i = 0; i < kColumnsSize; ++i) { + EXPECT_EQ(kColumns[i].default_visibility, + table->IsColumnVisible(kColumns[i].id)); + ToggleColumnVisibility(view, kColumns[i].id); + } + + // Sort by the first visible and initially ascending sortable column. + bool is_sorted = false; + int sorted_col_id = -1; + for (size_t i = 0; i < table->visible_columns().size(); ++i) { + const ui::TableColumn& column = table->visible_columns()[i].column; + if (column.sortable && column.initial_sort_is_ascending) { + // Toggle the sort twice for a descending sort. + table->ToggleSortOrder(static_cast(i)); + table->ToggleSortOrder(static_cast(i)); + is_sorted = true; + return; + } + } + + if (is_sorted) { + EXPECT_TRUE(table->is_sorted()); + EXPECT_FALSE(table->sort_descriptors().front().ascending); + EXPECT_EQ(table->sort_descriptors().front().column_id, sorted_col_id); + } + + // Close the task manager view and re-open. Expect the inverse of the default + // visibility, and the last sort order. + chrome::HideTaskManager(); + content::RunAllPendingInMessageLoop(); + ASSERT_FALSE(GetView()); + chrome::ShowTaskManager(browser()); + view = GetView(); + ASSERT_TRUE(view); + table = GetTable(); + ASSERT_TRUE(table); + + if (is_sorted) { + EXPECT_TRUE(table->is_sorted()); + EXPECT_FALSE(table->sort_descriptors().front().ascending); + EXPECT_EQ(table->sort_descriptors().front().column_id, sorted_col_id); + } + for (size_t i = 0; i < kColumnsSize; ++i) { + EXPECT_EQ(!kColumns[i].default_visibility, + table->IsColumnVisible(kColumns[i].id)); + } +} + +} // namespace task_management + diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 103da97aa594f..347f712eba634 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -643,6 +643,7 @@ 'browser/ui/views/frame/browser_view_browsertest.cc', 'browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc', 'browser/ui/views/media_router/media_router_ui_browsertest.cc', + 'browser/ui/views/new_task_manager_view_browsertest.cc', 'browser/ui/views/profiles/avatar_menu_button_browsertest.cc', 'browser/ui/views/profiles/profile_chooser_view_browsertest.cc', 'browser/ui/views/toolbar/browser_actions_container_browsertest.cc', diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 9d21f958f0fa6..08903efaf02c3 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -1357,6 +1357,10 @@ const char kBrowserWindowPlacementPopup[] = "browser.window_placement_popup"; // manager window to restore on startup. const char kTaskManagerWindowPlacement[] = "task_manager.window_placement"; +// The most recent stored column visibility of the task manager table to be +// restored on startup. +const char kTaskManagerColumnVisibility[] = "task_manager.column_visibility"; + // A collection of position, size, and other data relating to app windows to // restore on startup. const char kAppWindowPlacement[] = "browser.app_window_placement"; diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 8e03ec118810c..a6045082f7312 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -478,6 +478,7 @@ extern const char kBrowserSuppressDefaultBrowserPrompt[]; extern const char kBrowserWindowPlacement[]; extern const char kBrowserWindowPlacementPopup[]; extern const char kTaskManagerWindowPlacement[]; +extern const char kTaskManagerColumnVisibility[]; extern const char kAppWindowPlacement[]; extern const char kDownloadDefaultDirectory[];