Mac: Add sort support for task manager.

Also fix a leak in test Init reported by valgrind. The problem was that the test called a rogue init, which didn't cause the window to be shown, and hence -close didn't send a -windowWillClose: notification. Also restore -windowWillClose:, it accidentally got deleted in http://codereview.chromium.org/536086

BUG=32148,30398
TEST=Click a column in the task manager. Should sort.

Review URL: http://codereview.chromium.org/2873075

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54175 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/cocoa/task_manager_mac.h

@@ -7,6 +7,8 @@
 #pragma once
 
 #import <Cocoa/Cocoa.h>
+#include <vector>
+
 #include "base/scoped_nsobject.h"
 #include "chrome/browser/cocoa/table_row_nsimage_cache.h"
 #include "chrome/browser/task_manager.h"
@@ -25,6 +27,13 @@ class TaskManagerMac;
   TaskManagerModel* model_;  // weak
 
   scoped_nsobject<WindowSizeAutosaver> size_saver_;
+
+  // Contains a permutation of [0..|model_->ResourceCount() - 1|]. Used to
+  // implement sorting.
+  std::vector<int> indexShuffle_;
+
+  // Descriptor of the current sort column.
+  scoped_nsobject<NSSortDescriptor> currentSortDescriptor_;
 }
 
 // Creates and shows the task manager's window.
@@ -43,11 +52,15 @@ class TaskManagerMac;
 - (void)selectDoubleClickedTab:(id)sender;
 @end
 
+@interface TaskManagerWindowController (TestingAPI)
+- (NSTableView*)tableView;
+@end
+
 // This class listens to task changed events sent by chrome.
 class TaskManagerMac : public TaskManagerModelObserver,
                        public TableRowNSImageCache::Table {
  public:
-  TaskManagerMac();
+  TaskManagerMac(TaskManager* task_manager);
   virtual ~TaskManagerMac();
 
   // TaskManagerModelObserver
@@ -74,9 +87,11 @@ class TaskManagerMac : public TaskManagerModelObserver,
   // Lazily converts the image at the given row and caches it in |icon_cache_|.
   NSImage* GetImageForRow(int row);
 
+  // Returns the cocoa object. Used for testing.
+  TaskManagerWindowController* cocoa_controller() { return window_controller_; }
  private:
   // The task manager.
-   TaskManager* const task_manager_;  // weak
+  TaskManager* const task_manager_;  // weak
 
   // Our model.
   TaskManagerModel* const model_;  // weak

chrome/browser/cocoa/task_manager_mac.mm

@@ -15,17 +15,15 @@
 #include "chrome/common/pref_names.h"
 #include "grit/generated_resources.h"
 
-// TODO(thakis): Column sort comparator
-// TODO(thakis): Clicking column header doesn't sort
-// TODO(thakis): Default sort column
+namespace {
 
 // Width of "a" and most other letters/digits in "small" table views.
-static const int kCharWidth = 6;
+const int kCharWidth = 6;
 
 // Some of the strings below have spaces at the end or are missing letters, to
 // make the columns look nicer, and to take potentially longer localized strings
 // into account.
-static const struct ColumnWidth {
+const struct ColumnWidth {
   int columnId;
   int minWidth;
   int maxWidth;  // If this is -1, 1.5*minColumWidth is used as max width.
@@ -54,6 +52,28 @@ static const struct ColumnWidth {
       arraysize("15 ") * kCharWidth, -1 },
 };
 
+class SortHelper {
+ public:
+  SortHelper(TaskManagerModel* model, NSSortDescriptor* column)
+      : sort_column_([[column key] intValue]),
+        ascending_([column ascending]),
+        model_(model) {}
+
+  bool operator()(int a, int b) {
+    int cmp_result = model_->CompareValues(a, b, sort_column_ );
+    if (!ascending_)
+      cmp_result = -cmp_result;
+    // TODO(thakis): Do grouping like on GTK.
+    return cmp_result < 0;
+  }
+ private:
+  int sort_column_;
+  bool ascending_;
+  TaskManagerModel* model_;  // weak;
+};
+
+}  // namespace
+
 @interface TaskManagerWindowController (Private)
 - (NSTableColumn*)addColumnWithId:(int)columnId visible:(BOOL)isVisible;
 - (void)setUpTableColumns;
@@ -83,12 +103,22 @@ static const struct ColumnWidth {
                     path:prefs::kTaskManagerWindowPlacement
                    state:kSaveWindowRect]);
     }
-    [[self window] makeKeyAndOrderFront:self];
+    [self showWindow:self];
   }
   return self;
 }
 
+- (void)sortShuffleArray {
+  indexShuffle_.resize(model_->ResourceCount());
+  for (size_t i = 0; i < indexShuffle_.size(); ++i)
+    indexShuffle_[i] = i;
+
+  std::sort(indexShuffle_.begin(), indexShuffle_.end(),
+            SortHelper(model_, currentSortDescriptor_.get()));
+}
+
 - (void)reloadData {
+  [self sortShuffleArray];
   [tableView_ reloadData];
   [self adjustSelectionAndEndProcessButton];
 }
@@ -113,6 +143,10 @@ static const struct ColumnWidth {
   taskManager_->ActivateProcess(row);
 }
 
+- (NSTableView*)tableView {
+  return tableView_;
+}
+
 - (void)awakeFromNib {
   [self setUpTableColumns];
   [self setUpTableHeaderContextMenu];
@@ -141,6 +175,14 @@ static const struct ColumnWidth {
   [column.get() setHidden:!isVisible];
   [column.get() setEditable:NO];
 
+  // The page column should by default be sorted ascending.
+  BOOL ascending = columnId == IDS_TASK_MANAGER_PAGE_COLUMN;
+
+  scoped_nsobject<NSSortDescriptor> sortDescriptor([[NSSortDescriptor alloc]
+      initWithKey:[NSString stringWithFormat:@"%d", columnId]
+        ascending:ascending]);
+  [column.get() setSortDescriptorPrototype:sortDescriptor.get()];
+
   // Default values, only used in release builds if nobody notices the DCHECK
   // during development when adding new columns.
   int minWidth = 200, maxWidth = 400;
@@ -181,6 +223,10 @@ static const struct ColumnWidth {
   [nameCell.get() setFont:[[nameColumn dataCell] font]];
   [nameColumn setDataCell:nameCell.get()];
 
+  // Initially, sort on the tab name.
+  [tableView_ setSortDescriptors:
+      [NSArray arrayWithObject:[nameColumn sortDescriptorPrototype]]];
+
   [self addColumnWithId:IDS_TASK_MANAGER_PHYSICAL_MEM_COLUMN visible:YES];
   [self addColumnWithId:IDS_TASK_MANAGER_SHARED_MEM_COLUMN visible:NO];
   [self addColumnWithId:IDS_TASK_MANAGER_PRIVATE_MEM_COLUMN visible:NO];
@@ -260,6 +306,14 @@ static const struct ColumnWidth {
   [self adjustSelectionAndEndProcessButton];
 }
 
+- (void)windowWillClose:(NSNotification*)notification {
+  if (taskManagerObserver_) {
+    taskManagerObserver_->WindowWasClosed();
+    taskManagerObserver_ = nil;
+  }
+  [self autorelease];
+}
+
 @end
 
 @implementation TaskManagerWindowController (NSTableDataSource)
@@ -270,6 +324,8 @@ static const struct ColumnWidth {
 }
 
 - (NSString*)modelTextForRow:(int)row column:(int)columnId {
+  DCHECK_LT(static_cast<size_t>(row), indexShuffle_.size());
+  row = indexShuffle_[row];
   switch (columnId) {
     case IDS_TASK_MANAGER_PAGE_COLUMN:  // Process
       return base::SysWideToNSString(model_->GetResourceTitle(row));
@@ -362,14 +418,24 @@ static const struct ColumnWidth {
   return cell;
 }
 
+- (void)           tableView:(NSTableView*)tableView
+    sortDescriptorsDidChange:(NSArray*)oldDescriptors {
+  NSArray* newDescriptors = [tableView sortDescriptors];
+  if ([newDescriptors count] < 1)
+    return;
+
+  currentSortDescriptor_.reset([[newDescriptors objectAtIndex:0] retain]);
+  [self reloadData];  // Sorts.
+}
+
 @end
 
 ////////////////////////////////////////////////////////////////////////////////
 // TaskManagerMac implementation:
 
-TaskManagerMac::TaskManagerMac()
-  : task_manager_(TaskManager::GetInstance()),
-    model_(TaskManager::GetInstance()->model()),
+TaskManagerMac::TaskManagerMac(TaskManager* task_manager)
+  : task_manager_(task_manager),
+    model_(task_manager->model()),
     icon_cache_(this) {
   window_controller_ =
       [[TaskManagerWindowController alloc] initWithTaskManagerObserver:this];
@@ -380,7 +446,11 @@ TaskManagerMac::TaskManagerMac()
 TaskManagerMac* TaskManagerMac::instance_ = NULL;
 
 TaskManagerMac::~TaskManagerMac() {
-  task_manager_->OnWindowClosed();
+  if (this == instance_) {
+    // Do not do this when running in unit tests: |StartUpdating()| never got
+    // called in that case.
+    task_manager_->OnWindowClosed();
+  }
   model_->RemoveObserver(this);
 }
 
@@ -426,7 +496,7 @@ void TaskManagerMac::Show() {
     [[instance_->window_controller_ window]
         makeKeyAndOrderFront:instance_->window_controller_];
   } else {
-    instance_ = new TaskManagerMac;
+    instance_ = new TaskManagerMac(TaskManager::GetInstance());
     instance_->model_->StartUpdating();
   }
 }

chrome/browser/cocoa/task_manager_mac_unittest.mm

@@ -5,33 +5,70 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/scoped_nsobject.h"
+#include "base/utf_string_conversions.h"
 #import "chrome/browser/cocoa/task_manager_mac.h"
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 namespace {
 
-class TaskManagerWindowControllerTest : public CocoaTest {
+class TestResource : public TaskManager::Resource {
  public:
-  virtual void SetUp() {
-    CocoaTest::SetUp();
-    controller_ = [[TaskManagerWindowController alloc] init];
+  TestResource(const string16& title) : title_(title) {}
+  virtual std::wstring GetTitle() const { return UTF16ToWide(title_); }
+  virtual SkBitmap GetIcon() const { return SkBitmap(); }
+  virtual base::ProcessHandle GetProcess() const {
+    return base::GetCurrentProcessHandle();
   }
+  virtual bool SupportNetworkUsage() const { return false; }
+  virtual void SetSupportNetworkUsage() { NOTREACHED(); }
+  virtual void Refresh() {}
+  string16 title_;
+};
 
-  virtual void TearDown() {
-    [controller_ close];
-    CocoaTest::TearDown();
-  }
+}  // namespace
 
-  TaskManagerWindowController *controller_;
+class TaskManagerWindowControllerTest : public CocoaTest {
 };
 
-// Test creation, to ensure nothing leaks or crashes
+// Test creation, to ensure nothing leaks or crashes.
 TEST_F(TaskManagerWindowControllerTest, Init) {
+  TaskManager task_manager;
+  TaskManagerMac* bridge(new TaskManagerMac(&task_manager));
+  TaskManagerWindowController* controller = bridge->cocoa_controller();
+
+  // Releases the controller, which in turn deletes |bridge|.
+  [controller close];
 }
 
-// TODO(thakis): Add tests for more methods as they become implemented
-// (TaskManager::Show() etc).
+TEST_F(TaskManagerWindowControllerTest, Sort) {
+  TaskManager task_manager;
 
-}  // namespace
+  TestResource resource1(UTF8ToUTF16("zzz"));
+  TestResource resource2(UTF8ToUTF16("zza"));
+
+  task_manager.AddResource(&resource1);
+  task_manager.AddResource(&resource2);  // Will be in the same group.
+
+  TaskManagerMac* bridge(new TaskManagerMac(&task_manager));
+  TaskManagerWindowController* controller = bridge->cocoa_controller();
+  NSTableView* table = [controller tableView];
+  ASSERT_EQ(2, [controller numberOfRowsInTableView:table]);
+
+  // Test that table is sorted on title.
+  NSTableColumn* title_column = [table tableColumnWithIdentifier:
+      [NSNumber numberWithInt:IDS_TASK_MANAGER_PAGE_COLUMN]];
+  NSCell* cell;
+  cell = [controller tableView:table dataCellForTableColumn:title_column row:0];
+  EXPECT_TRUE([@"zza" isEqualToString:[cell title]]);
+  cell = [controller tableView:table dataCellForTableColumn:title_column row:1];
+  EXPECT_TRUE([@"zzz" isEqualToString:[cell title]]);
+
+  // Releases the controller, which in turn deletes |bridge|.
+  [controller close];
+
+  task_manager.RemoveResource(&resource1);
+  task_manager.RemoveResource(&resource2);
+}

chrome/browser/task_manager.h

@@ -152,6 +152,8 @@ class TaskManager {
   FRIEND_TEST_ALL_PREFIXES(TaskManagerTest, Basic);
   FRIEND_TEST_ALL_PREFIXES(TaskManagerTest, Resources);
   FRIEND_TEST_ALL_PREFIXES(TaskManagerTest, RefreshCalled);
+  FRIEND_TEST_ALL_PREFIXES(TaskManagerWindowControllerTest, Init);
+  FRIEND_TEST_ALL_PREFIXES(TaskManagerWindowControllerTest, Sort);
 
   // Obtain an instance via GetInstance().
   TaskManager();