TaskManager: Added functionality to remember the size of the task manager dialog.

TaskManager: Added functionality to remember the size of the task manager dialog so that it can be restored with the same size in further launches.


BUG=105117
TEST=None


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

[NaveenBobbili (Motorola), 2011-11-30 10:58:06]

Please review

[James Hawkins, 2011-11-30 16:20:32]

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/gtk/html_dial...
File chrome/browser/ui/gtk/html_dialog_gtk.cc (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/gtk/html_dial...
chrome/browser/ui/gtk/html_dialog_gtk.cc:119: // Stores the dialog bounds.
s/Stores/Store/

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/html_di...
File chrome/browser/ui/webui/html_dialog_ui.h (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/html_di...
chrome/browser/ui/webui/html_dialog_ui.h:74: virtual void
StoreDialogSize(gfx::Rect dialog_bounds){}
Space after closing parenthesis.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
File chrome/browser/ui/webui/task_manager_dialog.cc (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:68: int top = 0, left = 0, bottom
= 1, right = 1;
Why do you set top,left to 0 and bottom,right to 1 when you're about to
overwrite them?

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:74: size->SetSize(std::max(1,
right - left), std::max(1,bottom - top));
Space after comma.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:95: 
You added a blank line here, but there are not blank lines between the other
virtual methods. Rule #1: Be consistent.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:107: // *don't* restore them.
If this is the case, then why not just set "width", "height"?

[NaveenBobbili (Motorola), 2011-12-01 07:31:21]

I fixed review comments. Please take a look now.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/gtk/html_dial...
File chrome/browser/ui/gtk/html_dialog_gtk.cc (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/gtk/html_dial...
chrome/browser/ui/gtk/html_dialog_gtk.cc:119: // Stores the dialog bounds.
On 2011/11/30 16:20:32, James Hawkins wrote:
> s/Stores/Store/

Done.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/html_di...
File chrome/browser/ui/webui/html_dialog_ui.h (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/html_di...
chrome/browser/ui/webui/html_dialog_ui.h:74: virtual void
StoreDialogSize(gfx::Rect dialog_bounds){}
On 2011/11/30 16:20:32, James Hawkins wrote:
> Space after closing parenthesis.

Done.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
File chrome/browser/ui/webui/task_manager_dialog.cc (right):

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:68: int top = 0, left = 0, bottom
= 1, right = 1;
On 2011/11/30 16:20:32, James Hawkins wrote:
> Why do you set top,left to 0 and bottom,right to 1 when you're about to
> overwrite them?

I changed it to only store width and height now.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:74: size->SetSize(std::max(1,
right - left), std::max(1,bottom - top));
On 2011/11/30 16:20:32, James Hawkins wrote:
> Space after comma.

Done.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:95: 
On 2011/11/30 16:20:32, James Hawkins wrote:
> You added a blank line here, but there are not blank lines between the other
> virtual methods. Rule #1: Be consistent.

Sure . Done.

http://codereview.chromium.org/8741010/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:107: // *don't* restore them.
On 2011/11/30 16:20:32, James Hawkins wrote:
> If this is the case, then why not just set "width", "height"?

I changed it to only store width and height now.

[yoshiki, 2011-12-01 14:28:30]

http://codereview.chromium.org/8741010/diff/6001/chrome/browser/ui/webui/html...
File chrome/browser/ui/webui/html_dialog_ui.h (right):

http://codereview.chromium.org/8741010/diff/6001/chrome/browser/ui/webui/html...
chrome/browser/ui/webui/html_dialog_ui.h:74: virtual void
StoreDialogSize(gfx::Rect dialog_bounds) {}
Could you add "const" modifier to dialog_bounds argument?

[NaveenBobbili (Motorola), 2011-12-02 07:58:11]

Fixed comments. Please take a look now.

http://codereview.chromium.org/8741010/diff/6001/chrome/browser/ui/webui/html...
File chrome/browser/ui/webui/html_dialog_ui.h (right):

http://codereview.chromium.org/8741010/diff/6001/chrome/browser/ui/webui/html...
chrome/browser/ui/webui/html_dialog_ui.h:74: virtual void
StoreDialogSize(gfx::Rect dialog_bounds) {}
On 2011/12/01 14:28:30, yoshiki wrote:
> Could you add "const" modifier to dialog_bounds argument?

Done.

[James Hawkins, 2011-12-02 16:31:30]

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
File chrome/browser/ui/webui/task_manager_dialog.cc (right):

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
chrome/browser/ui/webui/task_manager_dialog.cc:63: // If we previously saved the
dialog's bounds, use them.
Don't use 'we' in comments, it's ambiguous.

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
chrome/browser/ui/webui/task_manager_dialog.cc:97: // Store the dialog's size so
we can restore it the next time it's opened.
Same here about 'we'.

[NaveenBobbili (Motorola), 2011-12-03 01:25:33]

Fixed comments . Please review.

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
File chrome/browser/ui/webui/task_manager_dialog.cc (right):

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
chrome/browser/ui/webui/task_manager_dialog.cc:63: // If we previously saved the
dialog's bounds, use them.
On 2011/12/02 16:31:31, James Hawkins wrote:
> Don't use 'we' in comments, it's ambiguous.

Done.

http://codereview.chromium.org/8741010/diff/13001/chrome/browser/ui/webui/tas...
chrome/browser/ui/webui/task_manager_dialog.cc:97: // Store the dialog's size so
we can restore it the next time it's opened.
On 2011/12/02 16:31:31, James Hawkins wrote:
> Same here about 'we'.

Done.

[James Hawkins, 2011-12-04 18:08:04]

lgtm

[yoshiki, 2011-12-05 04:18:40]

LGTM

[commit-bot: I haz the power, 2011-12-05 09:09:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qghc36@motorola.com/8741010/15001

[commit-bot: I haz the power, 2011-12-05 09:09:25]

Can't apply patch for file chrome/browser/ui/webui/task_manager_dialog.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/ui/webui/task_manager_dialog.cc
Hunk #1 FAILED at 7.
Hunk #2 succeeded at 57 (offset 1 line).
Hunk #3 succeeded at 90 (offset 1 line).
1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/ui/webui/task_manager_dialog.cc.rej

[commit-bot: I haz the power, 2011-12-06 11:05:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/qghc36@motorola.com/8741010/21001

[commit-bot: I haz the power, 2011-12-06 13:49:51]

Change committed as 113198