Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(182)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 7604032: Showing Task manager on the foreground window. (Closed)

Created:
9 years, 4 months ago by yoshiki
Modified:
9 years, 4 months ago
Reviewers:
xiyuan, prasadt
CC:
chromium-reviews, jennb, Dmitry Titov, jianli, dcheng, Paweł Hajdan Jr.
Visibility:
Public.

Description

Showing Task manager on the foreground window. If the other Task Manager opens, closes it and opens new Task Manager on the current window. BUG=chromium-os:8272 TEST=manual on chromium-os Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96881

Patch Set 1 #

Total comments: 12

Patch Set 2 : to be commited #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -42 lines) Patch
M chrome/browser/ui/browser.h View 1 1 chunk +6 lines, -6 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/browser_window.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/browser_window_gtk.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/panels/panel.h View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/panels/panel.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/task_manager_dialog.cc View 1 5 chunks +27 lines, -11 lines 0 comments Download
M chrome/test/base/test_browser_window.h View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/test/base/test_browser_window.cc View 1 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
yoshiki
Please review the code.
9 years, 4 months ago (2011-08-11 05:44:43 UTC) #1
xiyuan
LGTM with nits http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/browser.h File chrome/browser/ui/browser.h (right): http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/browser.h#newcode440 chrome/browser/ui/browser.h:440: gfx::NativeWindow BrowserShowHtmlDialog(HtmlDialogUIDelegate* delegate, nit: Update comments ...
9 years, 4 months ago (2011-08-11 16:36:47 UTC) #2
prasadt
LGTM with one nit. http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel.cc File chrome/browser/ui/panels/panel.cc (right): http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel.cc#newcode315 chrome/browser/ui/panels/panel.cc:315: gfx::NativeWindow parent_window) { Fix indenting.
9 years, 4 months ago (2011-08-11 17:30:37 UTC) #3
yoshiki
9 years, 4 months ago (2011-08-16 02:13:30 UTC) #4 
Thanks!

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/browser.h
File chrome/browser/ui/browser.h (right):

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/browser.h#new...
chrome/browser/ui/browser.h:440: gfx::NativeWindow
BrowserShowHtmlDialog(HtmlDialogUIDelegate* delegate,
On 2011/08/11 16:36:47, xiyuan wrote:
> nit: Update comments above for the return value?

Done.

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:315: gfx::NativeWindow parent_window) {
On 2011/08/11 17:30:37, prasadt wrote:
> Fix indenting.

Done.

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel.h
File chrome/browser/ui/panels/panel.h (right):

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.h:109: gfx::NativeWindow parent_window) OVERRIDE;
On 2011/08/11 16:36:47, xiyuan wrote:
> nit: either align with arg on previous line or 4-space indent.

Done.

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

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:107: return;
On 2011/08/11 16:36:47, xiyuan wrote:
> Could we do something to bring up existing task manager on other platforms?
> Would window_->Activate() work?

Done.

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:111: show_count_++;
On 2011/08/11 16:36:47, xiyuan wrote:
> nit: ++show_count_;

Done.

http://codereview.chromium.org/7604032/diff/1/chrome/browser/ui/webui/task_ma...
chrome/browser/ui/webui/task_manager_dialog.cc:116: show_count_--;
On 2011/08/11 16:36:47, xiyuan wrote:
> nit: --show_count_;

Done.

Powered by Google App Engine
This is Rietveld 408576698