Task Manager Should remember the most recently enabled columns.

chrome/browser/ui/views/new_task_manager_view.cc

Index: chrome/browser/ui/views/new_task_manager_view.cc
diff --git a/chrome/browser/ui/views/new_task_manager_view.cc b/chrome/browser/ui/views/new_task_manager_view.cc
index 6e07c9c5b8843fcec0ebb040de89472ff4e60622..67fa32a8313cbdd112f7c95698bcc51105438ea5 100644
--- a/chrome/browser/ui/views/new_task_manager_view.cc
+++ b/chrome/browser/ui/views/new_task_manager_view.cc
@@ -237,6 +237,139 @@ class TaskManagerValuesStringifier {
   DISALLOW_COPY_AND_ASSIGN(TaskManagerValuesStringifier);
 };
 
+// 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 version number we manually assign for the list of columns. Any addition/
+// removal to/from the columns list must be accompanied by incrementing the
+// version number. This is needed so that we can restore the visible columns
+// correctly.
+const int kColumnsVersion = 1;
+
+// The task manager table columns and their properties.
+// IMPORTANT: Do NOT change the below list without changing the above version
+// number.

[Lei Zhang, 2015/08/28 22:47:44]

Remind readers about COLUMNS_LITS too?

[afakhry, 2015/08/29 00:30:29]

Done.

+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);
+
+// 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();
+  }
+}
+
+// Session Restore Keys.
+const char kColumnVersionKey[] = "columns_version";
+const char kSortColumnIdKey[] = "sort_column_id";
+const char kSortIsAscendingKey[] = "sort_is_ascending";
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -280,8 +413,20 @@ class NewTaskManagerView::TableModel
 
   // 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);
+  // refresh types in the task manager. It will also update the
+  // |visibility_settings| with the new visibility so that it's stored in the
+  // prefs when the task manager window is closed.
+  void ToggleColumnVisibility(views::TableView* table,
+                              base::DictionaryValue* visibility_settings,
+                              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;
@@ -620,14 +765,30 @@ void NewTaskManagerView::TableModel::KillTask(int row_index) {
 
 void NewTaskManagerView::TableModel::ToggleColumnVisibility(
     views::TableView* table,
+    base::DictionaryValue* visibility_settings,
     int column_id) {
   DCHECK(table);
+  DCHECK(visibility_settings);
 
   bool new_visibility = !table->IsColumnVisible(column_id);
   table->SetColumnVisibility(column_id, new_visibility);
+  visibility_settings->SetBoolean(GetColumnIdAsString(column_id),
+                                  new_visibility);
+
+  UpdateRefreshTypes(table, 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 +797,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 +1055,7 @@ void NewTaskManagerView::WindowClosing() {
     // owned by the Views hierarchy.
     g_task_manager_view = nullptr;
   }
+  StoreColumnsSettings();
   table_model_->StopUpdating();
 }
 
@@ -968,7 +1130,9 @@ bool NewTaskManagerView::GetAcceleratorForCommandId(
 }
 
 void NewTaskManagerView::ExecuteCommand(int id, int event_flags) {
-  table_model_->ToggleColumnVisibility(tab_table_, id);
+  table_model_->ToggleColumnVisibility(tab_table_,
+                                       columns_settings_.get(),
+                                       id);
 }
 
 NewTaskManagerView::NewTaskManagerView(chrome::HostDesktopType desktop_type)
@@ -976,147 +1140,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;
-
-#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)
+  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;
+
+    columns_settings_->SetBoolean(GetColumnIdAsString(col_data.id),
+                                            col_data.default_visibility);

[Lei Zhang, 2015/08/28 22:47:45]

nit: indentation

[afakhry, 2015/08/29 00:30:29]

Done.

+  }
 
+  // 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);
 
+  // Retrieve and update the table columns visibilities.
+  RetrieveSavedColumnsSettings();
+  UpdateTableFromColumnsSettings();
+
   kill_button_ = new views::LabelButton(this,
       l10n_util::GetStringUTF16(IDS_TASK_MANAGER_KILL));
   kill_button_->SetStyle(views::Button::STYLE_BUTTON);
@@ -1154,5 +1214,95 @@ void NewTaskManagerView::RetriveSavedAlwaysOnTopState() {
     dictionary->GetBoolean("always_on_top", &is_always_on_top_);
 }
 
+void NewTaskManagerView::RetrieveSavedColumnsSettings() {
+  if (!g_browser_process->local_state())
+    return;
+
+  const base::DictionaryValue* dictionary =
+      g_browser_process->local_state()->GetDictionary(
+          prefs::kTaskManagerColumnVisibility);
+  if (!dictionary)
+    return;
+
+  int columns_version;
+  if (!dictionary->GetInteger(kColumnVersionKey, &columns_version))
+    return;
+
+  // If the number of columns changes from one version to another (i.e. the

[Lei Zhang, 2015/08/28 22:47:44]

So the other possible way of doing this is to be very tolerant of bad/outdated
data in UpdateTableFromColumnsSettings() and just try to make a best effort
attempt at restoring the settings from the pref, ignoring any pref that do not
make sense. Then maybe we won't need a version number. Do you think that may
work better?

[afakhry, 2015/08/29 00:30:29]

I agree. I actually combined the Retrieve() and UpdateTable() functions into
one. So now we loop through the values we're sure they're correct. try to
retrieve their last stored values from the local state, if they succeed, fine,
if not, we're still using the default values. And then update the table view
while we're there. Now |columns_settings_| is always guaranteed to contain
correct values rather than possibly garbage we got from the below line that I
removed |columns_settings_.reset(dictionary->DeepCopy());|.

+  // columns version number changes), we skip and use the default visibility of
+  // the columns as defined in |kColumns|.
+  if (columns_version != kColumnsVersion)
+    return;
+
+  columns_settings_.reset(dictionary->DeepCopy());
+}
+
+void NewTaskManagerView::UpdateTableFromColumnsSettings() {
+  std::string sorted_col_id;
+  bool sort_is_ascending = true;
+  columns_settings_->GetString(kSortColumnIdKey, &sorted_col_id);
+  columns_settings_->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;
+    bool visible = false;
+    bool success = columns_settings_->GetBoolean(
+        GetColumnIdAsString(col_id), &visible);
+
+    DCHECK(success);

[Lei Zhang, 2015/08/28 22:47:44]

Given the existence of malware and other external programs that try to mess with
Chrome's setting files, (boo!!!) I would not count on this DCHECK() to hold in
real life. Can we just use the default visibility/sorting on failure?

[afakhry, 2015/08/29 00:30:29]

Done.

+
+    tab_table_->SetColumnVisibility(col_id, visible);
+    table_model_->UpdateRefreshTypes(tab_table_, col_id, visible);
+
+    if (visible) {
+      if (sorted_col_id == GetColumnIdAsString(col_id)) {
+        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.

[afakhry, 2015/08/27 19:56:52]

Obviously not a big fan of the TableView API. I personally like SWT:
http://help.eclipse.org/juno/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%...

+          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);
+  }
+
+  dict_update->SetInteger(kColumnVersionKey, kColumnsVersion);
+}
+
 }  // namespace task_management