Task Manager for OS X: Set up columns, show rows.

Task Manager for OS X: Set up columns, show rows.

This is more work-in-progress for the OS X task manager. The table header is complete (the right columns are there, and right-clicking the header gives a column-selection context menu just like in e.g. Task Manager), and the rows data is displayed. However, all the metrics (memory, cpu, etc) seem to be all wrong. Because of this and because too much stuff is still unimplemented (favicons in rows, the "kill process" button, the link, etc), I won't commit the change to MainMenu.xib, so users still won't be able to open this dialog.

BUG=13156
TEST=Connect view->task manager to first responder's commandDispatch:, set the menu item to "enabled", and give the menu item the tag 40005. Now opening the task manager should actually display data. Right-clicking the table header should show a functional menu that lets you select which columns to display. Clicking a column header should not select the column, double-clicking an item should not edit it. Also, when resizing the task manager, stuff should now behave in a sane way.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26687

[Nico, 2009-09-21 04:48:33]

More work-in-progress for the task manager.

[pink (ping after 24hrs), 2009-09-21 15:31:32]

Be sure to revert the main menu change. I'll take another look once you've added
in the comments.

http://codereview.chromium.org/208043/diff/1/5
File chrome/browser/cocoa/task_manager_mac.mm (right):

http://codereview.chromium.org/208043/diff/1/5#newcode56
Line 56: [tableView_ setDataSource:self];
you can't do this from the nib?

http://codereview.chromium.org/208043/diff/1/5#newcode59
Line 59: - (void)addColumnWithId:(int)columnId visible:(BOOL)isVisible {
needs function-level comment. For example, why would you want to add invisible
columns?

http://codereview.chromium.org/208043/diff/1/5#newcode76
Line 76: - (void)setUpTableColumns {
needs function comment

http://codereview.chromium.org/208043/diff/1/5#newcode89
Line 89: - (void)setUpTableHeaderContextMenu {
needs function comment

http://codereview.chromium.org/208043/diff/1/5#newcode104
Line 104: - (void)toggleColumn:(id)item {
needs function comment

http://codereview.chromium.org/208043/diff/1/5#newcode171
Line 171: objectValueForTableColumn:(NSTableColumn*)tableColumn
indent 4 spaces

http://codereview.chromium.org/208043/diff/1/5#newcode172
Line 172: row:(NSInteger)rowIndex {
line up : with line above

[Nico, 2009-09-21 15:52:46]

http://codereview.chromium.org/208043/diff/1/5
File chrome/browser/cocoa/task_manager_mac.mm (right):

http://codereview.chromium.org/208043/diff/1/5#newcode56
Line 56: [tableView_ setDataSource:self];
On 2009/09/21 15:31:32, pink wrote:
> you can't do this from the nib?

I can, but for some reason I liked it better this way yesterday. Anyway, Done.

As a consequence, I had to weaken the DCHECK() below, since Cocoa apparently
sets up the data source before settting tableView_.

http://codereview.chromium.org/208043/diff/1/5#newcode59
Line 59: - (void)addColumnWithId:(int)columnId visible:(BOOL)isVisible {
On 2009/09/21 15:31:32, pink wrote:
> needs function-level comment. For example, why would you want to add invisible
> columns?

Done.

http://codereview.chromium.org/208043/diff/1/5#newcode76
Line 76: - (void)setUpTableColumns {
On 2009/09/21 15:31:32, pink wrote:
> needs function comment

Done.

http://codereview.chromium.org/208043/diff/1/5#newcode89
Line 89: - (void)setUpTableHeaderContextMenu {
On 2009/09/21 15:31:32, pink wrote:
> needs function comment

Done.

http://codereview.chromium.org/208043/diff/1/5#newcode104
Line 104: - (void)toggleColumn:(id)item {
On 2009/09/21 15:31:32, pink wrote:
> needs function comment

Done.

http://codereview.chromium.org/208043/diff/1/5#newcode171
Line 171: objectValueForTableColumn:(NSTableColumn*)tableColumn
On 2009/09/21 15:31:32, pink wrote:
> indent 4 spaces

Done.

http://codereview.chromium.org/208043/diff/1/5#newcode172
Line 172: row:(NSInteger)rowIndex {
On 2009/09/21 15:31:32, pink wrote:
> line up : with line above

Done. The intention was to be consistent with at least one of the two other
places where we have this function. But I guess since the two other places are
not consistent with each other, making this one inconsistent with those two is
just another kind of consistency ;-)

[pink (ping after 24hrs), 2009-09-21 16:15:19]

lgtm